diff --git a/src/module.cpp b/src/module.cpp index 859236e72..f6702d1d0 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -4895,14 +4895,17 @@ void RM_FreeThreadSafeContext(RedisModuleCtx *ctx) { zfree(ctx); } +static 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); - moduleAcquireGIL(FALSE /*fServerThread*/); - if (serverTL == nullptr) + if (serverTL == nullptr) { serverTL = &g_pserver->rgthreadvar[IDX_EVENT_LOOP_MAIN]; // arbitrary module threads get the main thread context + g_fModuleThread = true; + } + moduleAcquireGIL(FALSE /*fServerThread*/); } /* Release the server lock after a thread safe API call was executed. */ @@ -4911,10 +4914,20 @@ void RM_ThreadSafeContextUnlock(RedisModuleCtx *ctx) { moduleReleaseGIL(FALSE /*fServerThread*/); } +// A module may be triggered synchronously in a non-module context. In this scenario we don't lock again +// as the server thread acquisition is sufficient. If we did try to lock we would deadlock +static bool FModuleCallBackLock(bool fServerThread) +{ + return !fServerThread && aeThreadOwnsLock() && !g_fModuleThread && s_cAcquisitionsServer > 0; +} void moduleAcquireGIL(int fServerThread) { std::unique_lock lock(s_mutex); int *pcheck = fServerThread ? &s_cAcquisitionsModule : &s_cAcquisitionsServer; + if (FModuleCallBackLock(fServerThread)) { + return; + } + while (*pcheck > 0) s_cv.wait(lock); @@ -4932,6 +4945,10 @@ void moduleAcquireGIL(int fServerThread) { void moduleReleaseGIL(int fServerThread) { std::unique_lock lock(s_mutex); + if (FModuleCallBackLock(fServerThread)) { + return; + } + if (fServerThread) { --s_cAcquisitionsServer; diff --git a/tests/modules/hooks.c b/tests/modules/hooks.c index 665a20481..ff08c3d36 100644 --- a/tests/modules/hooks.c +++ b/tests/modules/hooks.c @@ -30,6 +30,7 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#define REDISMODULE_EXPERIMENTAL_API 1 #include "redismodule.h" #include #include @@ -143,10 +144,12 @@ void flushdbCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void { REDISMODULE_NOT_USED(e); + RedisModule_ThreadSafeContextLock(ctx); RedisModuleFlushInfo *fi = data; char *keyname = (sub == REDISMODULE_SUBEVENT_FLUSHDB_START) ? "flush-start" : "flush-end"; LogNumericEvent(ctx, keyname, fi->dbnum); + RedisModule_ThreadSafeContextUnlock(ctx); } void roleChangeCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void *data)