[MOD-8115] Free spec resources in the main thread (#5324)

meiravgri · web-flow · commit fc508014c80a · 2024-12-09T14:13:42.000Z
* free spec resources in the main thread. * improve cpp gc tests suite: allow multiple gc runs in each test move args to test suite members use pthread_join instead of pthread_detach loop to enable multiple gc interval taken from #5049
diff --git a/tests/cpptests/common.cpp b/tests/cpptests/common.cpp
@@ -26,6 +26,7 @@ class MyEnvironment : public ::testing::Environment {
     const char *arguments[] = {"NOGC"};
     // No arguments..
     RMCK_Bootstrap(my_OnLoad, arguments, 1);
+    RSGlobalConfig.freeResourcesThread = false;
   }
 
   virtual void TearDown() {
diff --git a/tests/cpptests/index_utils.cpp b/tests/cpptests/index_utils.cpp
@@ -70,10 +70,7 @@ RefManager *createSpec(RedisModuleCtx *ctx) {
 }
 
 void freeSpec(RefManager *ism) {
-    int free_resources_config = RSGlobalConfig.freeResourcesThread;
-    RSGlobalConfig.freeResourcesThread = false;
     IndexSpec_RemoveFromGlobals({ism});
-    RSGlobalConfig.freeResourcesThread = free_resources_config;
 }
 
 NumericRangeTree *getNumericTree(IndexSpec *spec, const char *field) {
diff --git a/tests/cpptests/test_cpp_forkgc.cpp b/tests/cpptests/test_cpp_forkgc.cpp
@@ -47,22 +47,26 @@ static timespec getTimespecCb(void *) {
 typedef struct {
   void *fgc;
   RefManager *ism;
+  volatile bool runGc;
 } args_t;
 
-static pthread_t thread;
-
 void *cbWrapper(void *args) {
   args_t *fgcArgs = (args_t *)args;
-  ForkGC *fgc = reinterpret_cast<ForkGC *>(get_spec(fgcArgs->ism)->gc->gcCtx);
+  GCContext *gc = get_spec(fgcArgs->ism)->gc;
+  ForkGC *fgc = reinterpret_cast<ForkGC *>(gc->gcCtx);
 
-  // sync thread
-  while (fgc->pauseState != FGC_PAUSED_CHILD) {
-    usleep(500);
-  }
+  while (true) {
+    // sync thread
+    while (fgcArgs->runGc && fgc->pauseState != FGC_PAUSED_CHILD) {
+      usleep(500);
+    }
+    if (!fgcArgs->runGc) {
+      break;
+    }
 
-  // run ForkGC
-  get_spec(fgcArgs->ism)->gc->callbacks.periodicCallback(fgc);
-  rm_free(args);
+    // run ForkGC
+    gc->callbacks.periodicCallback(fgc);
+  }
   return NULL;
 }
 
@@ -71,6 +75,8 @@ class FGCTest : public ::testing::Test {
   RMCK::Context ctx;
   RefManager *ism;
   ForkGC *fgc;
+  args_t args;
+  pthread_t thread;
 
   void SetUp() override {
     ism = createSpec(ctx);
@@ -82,17 +88,16 @@ class FGCTest : public ::testing::Test {
   void runGcThread() {
     fgc = reinterpret_cast<ForkGC *>(get_spec(ism)->gc->gcCtx);
     thread = {0};
-    args_t *args = (args_t *)rm_calloc(1, sizeof(*args));
-    *args = {.fgc = fgc, .ism = ism};
+    args = {.fgc = fgc, .ism = ism, .runGc = true};
 
-    pthread_create(&thread, NULL, cbWrapper, args);
+    pthread_create(&thread, NULL, cbWrapper, &args);
   }
 
   void TearDown() override {
+    args.runGc = false;
+    // wait for the gc thread to finish current loop and exit the thread
+    pthread_join(thread, NULL);
     freeSpec(ism);
-    // Detach from the gc to make sure we are not stuck on waiting
-    // for the pauseState to be changed.
-    pthread_detach(thread);
   }
 
   size_t addDocumentWrapper(const char *docid, const char *field, const char *value) {

Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,7 @@ class MyEnvironment : public ::testing::Environment {`
`26`	`26`	`const char *arguments[] = {"NOGC"};`
`27`	`27`	`// No arguments..`
`28`	`28`	`RMCK_Bootstrap(my_OnLoad, arguments, 1);`
	`29`	`+ RSGlobalConfig.freeResourcesThread = false;`
`29`	`30`	`}`
`30`	`31`
`31`	`32`	`virtual void TearDown() {`
Original file line number	Diff line number	Diff line change
`@@ -70,10 +70,7 @@ RefManager createSpec(RedisModuleCtx ctx) {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`void freeSpec(RefManager *ism) {`
`73`		`- int free_resources_config = RSGlobalConfig.freeResourcesThread;`
`74`		`- RSGlobalConfig.freeResourcesThread = false;`
`75`	`73`	`IndexSpec_RemoveFromGlobals({ism});`
`76`		`- RSGlobalConfig.freeResourcesThread = free_resources_config;`
`77`	`74`	`}`
`78`	`75`
`79`	`76`	`NumericRangeTree getNumericTree(IndexSpec spec, const char *field) {`