see #7544, added RedisModule_HoldString api. (#7577)

Added RedisModule_HoldString that either returns a
shallow copy of the given String (by increasing
the String ref count) or a new deep copy of String
in case its not possible to get a shallow copy.

Co-authored-by: Itamar Haber <itamar@redislabs.com>

src/module.c

@@ -1138,6 +1138,65 @@ void RM_RetainString(RedisModuleCtx *ctx, RedisModuleString *str) {
     }
 }
 
+/**
+* This function can be used instead of RedisModule_RetainString().
+* The main difference between the two is that this function will always
+* succeed, whereas RedisModule_RetainString() may fail because of an
+* assertion.
+* 
+* The function returns a pointer to RedisModuleString, which is owned
+* by the caller. It requires a call to RedisModule_FreeString() to free
+* the string when automatic memory management is disabled for the context.
+* When automatic memory management is enabled, you can either call
+* RedisModule_FreeString() or let the automation free it.
+* 
+* This function is more efficient than RedisModule_CreateStringFromString()
+* because whenever possible, it avoids copying the underlying
+* RedisModuleString. The disadvantage of using this function is that it
+* might not be possible to use RedisModule_StringAppendBuffer() on the
+* returned RedisModuleString.
+* 
+* It is possible to call this function with a NULL context.
+ */
+RedisModuleString* RM_HoldString(RedisModuleCtx *ctx, RedisModuleString *str) {
+    if (str->refcount == OBJ_STATIC_REFCOUNT) {
+        return RM_CreateStringFromString(ctx, str);
+    }
+
+    incrRefCount(str);
+    if (ctx != NULL) {
+        /*
+         * Put the str in the auto memory management of the ctx.
+         * It might already be there, in this case, the ref count will
+         * be 2 and we will decrease the ref count twice and free the
+         * object in the auto memory free function.
+         *
+         * Why we can not do the same trick of just remove the object
+         * from the auto memory (like in RM_RetainString)?
+         * This code shows the issue:
+         *
+         * RM_AutoMemory(ctx);
+         * str1 = RM_CreateString(ctx, "test", 4);
+         * str2 = RM_HoldString(ctx, str1);
+         * RM_FreeString(str1);
+         * RM_FreeString(str2);
+         *
+         * If after the RM_HoldString we would just remove the string from
+         * the auto memory, this example will cause access to a freed memory
+         * on 'RM_FreeString(str2);' because the String will be free
+         * on 'RM_FreeString(str1);'.
+         *
+         * So it's safer to just increase the ref count
+         * and add the String to auto memory again.
+         *
+         * The limitation is that it is not possible to use RedisModule_StringAppendBuffer
+         * on the String.
+         */
+        autoMemoryAdd(ctx,REDISMODULE_AM_STRING,str);
+    }
+    return str;
+}
+
 /* Given a string module object, this function returns the string pointer
  * and length of the string. The returned pointer and length should only
  * be used for read only accesses and never modified. */
@@ -7832,6 +7891,7 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(LatencyAddSample);
     REGISTER_API(StringAppendBuffer);
     REGISTER_API(RetainString);
+    REGISTER_API(HoldString);
     REGISTER_API(StringCompare);
     REGISTER_API(GetContextFromIO);
     REGISTER_API(GetKeyNameFromIO);

src/redismodule.h

@@ -569,6 +569,7 @@ void REDISMODULE_API_FUNC(RedisModule__Assert)(const char *estr, const char *fil
 void REDISMODULE_API_FUNC(RedisModule_LatencyAddSample)(const char *event, mstime_t latency);
 int REDISMODULE_API_FUNC(RedisModule_StringAppendBuffer)(RedisModuleCtx *ctx, RedisModuleString *str, const char *buf, size_t len);
 void REDISMODULE_API_FUNC(RedisModule_RetainString)(RedisModuleCtx *ctx, RedisModuleString *str);
+RedisModuleString* REDISMODULE_API_FUNC(RedisModule_HoldString)(RedisModuleCtx *ctx, RedisModuleString *str);
 int REDISMODULE_API_FUNC(RedisModule_StringCompare)(RedisModuleString *a, RedisModuleString *b);
 RedisModuleCtx *REDISMODULE_API_FUNC(RedisModule_GetContextFromIO)(RedisModuleIO *io);
 const RedisModuleString *REDISMODULE_API_FUNC(RedisModule_GetKeyNameFromIO)(RedisModuleIO *io);
@@ -807,6 +808,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(LatencyAddSample);
     REDISMODULE_GET_API(StringAppendBuffer);
     REDISMODULE_GET_API(RetainString);
+    REDISMODULE_GET_API(HoldString);
     REDISMODULE_GET_API(StringCompare);
     REDISMODULE_GET_API(GetContextFromIO);
     REDISMODULE_GET_API(GetKeyNameFromIO);

tests/modules/keyspace_events.c

@@ -48,7 +48,7 @@ static int KeySpace_Notification(RedisModuleCtx *ctx, int type, const char *even
         int nokey;
         RedisModule_DictGetC(loaded_event_log, (void*)keyName, strlen(keyName), &nokey);
         if(nokey){
-            RedisModule_DictSetC(loaded_event_log, (void*)keyName, strlen(keyName), NULL);
+            RedisModule_DictSetC(loaded_event_log, (void*)keyName, strlen(keyName), RedisModule_HoldString(ctx, key));
         }
     }
 
@@ -63,9 +63,15 @@ static int cmdIsKeyLoaded(RedisModuleCtx *ctx, RedisModuleString **argv, int arg
     const char* key  = RedisModule_StringPtrLen(argv[1], NULL);
 
     int nokey;
-    RedisModule_DictGetC(loaded_event_log, (void*)key, strlen(key), &nokey);
+    RedisModuleString* keyStr = RedisModule_DictGetC(loaded_event_log, (void*)key, strlen(key), &nokey);
 
+    RedisModule_ReplyWithArray(ctx, 2);
     RedisModule_ReplyWithLongLong(ctx, !nokey);
+    if(nokey){
+        RedisModule_ReplyWithNull(ctx);
+    }else{
+        RedisModule_ReplyWithString(ctx, keyStr);
+    }
     return REDISMODULE_OK;
 }
 
@@ -93,6 +99,13 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 }
 
 int RedisModule_OnUnload(RedisModuleCtx *ctx) {
+    RedisModuleDictIter *iter = RedisModule_DictIteratorStartC(loaded_event_log, "^", NULL, 0);
+    char* key;
+    size_t keyLen;
+    RedisModuleString* val;
+    while((key = RedisModule_DictNextC(iter, &keyLen, (void**)&val))){
+        RedisModule_FreeString(ctx, val);
+    }
     RedisModule_FreeDict(ctx, loaded_event_log);
     loaded_event_log = NULL;
     return REDISMODULE_OK;

tests/unit/moduleapi/keyspace_events.tcl

@@ -11,12 +11,12 @@ tags "modules" {
             r zadd t 1 f1 2 f2
             r xadd s * f v
             r debug reload
-            assert_equal 1 [r keyspace.is_key_loaded x]
+            assert_equal {1 x} [r keyspace.is_key_loaded x]
-            assert_equal 1 [r keyspace.is_key_loaded y]
+            assert_equal {1 y} [r keyspace.is_key_loaded y]
-            assert_equal 1 [r keyspace.is_key_loaded z]
+            assert_equal {1 z} [r keyspace.is_key_loaded z]
-            assert_equal 1 [r keyspace.is_key_loaded p]
+            assert_equal {1 p} [r keyspace.is_key_loaded p]
-            assert_equal 1 [r keyspace.is_key_loaded t]
+            assert_equal {1 t} [r keyspace.is_key_loaded t]
-            assert_equal 1 [r keyspace.is_key_loaded s]
+            assert_equal {1 s} [r keyspace.is_key_loaded s]
         }
 	}
 }