Added initial out-of-memory handling tests

The tests are very static and only verifies the handling of the
first allocation in the API functions.
Next step could be to randomize when an allocation fail to make
sure that all allocation failures are handled.

README.md

@@ -15,7 +15,7 @@ Hiredis-cluster is a fork of Hiredis-vip, with the following improvements:
 * Using CMake as build system
 * Code style guide (using clang-format)
 * Improved testing
-* Memory leak corrections
+* Memory leak corrections and allocation failure handling
 
 ## Features
 

hircluster.c

@@ -858,7 +858,7 @@ dict *parse_cluster_slots(redisClusterContext *cc, redisReply *reply,
                         master = dictGetEntryVal(den);
                         ret = cluster_slot_ref_node(slot, master);
                         if (ret != REDIS_OK) {
-                            goto oom;
+                            goto error;
                         }
 
                         slot = NULL;
@@ -886,7 +886,7 @@ dict *parse_cluster_slots(redisClusterContext *cc, redisReply *reply,
 
                     ret = cluster_slot_ref_node(slot, master);
                     if (ret != REDIS_OK) {
-                        goto oom;
+                        goto error;
                     }
 
                     slot = NULL;
@@ -1767,6 +1767,7 @@ int redisClusterSetOptionAddNode(redisClusterContext *cc, const char *addr) {
     if (cc->nodes == NULL) {
         cc->nodes = dictCreate(&clusterNodesDictType, NULL);
         if (cc->nodes == NULL) {
+            __redisClusterSetError(cc, REDIS_ERR_OOM, "Out of memory");
             return REDIS_ERR;
         }
     }
@@ -1955,6 +1956,7 @@ int redisClusterSetOptionConnectTimeout(redisClusterContext *cc,
     if (cc->connect_timeout == NULL) {
         cc->connect_timeout = hi_malloc(sizeof(struct timeval));
         if (cc->connect_timeout == NULL) {
+            __redisClusterSetError(cc, REDIS_ERR_OOM, "Out of memory");
             return REDIS_ERR;
         }
     }
@@ -4169,7 +4171,7 @@ int redisClusterAsyncFormattedCommand(redisClusterAsyncContext *acc,
 
     cad = cluster_async_data_get();
     if (cad == NULL) {
-        goto error;
+        goto oom;
     }
 
     cad->acc = acc;

tests/CMakeLists.txt

@@ -78,6 +78,11 @@ if(ENABLE_IPV6_TESTS)
   set_tests_properties(ct_connection_ipv6 PROPERTIES LABELS "CT")
 endif()
 
+add_executable(ct_out_of_memory_handling ct_out_of_memory_handling.c)
+target_link_libraries(ct_out_of_memory_handling hiredis_cluster hiredis ${SSL_LIBRARY})
+add_test(NAME ct_out_of_memory_handling COMMAND "$<TARGET_FILE:ct_out_of_memory_handling>")
+set_tests_properties(ct_out_of_memory_handling PROPERTIES LABELS "CT")
+
 if(ENABLE_SSL)
   # Executable: tls
   add_executable(example_tls main_tls.c)

tests/ct_out_of_memory_handling.c

@@ -0,0 +1,216 @@
+#include "hircluster.h"
+#include "test_utils.h"
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define CLUSTER_NODE "127.0.0.1:7000"
+
+// A OOM failing malloc()
+static void *hi_malloc_fail(size_t size) {
+    UNUSED(size);
+    return NULL;
+}
+
+// A OOM failing calloc()
+static void *hi_calloc_fail(size_t nmemb, size_t size) {
+    UNUSED(nmemb);
+    UNUSED(size);
+    return NULL;
+}
+
+// A OOM failing realloc()
+static void *hi_realloc_fail(void *ptr, size_t size) {
+    UNUSED(ptr);
+    UNUSED(size);
+    return NULL;
+}
+
+// Reset the error string (between tests)
+void reset_cluster_errors(redisClusterContext *cc) {
+    cc->err = 0;
+    memset(cc->errstr, '\0', strlen(cc->errstr));
+}
+
+// Test OOM handling in first allocation in:
+// redisClusterContextInit();
+void test_context_init() {
+    hiredisAllocFuncs ha = {
+        .mallocFn = hi_malloc_fail,
+        .callocFn = hi_calloc_fail,
+        .reallocFn = hi_realloc_fail,
+        .strdupFn = strdup,
+        .freeFn = free,
+    };
+
+    // Override allocators
+    hiredisSetAllocators(&ha);
+    {
+        redisClusterContext *cc = redisClusterContextInit();
+        assert(cc == NULL);
+    }
+    hiredisResetAllocators();
+}
+
+// Test OOM handling in first allocation in:
+// redisClusterSetOptionXXXX
+void test_setoptions() {
+    hiredisAllocFuncs ha = {
+        .mallocFn = hi_malloc_fail,
+        .callocFn = hi_calloc_fail,
+        .reallocFn = hi_realloc_fail,
+        .strdupFn = strdup,
+        .freeFn = free,
+    };
+
+    redisClusterContext *cc = redisClusterContextInit();
+    assert(cc);
+    ASSERT_STR_EQ(cc->errstr, "");
+
+    // Override allocators
+    hiredisSetAllocators(&ha);
+    {
+        int result;
+        result = redisClusterSetOptionAddNodes(cc, CLUSTER_NODE);
+        assert(result == REDIS_ERR);
+        ASSERT_STR_STARTS_WITH(cc->errstr, "servers address is error");
+
+        reset_cluster_errors(cc);
+
+        result = redisClusterSetOptionAddNode(cc, CLUSTER_NODE);
+        assert(result == REDIS_ERR);
+        ASSERT_STR_EQ(cc->errstr, "Out of memory");
+
+        reset_cluster_errors(cc);
+
+        struct timeval timeout = {0, 500000};
+        result = redisClusterSetOptionConnectTimeout(cc, timeout);
+        assert(result == REDIS_ERR);
+        ASSERT_STR_EQ(cc->errstr, "Out of memory");
+
+        reset_cluster_errors(cc);
+
+        result = redisClusterSetOptionTimeout(cc, timeout);
+        assert(result == REDIS_ERR);
+        ASSERT_STR_EQ(cc->errstr, "Out of memory");
+    }
+    hiredisResetAllocators();
+
+    redisClusterFree(cc);
+}
+
+// Test OOM handling in first allocation in:
+// redisClusterConnect2()
+void test_cluster_connect() {
+    hiredisAllocFuncs ha = {
+        .mallocFn = hi_malloc_fail,
+        .callocFn = hi_calloc_fail,
+        .reallocFn = hi_realloc_fail,
+        .strdupFn = strdup,
+        .freeFn = free,
+    };
+    struct timeval timeout = {0, 500000};
+
+    redisClusterContext *cc = redisClusterContextInit();
+    assert(cc);
+    redisClusterSetOptionAddNodes(cc, CLUSTER_NODE);
+    redisClusterSetOptionConnectTimeout(cc, timeout);
+    ASSERT_STR_EQ(cc->errstr, "");
+
+    // Override allocators
+    hiredisSetAllocators(&ha);
+    {
+        int result;
+        result = redisClusterConnect2(cc);
+        assert(result == REDIS_ERR);
+        ASSERT_STR_EQ(cc->errstr, "Out of memory");
+    }
+    hiredisResetAllocators();
+
+    redisClusterFree(cc);
+}
+
+// Test OOM handling in first allocation in:
+// redisClusterCommand()
+void test_cluster_command() {
+    hiredisAllocFuncs ha = {
+        .mallocFn = hi_malloc_fail,
+        .callocFn = hi_calloc_fail,
+        .reallocFn = hi_realloc_fail,
+        .strdupFn = strdup,
+        .freeFn = free,
+    };
+
+    struct timeval timeout = {0, 500000};
+
+    redisClusterContext *cc = redisClusterContextInit();
+    assert(cc);
+    redisClusterSetOptionAddNodes(cc, CLUSTER_NODE);
+    redisClusterSetOptionConnectTimeout(cc, timeout);
+
+    int result;
+    result = redisClusterConnect2(cc);
+    ASSERT_MSG(result == REDIS_OK, cc->errstr);
+    ASSERT_STR_EQ(cc->errstr, "");
+
+    // Override allocators
+    hiredisSetAllocators(&ha);
+    {
+        redisReply *reply;
+        reply = (redisReply *)redisClusterCommand(cc, "SET key value");
+        assert(reply == NULL);
+
+        ASSERT_STR_EQ(cc->errstr, "Out of memory");
+    }
+    hiredisResetAllocators();
+
+    redisClusterFree(cc);
+}
+
+// Test OOM handling in first allocation in:
+// redisClusterAppendCommand()
+void test_cluster_append_command() {
+    hiredisAllocFuncs ha = {
+        .mallocFn = hi_malloc_fail,
+        .callocFn = hi_calloc_fail,
+        .reallocFn = hi_realloc_fail,
+        .strdupFn = strdup,
+        .freeFn = free,
+    };
+
+    struct timeval timeout = {0, 500000};
+
+    redisClusterContext *cc = redisClusterContextInit();
+    assert(cc);
+    redisClusterSetOptionAddNodes(cc, CLUSTER_NODE);
+    redisClusterSetOptionConnectTimeout(cc, timeout);
+
+    int result;
+    result = redisClusterConnect2(cc);
+    ASSERT_MSG(result == REDIS_OK, cc->errstr);
+    ASSERT_STR_EQ(cc->errstr, "");
+
+    // Override allocators
+    hiredisSetAllocators(&ha);
+    {
+        result = redisClusterAppendCommand(cc, "SET foo one");
+        assert(result == REDIS_ERR);
+        ASSERT_STR_EQ(cc->errstr, "Out of memory");
+    }
+    hiredisResetAllocators();
+
+    redisClusterFree(cc);
+}
+
+int main() {
+    // Test the handling of an out-of-memory situation
+    // in the first allocation done in the API functions.
+    test_context_init();
+    test_setoptions();
+    test_cluster_connect();
+    test_cluster_command();
+    test_cluster_append_command();
+
+    return 0;
+}

tests/test_utils.h

@@ -57,4 +57,10 @@
                    _ctx->errstr);                                              \
     }
 
+#define ASSERT_STR_EQ(_s1, _s2)                                                \
+    { assert(strcmp(_s1, _s2) == 0); }
+
+#define ASSERT_STR_STARTS_WITH(_s1, _s2)                                       \
+    { assert(strncmp(_s1, _s2, strlen(_s2)) == 0); }
+
 #endif