From b8eb2a73408fa3b8845760857dd6fcccb62107fe Mon Sep 17 00:00:00 2001 From: sundb Date: Thu, 31 Mar 2022 20:26:10 +0800 Subject: [PATCH] Fix failing moduleconfigs tests and memory leak (#10501) Fix global `strval` not reset to NULL after being freed, causing a crash on alpine (most likely because the dynamic library loader doesn't init globals on reload) By the way, fix the memory leak of using `RedisModule_Free` to free `RedisModuleString`, and add a corresponding test. --- tests/modules/moduleconfigs.c | 7 +++++-- tests/unit/moduleapi/moduleconfigs.tcl | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/modules/moduleconfigs.c b/tests/modules/moduleconfigs.c index ddf254ab3..a9e434a7b 100644 --- a/tests/modules/moduleconfigs.c +++ b/tests/modules/moduleconfigs.c @@ -48,7 +48,7 @@ int setStringConfigCommand(const char *name, RedisModuleString *new, void *privd *err = RedisModule_CreateString(NULL, "Cannot set string to 'rejectisfreed'", 36); return REDISMODULE_ERR; } - RedisModule_Free(strval); + if (strval) RedisModule_FreeString(NULL, strval); RedisModule_RetainString(NULL, new); strval = new; return REDISMODULE_OK; @@ -134,6 +134,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) int RedisModule_OnUnload(RedisModuleCtx *ctx) { REDISMODULE_NOT_USED(ctx); - if (strval) RedisModule_FreeString(ctx, strval); + if (strval) { + RedisModule_FreeString(ctx, strval); + strval = NULL; + } return REDISMODULE_OK; } \ No newline at end of file diff --git a/tests/unit/moduleapi/moduleconfigs.tcl b/tests/unit/moduleapi/moduleconfigs.tcl index 0bb3a639c..01aa1e88e 100644 --- a/tests/unit/moduleapi/moduleconfigs.tcl +++ b/tests/unit/moduleapi/moduleconfigs.tcl @@ -22,6 +22,9 @@ start_server {tags {"modules"}} { assert_equal [r config get moduleconfigs.memory_numeric] "moduleconfigs.memory_numeric 1048576" r config set moduleconfigs.string wafflewednesdays assert_equal [r config get moduleconfigs.string] "moduleconfigs.string wafflewednesdays" + set not_embstr [string repeat A 50] + r config set moduleconfigs.string $not_embstr + assert_equal [r config get moduleconfigs.string] "moduleconfigs.string $not_embstr" r config set moduleconfigs.string \x73\x75\x70\x65\x72\x20\x00\x73\x65\x63\x72\x65\x74\x20\x70\x61\x73\x73\x77\x6f\x72\x64 assert_equal [r config get moduleconfigs.string] "moduleconfigs.string {super \0secret password}" r config set moduleconfigs.enum two