Added logic back to only acquire/release GIL if modules are enabled, without causing deadlocks

Former-commit-id: 9ab36ddc36e1d12e41d2eca917ee24a44a82df52
This commit is contained in:
VivekSainiEQ 2021-03-29 18:46:06 +00:00 committed by John Sully
parent 662d5faa8b
commit 239bb9cf65
2 changed files with 10 additions and 4 deletions

View File

@ -2401,8 +2401,10 @@ void beforeSleep(struct aeEventLoop *eventLoop) {
locker.disarm(); locker.disarm();
if (!fSentReplies) if (!fSentReplies)
handleClientsWithPendingWrites(iel, aof_state); 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 !!! */ /* Do NOT add anything below moduleReleaseGIL !!! */
} }
@ -2413,8 +2415,10 @@ void afterSleep(struct aeEventLoop *eventLoop) {
UNUSED(eventLoop); UNUSED(eventLoop);
/* Do NOT add anything above moduleAcquireGIL !!! */ /* Do NOT add anything above moduleAcquireGIL !!! */
/* Aquire the modules GIL so that their threads won't touch anything. */ /* Aquire the modules GIL so that their threads won't touch anything.
moduleAcquireGIL(TRUE /*fServerThread*/); 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 ======================== */ /* =========================== Server initialization ======================== */

View File

@ -1389,6 +1389,8 @@ struct redisServerThreadVars {
char neterr[ANET_ERR_LEN]; /* Error buffer for anet.c */ char neterr[ANET_ERR_LEN]; /* Error buffer for anet.c */
long unsigned commandsExecuted = 0; long unsigned commandsExecuted = 0;
bool fRetrySetAofEvent = false; bool fRetrySetAofEvent = false;
bool modulesEnabledThisAeLoop = false; /* In this loop of aeMain, were modules enabled before
the thread went to sleep? */
std::vector<client*> vecclientsProcess; std::vector<client*> vecclientsProcess;
dictAsyncRehashCtl *rehashCtl = nullptr; dictAsyncRehashCtl *rehashCtl = nullptr;
}; };