gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prevent deadlock in RM_ThreadSafeContextLock() when triggered as part of a module callback in a server thread

Browse Source 
Former-commit-id: e01312642be3cc78e7b383dee958a9b5c0ffc103
This commit is contained in:
John Sully 2020-07-12 18:17:53 +00:00
parent cd08792df7
commit efc8e719f9
2 changed files with 22 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/module.cpp
View File

@ -4895,14 +4895,17 @@ void RM_FreeThreadSafeContext(RedisModuleCtx *ctx) {
zfree(ctx); zfree(ctx);
} }
static bool g_fModuleThread = false;
/* Acquire the server lock before executing a thread safe API call. /* Acquire the server lock before executing a thread safe API call.
* This is not needed for `RedisModule_Reply*` calls when there is * This is not needed for `RedisModule_Reply*` calls when there is
* a blocked client connected to the thread safe context. */ * a blocked client connected to the thread safe context. */
void RM_ThreadSafeContextLock(RedisModuleCtx *ctx) { void RM_ThreadSafeContextLock(RedisModuleCtx *ctx) {
UNUSED(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 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. */ /* Release the server lock after a thread safe API call was executed. */
@ -4911,10 +4914,20 @@ void RM_ThreadSafeContextUnlock(RedisModuleCtx *ctx) {
moduleReleaseGIL(FALSE /*fServerThread*/); 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) { void moduleAcquireGIL(int fServerThread) {
std::unique_lock<std::mutex> lock(s_mutex); std::unique_lock<std::mutex> lock(s_mutex);
int *pcheck = fServerThread ? &s_cAcquisitionsModule : &s_cAcquisitionsServer; int *pcheck = fServerThread ? &s_cAcquisitionsModule : &s_cAcquisitionsServer;
if (FModuleCallBackLock(fServerThread)) {
return;
}
while (*pcheck > 0) while (*pcheck > 0)
s_cv.wait(lock); s_cv.wait(lock);
@ -4932,6 +4945,10 @@ void moduleAcquireGIL(int fServerThread) {
void moduleReleaseGIL(int fServerThread) { void moduleReleaseGIL(int fServerThread) {
std::unique_lock<std::mutex> lock(s_mutex); std::unique_lock<std::mutex> lock(s_mutex);
if (FModuleCallBackLock(fServerThread)) {
return;
}
if (fServerThread) if (fServerThread)
{ {
--s_cAcquisitionsServer; --s_cAcquisitionsServer;

3
tests/modules/hooks.c
View File

@ -30,6 +30,7 @@
* POSSIBILITY OF SUCH DAMAGE. * POSSIBILITY OF SUCH DAMAGE.
*/ */
#define REDISMODULE_EXPERIMENTAL_API 1
#include "redismodule.h" #include "redismodule.h"
#include <stdio.h> #include <stdio.h>
#include <string.h> #include <string.h>
@ -143,10 +144,12 @@ void flushdbCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void
{ {
REDISMODULE_NOT_USED(e); REDISMODULE_NOT_USED(e);
RedisModule_ThreadSafeContextLock(ctx);
RedisModuleFlushInfo *fi = data; RedisModuleFlushInfo *fi = data;
char *keyname = (sub == REDISMODULE_SUBEVENT_FLUSHDB_START) ? char *keyname = (sub == REDISMODULE_SUBEVENT_FLUSHDB_START) ?
"flush-start" : "flush-end"; "flush-start" : "flush-end";
LogNumericEvent(ctx, keyname, fi->dbnum); LogNumericEvent(ctx, keyname, fi->dbnum);
RedisModule_ThreadSafeContextUnlock(ctx);
} }
void roleChangeCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void *data) void roleChangeCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void *data)