Free resources when 'nget.noatomic' thread terminates

Call 'pthread_detach' from newly created thread.

Update unit tests to check that 'pthread_detach'
is called.

Update redismodule test container base image to '2-rmr1.13.1'
because '1-rmr1.13.1' no longer exists.

Previously the redismodule command 'nget.noatomic'
started a thread but the started thread was never
cleaned up because by default pthread cleans up
threads only when 'pthread_join' is called.
In case of 'nget.noatomic' the thread from which
'pthread_create' is called terminates immediately
and 'pthread_join' is never called resulting in
a resource leak.
We expect the created thread to be joined.
Therefore we need to call 'pthread_detach' from the
newly created thread which signals to pthread that
the resources allocated by the newly created thread
can be deallocated when the thread terminates.

Signed-off-by: Heinonen Arvo <arvo.heinonen@nokia.com>
Change-Id: If4d076fc4d58f65896638e99948c2d91a0000a9b

diff --git a/docker/Dockerfile.redis b/docker/Dockerfile.redis
index 459f583..f3fe3fb 100644 (file)
--- a/docker/Dockerfile.redis
+++ b/docker/Dockerfile.redis
@@ -56,7 +56,7 @@ RUN ./autogen.sh && \
     make test
 
 
-FROM nexus3.o-ran-sc.org:10004/bldr-alpine3-go:1-rmr1.13.1 as build-env
+FROM nexus3.o-ran-sc.org:10004/bldr-alpine3-go:2-rmr1.13.1 as build-env
 
 RUN apk add cpputest
 COPY ./redismodule /redismodule

diff --git a/redismodule/src/exstrings.c b/redismodule/src/exstrings.c
index 4211f56..11102d3 100755 (executable)
--- a/redismodule/src/exstrings.c
+++ b/redismodule/src/exstrings.c
@@ -774,6 +774,8 @@ int Nget_RedisCommand(RedisModuleCtx *ctx, NgetArgs* nget_args, bool using_threa
  */
 void *NGet_NoAtomic_ThreadMain(void *arg)
 {
+    pthread_detach(pthread_self());
+
     RedisModuleBlockedClientArgs *bca = arg;
     RedisModuleBlockedClient *bc = bca->bc;
     RedisModuleCtx *ctx = RedisModule_GetThreadSafeContext(bc);

diff --git a/redismodule/tst/mock/include/commonStub.h b/redismodule/tst/mock/include/commonStub.h
index dc87fa7..e57de1b 100644 (file)
--- a/redismodule/tst/mock/include/commonStub.h
+++ b/redismodule/tst/mock/include/commonStub.h
@@ -25,8 +25,13 @@
 
 #include <pthread.h>
 
+#define UT_DUMMY_THREAD_ID 1234
+
 int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
                    void *(*start_routine) (void *), void *arg);
 
+int pthread_detach(pthread_t thread);
+
+pthread_t pthread_self(void);
 
 #endif

diff --git a/redismodule/tst/mock/src/commonStub.cpp b/redismodule/tst/mock/src/commonStub.cpp
index 2df1df9..5e9a0f1 100644 (file)
--- a/redismodule/tst/mock/src/commonStub.cpp
+++ b/redismodule/tst/mock/src/commonStub.cpp
@@ -58,3 +58,19 @@ int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
         .actualCall("pthread_create")
         .returnIntValueOrDefault(0);
 }
+
+int pthread_detach(pthread_t thread)
+{
+    (void)thread;
+
+    return mock()
+        .actualCall("pthread_detach")
+        .returnIntValueOrDefault(0);
+}
+
+pthread_t pthread_self(void)
+{
+    return mock()
+        .actualCall("pthread_self")
+        .returnIntValueOrDefault(UT_DUMMY_THREAD_ID);
+}

diff --git a/redismodule/tst/src/exstrings_nget_test.cpp b/redismodule/tst/src/exstrings_nget_test.cpp
index 103aa6e..1944876 100644 (file)
--- a/redismodule/tst/src/exstrings_nget_test.cpp
+++ b/redismodule/tst/src/exstrings_nget_test.cpp
@@ -47,6 +47,12 @@ TEST_GROUP(exstrings_nget)
 
 };
 
+void threadDetachedSuccess()
+{
+    mock().expectOneCall("pthread_detach")
+        .andReturnValue(0);
+}
+
 void nKeysFoundMget(long keys)
 {
     for (long i = 0 ; i < keys ; i++) {
@@ -399,6 +405,7 @@ TEST(exstrings_nget, nget_noatomic_threadmain_3_keys_scanned_3_keys_mget)
     bca->argc = 2;
 
     mock().ignoreOtherCalls();
+    threadDetachedSuccess();
     mock().expectOneCall("RedisModule_ReplyWithArray")
           .withParameter("len", (long)REDISMODULE_POSTPONED_ARRAY_LEN);
     mock().expectOneCall("RedisModule_Call")
@@ -433,6 +440,7 @@ TEST(exstrings_nget, nget_noatomic_threadmain_3_keys_scanned_0_keys_mget)
     bca->argc = 2;
 
     mock().ignoreOtherCalls();
+    threadDetachedSuccess();
     mock().expectOneCall("RedisModule_ReplyWithArray")
           .withParameter("len", (long)REDISMODULE_POSTPONED_ARRAY_LEN);
     mock().expectOneCall("RedisModule_Call")
@@ -467,6 +475,7 @@ TEST(exstrings_nget, nget_noatomic_threadmain_3_keys_scanned_2_keys_mget)
     bca->argc = 2;
 
     mock().ignoreOtherCalls();
+    threadDetachedSuccess();
     mock().expectOneCall("RedisModule_ReplyWithArray")
           .withParameter("len", (long)REDISMODULE_POSTPONED_ARRAY_LEN);
     mock().expectOneCall("RedisModule_Call")
@@ -502,6 +511,7 @@ TEST(exstrings_nget, nget_noatomic_threadmain_scan_returned_zero_keys)
     bca->argc = 2;
 
     mock().ignoreOtherCalls();
+    threadDetachedSuccess();
     mock().expectOneCall("RedisModule_ReplyWithArray")
           .withParameter("len", (long)REDISMODULE_POSTPONED_ARRAY_LEN);
     mock().expectOneCall("RedisModule_Call")
@@ -519,3 +529,24 @@ TEST(exstrings_nget, nget_noatomic_threadmain_scan_returned_zero_keys)
 
     delete []redisStrVec;
 }
+
+TEST(exstrings_nget, nget_noatomic_threadmain_thread_detached)
+{
+    RedisModuleCtx ctx;
+    RedisModuleBlockedClientArgs *bca = (RedisModuleBlockedClientArgs*)malloc(sizeof(RedisModuleBlockedClientArgs));
+    RedisModuleBlockedClient *bc = RedisModule_BlockClient(&ctx,NULL,NULL,NULL,0);
+    RedisModuleString ** redisStrVec = createRedisStrVec(2);
+
+    bca->bc = bc;
+    bca->argv = redisStrVec;
+    bca->argc = 2;
+
+    mock().ignoreOtherCalls();
+    threadDetachedSuccess();
+
+    NGet_NoAtomic_ThreadMain((void*)bca);
+
+    mock().checkExpectations();
+
+    delete []redisStrVec;
+}