From 68d213f4c9c1c3161929a5e20ca4f2b27665c8fd Mon Sep 17 00:00:00 2001 From: VivekSainiEQ Date: Thu, 18 Feb 2021 23:10:19 +0000 Subject: [PATCH] Removed hasModuleGIL boolean and added fix from PR #292 --- src/module.cpp | 14 ++++++++------ src/server.cpp | 3 +-- src/server.h | 1 - 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/module.cpp b/src/module.cpp index d0f719566..f2450af2e 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -329,6 +329,8 @@ static int s_cAcquisitionsModule = 0; static std::mutex s_mutex; static std::condition_variable s_cv; static std::recursive_mutex s_mutexModule; +static redisServerThreadVars vars; /* Server thread local variables to be used by module threads */ +thread_local bool g_fModuleThread = false; typedef void (*RedisModuleForkDoneHandler) (int exitcode, int bysignal, void *user_data); @@ -4772,6 +4774,10 @@ int moduleClientIsBlockedOnKeys(client *c) { * RedisModule_BlockClientOnKeys() is accessible from the timeout * callback via RM_GetBlockedClientPrivateData). */ int RM_UnblockClient(RedisModuleBlockedClient *bc, void *privdata) { + if (serverTL == nullptr) { + serverTL = &vars; + g_fModuleThread = true; + } if (bc->blocked_on_keys) { /* In theory the user should always pass the timeout handler as an * argument, but better to be safe than sorry. */ @@ -5045,15 +5051,14 @@ void RM_FreeThreadSafeContext(RedisModuleCtx *ctx) { zfree(ctx); } -static redisServerThreadVars vars; -thread_local bool g_fModuleThread = false; + /* Acquire the server lock before executing a thread safe API call. * This is not needed for `RedisModule_Reply*` calls when there is * a blocked client connected to the thread safe context. */ void RM_ThreadSafeContextLock(RedisModuleCtx *ctx) { UNUSED(ctx); if (serverTL == nullptr) { - serverTL = &vars; // arbitrary module threads get the main thread context + serverTL = &vars; g_fModuleThread = true; } moduleAcquireGIL(FALSE /*fServerThread*/, true /*fExclusive*/); @@ -5102,7 +5107,6 @@ void moduleAcquireGIL(int fServerThread, int fExclusive) { if (fServerThread) { ++s_cAcquisitionsServer; - serverTL->hasModuleGIL = true; } else { @@ -5138,7 +5142,6 @@ int moduleTryAcquireGIL(bool fServerThread, int fExclusive) { if (fServerThread) { ++s_cAcquisitionsServer; - serverTL->hasModuleGIL = true; } else { @@ -5163,7 +5166,6 @@ void moduleReleaseGIL(int fServerThread, int fExclusive) { if (fServerThread) { --s_cAcquisitionsServer; - serverTL->hasModuleGIL = false; } else { diff --git a/src/server.cpp b/src/server.cpp index 8ee76b4e5..2134669d0 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -5672,9 +5672,8 @@ void *workerThreadMain(void *parg) catch (ShutdownException) { } + moduleReleaseGIL(true); serverAssert(!GlobalLocksAcquired()); - if (serverTL->hasModuleGIL) - moduleReleaseGIL(true); aeDeleteEventLoop(el); return NULL; diff --git a/src/server.h b/src/server.h index fa43ae160..01c9b90dc 100644 --- a/src/server.h +++ b/src/server.h @@ -1390,7 +1390,6 @@ struct redisServerThreadVars { long unsigned commandsExecuted = 0; bool fRetrySetAofEvent = false; std::vector vecclientsProcess; - bool hasModuleGIL = false; /* Does this thread own the moduleGIL lock? */ }; struct redisMaster {