Merge pull request #6496 from oranagra/module_tests_valgrind

Make module tests pass with valgrind, and fix a leak in diskless load

4 files changed, 36 insertions, 28 deletions

diff --git a/src/rdb.c b/src/rdb.c
index c92b9f4a1..f530219a4 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -2172,7 +2172,11 @@ int rdbLoadRio(rio *rdb, rdbSaveInfo *rsi, int loading_aof) {
         /* Read key */
         if ((key = rdbLoadStringObject(rdb)) == NULL) goto eoferr;
         /* Read value */
-        if ((val = rdbLoadObject(type,rdb,key)) == NULL) goto eoferr;
+        if ((val = rdbLoadObject(type,rdb,key)) == NULL) {
+            decrRefCount(key);
+            goto eoferr;
+        }
+
         /* Check if the key already expired. This function is used when loading
          * an RDB file from disk, either at startup, or when an RDB was
          * received from the master. In the latter case, the master is
diff --git a/tests/modules/commandfilter.c b/tests/modules/commandfilter.c
index d25d49c44..571ed1701 100644
--- a/tests/modules/commandfilter.c
+++ b/tests/modules/commandfilter.c
@@ -147,3 +147,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 
     return REDISMODULE_OK;
 }
+
+int RedisModule_OnUnload(RedisModuleCtx *ctx) {
+    RedisModule_FreeString(ctx, log_key_name);
+    return REDISMODULE_OK;
+}
diff --git a/tests/modules/testrdb.c b/tests/modules/testrdb.c
index d73c8bfd3..eb8d1a999 100644
--- a/tests/modules/testrdb.c
+++ b/tests/modules/testrdb.c
@@ -238,3 +238,11 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 
     return REDISMODULE_OK;
 }
+
+int RedisModule_OnUnload(RedisModuleCtx *ctx) {
+    if (before_str)
+        RedisModule_FreeString(ctx, before_str);
+    if (after_str)
+        RedisModule_FreeString(ctx, after_str);
+    return REDISMODULE_OK;
+}
diff --git a/tests/unit/moduleapi/testrdb.tcl b/tests/unit/moduleapi/testrdb.tcl
index c72570002..a93b34b69 100644
--- a/tests/unit/moduleapi/testrdb.tcl
+++ b/tests/unit/moduleapi/testrdb.tcl
@@ -1,57 +1,48 @@
 set testmodule [file normalize tests/modules/testrdb.so]
 
-proc restart_and_wait {} {
-    catch {
-        r debug restart
-    }
-
-    # wait for the server to come back up
-    set retry 50
-    while {$retry} {
-        if {[catch { r ping }]} {
-            after 100
-        } else {
-            break
-        }
-        incr retry -1
-    }
-}
-
 tags "modules" {
-    start_server [list overrides [list loadmodule "$testmodule"]] {
-        test {modules are able to persist types} {
+    test {modules are able to persist types} {
+        start_server [list overrides [list loadmodule "$testmodule"]] {
             r testrdb.set.key key1 value1
             assert_equal "value1" [r testrdb.get.key key1]
             r debug reload
             assert_equal "value1" [r testrdb.get.key key1]
         }
+    }
 
-        test {modules global are lost without aux} {
+    test {modules global are lost without aux} {
+        set server_path [tmpdir "server.module-testrdb"]
+        start_server [list overrides [list loadmodule "$testmodule" "dir" $server_path]] {
             r testrdb.set.before global1
             assert_equal "global1" [r testrdb.get.before]
-            restart_and_wait
+        }
+        start_server [list overrides [list loadmodule "$testmodule" "dir" $server_path]] {
             assert_equal "" [r testrdb.get.before]
         }
     }
 
-    start_server [list overrides [list loadmodule "$testmodule 2"]] {
-        test {modules are able to persist globals before and after} {
+    test {modules are able to persist globals before and after} {
+        set server_path [tmpdir "server.module-testrdb"]
+        start_server [list overrides [list loadmodule "$testmodule 2" "dir" $server_path]] {
             r testrdb.set.before global1
             r testrdb.set.after global2
             assert_equal "global1" [r testrdb.get.before]
             assert_equal "global2" [r testrdb.get.after]
-            restart_and_wait
+        }
+        start_server [list overrides [list loadmodule "$testmodule 2" "dir" $server_path]] {
             assert_equal "global1" [r testrdb.get.before]
             assert_equal "global2" [r testrdb.get.after]
         }
 
     }
 
-    start_server [list overrides [list loadmodule "$testmodule 1"]] {
-        test {modules are able to persist globals just after} {
+    test {modules are able to persist globals just after} {
+        set server_path [tmpdir "server.module-testrdb"]
+        start_server [list overrides [list loadmodule "$testmodule 1" "dir" $server_path]] {
             r testrdb.set.after global2
             assert_equal "global2" [r testrdb.get.after]
-            restart_and_wait
+        }
+        start_server [list overrides [list loadmodule "$testmodule 1" "dir" $server_path]] {
             assert_equal "global2" [r testrdb.get.after]
         }
     }