From f885e364ba9b9fe83b9b94e665547d70040ee89c Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 22 Nov 2020 14:00:51 +0200 Subject: [PATCH] Fix bug with module GIL being released prematurely (#8061) This is hopefully usually harmles. The server.ready_keys will usually be empty so the code after releasing the GIL will soon be done. The only case where it'll actually process things is when a module releases a client (or module) blocked on a key, by triggering this NOT from within a command (e.g. a timer event). This bug was introduced in redis 6.0.9, see #7903 (cherry picked from commit e6fa47380a5274119ed37c7a5ea7455d4b7dbdcc) --- src/server.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/server.c b/src/server.c index 09652294d..1f29c1d93 100644 --- a/src/server.c +++ b/src/server.c @@ -2199,15 +2199,17 @@ void beforeSleep(struct aeEventLoop *eventLoop) { /* Close clients that need to be closed asynchronous */ freeClientsInAsyncFreeQueue(); + /* Try to process blocked clients every once in while. Example: A module + * calls RM_SignalKeyAsReady from within a timer callback (So we don't + * visit processCommand() at all). */ + handleClientsBlockedOnKeys(); + /* Before we are going to sleep, let the threads access the dataset by * releasing the GIL. Redis main thread will not touch anything at this * time. */ if (moduleCount()) moduleReleaseGIL(); - /* Try to process blocked clients every once in while. Example: A module - * calls RM_SignalKeyAsReady from within a timer callback (So we don't - * visit processCommand() at all). */ - handleClientsBlockedOnKeys(); + /* Do NOT add anything below moduleReleaseGIL !!! */ } /* This function is called immediately after the event loop multiplexing @@ -2216,6 +2218,9 @@ void beforeSleep(struct aeEventLoop *eventLoop) { 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. */ if (!ProcessingEventsWhileBlocked) { if (moduleCount()) moduleAcquireGIL(); }