Add OOM testing of parsing replicas

Fix memory issues when the parsing of replicas fails. Signed-off-by: Björn Svensson <[email protected]>
bjosv · Oct 11, 2024 · c1a80bb · c1a80bb
1 parent 12920a0
commit c1a80bb
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 5 deletions.
diff --git a/src/cluster.c b/src/cluster.c
@@ -958,7 +958,6 @@ static dict *parse_cluster_nodes(valkeyClusterContext *cc, valkeyReply *reply) {
                     ret = cluster_master_slave_mapping_with_name(
                         cc, &nodes_name, master, master->name);
                     if (ret != VALKEY_OK) {
-                        freeValkeyClusterNode(master);
                         goto error;
                     }
                 }
@@ -1054,6 +1053,15 @@ static dict *parse_cluster_nodes(valkeyClusterContext *cc, valkeyReply *reply) {
         dictRelease(nodes);
     }
     if (nodes_name != NULL) {
+        /* Only free parsed replicas since the `nodes` dict owns primary nodes. */
+        dictIterator di;
+        dictInitIterator(&di, nodes_name);
+        dictEntry *de;
+        while ((de = dictNext(&di))) {
+            valkeyClusterNode *node = dictGetEntryVal(de);
+            if (node->role == VALKEY_ROLE_SLAVE)
+                freeValkeyClusterNode(node);
+        }
         dictRelease(nodes_name);
     }
     return NULL;

diff --git a/tests/ct_out_of_memory_handling.c b/tests/ct_out_of_memory_handling.c
@@ -125,6 +125,7 @@ void test_alloc_failure_handling(void) {
         cc = valkeyClusterContextInit();
         assert(cc);
     }
+    cc->flags |= VALKEYCLUSTER_FLAG_ADD_SLAVE;
 
     // Add nodes
     {
@@ -170,14 +171,14 @@ void test_alloc_failure_handling(void) {
 
     // Connect
     {
-        for (int i = 0; i < 128; ++i) {
+        for (int i = 0; i < 148; ++i) {
             prepare_allocation_test(cc, i);
             result = valkeyClusterConnect2(cc);
             assert(result == VALKEY_ERR);
             ASSERT_STR_EQ(cc->errstr, "Out of memory");
         }
 
-        prepare_allocation_test(cc, 128);
+        prepare_allocation_test(cc, 148);
         result = valkeyClusterConnect2(cc);
         assert(result == VALKEY_OK);
     }
@@ -493,6 +494,7 @@ void test_alloc_failure_handling_async(void) {
         acc = valkeyClusterAsyncContextInit();
         assert(acc);
     }
+    acc->cc->flags |= VALKEYCLUSTER_FLAG_ADD_SLAVE;
 
     // Set callbacks
     {
@@ -519,14 +521,14 @@ void test_alloc_failure_handling_async(void) {
 
     // Connect
     {
-        for (int i = 0; i < 126; ++i) {
+        for (int i = 0; i < 146; ++i) {
             prepare_allocation_test(acc->cc, i);
             result = valkeyClusterConnect2(acc->cc);
             assert(result == VALKEY_ERR);
             ASSERT_STR_EQ(acc->cc->errstr, "Out of memory");
         }
 
-        prepare_allocation_test(acc->cc, 126);
+        prepare_allocation_test(acc->cc, 146);
         result = valkeyClusterConnect2(acc->cc);
         assert(result == VALKEY_OK);
     }