From 239bb9cf654ecf9c8c3ccdb7fd4d00eb146595f2 Mon Sep 17 00:00:00 2001 From: VivekSainiEQ Date: Mon, 29 Mar 2021 18:46:06 +0000 Subject: [PATCH] Added logic back to only acquire/release GIL if modules are enabled, without causing deadlocks Former-commit-id: 9ab36ddc36e1d12e41d2eca917ee24a44a82df52 --- src/server.cpp | 12 ++++++++---- src/server.h | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/server.cpp b/src/server.cpp index 48b0cf89d..2f7fdb704 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2401,8 +2401,10 @@ void beforeSleep(struct aeEventLoop *eventLoop) { locker.disarm(); if (!fSentReplies) handleClientsWithPendingWrites(iel, aof_state); - moduleReleaseGIL(TRUE /*fServerThread*/); - + /* Determine whether the modules are enabled before sleeping, and use that result + both here, and after wakeup to avoid double acquire or release of the GIL */ + serverTL->modulesEnabledThisAeLoop = !!moduleCount(); + if (serverTL->modulesEnabledThisAeLoop) moduleReleaseGIL(TRUE /*fServerThread*/); /* Do NOT add anything below moduleReleaseGIL !!! */ } @@ -2413,8 +2415,10 @@ void afterSleep(struct aeEventLoop *eventLoop) { UNUSED(eventLoop); /* Do NOT add anything above moduleAcquireGIL !!! */ - /* Aquire the modules GIL so that their threads won't touch anything. */ - moduleAcquireGIL(TRUE /*fServerThread*/); + /* Aquire the modules GIL so that their threads won't touch anything. + Don't check here that modules are enabled, rather use the result from beforeSleep + Otherwise you may double acquire the GIL and cause deadlocks in the module */ + if (serverTL->modulesEnabledThisAeLoop) moduleAcquireGIL(TRUE /*fServerThread*/); } /* =========================== Server initialization ======================== */ diff --git a/src/server.h b/src/server.h index bc3b62fb4..a0ee1bfc0 100644 --- a/src/server.h +++ b/src/server.h @@ -1389,6 +1389,8 @@ struct redisServerThreadVars { char neterr[ANET_ERR_LEN]; /* Error buffer for anet.c */ long unsigned commandsExecuted = 0; bool fRetrySetAofEvent = false; + bool modulesEnabledThisAeLoop = false; /* In this loop of aeMain, were modules enabled before + the thread went to sleep? */ std::vector vecclientsProcess; dictAsyncRehashCtl *rehashCtl = nullptr; };