]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
multi-tenant: fix loader dead lock 10383/head
authorVictor Julien <vjulien@oisf.net>
Sun, 11 Feb 2024 08:29:38 +0000 (09:29 +0100)
committerVictor Julien <victor@inliniac.net>
Mon, 12 Feb 2024 11:45:31 +0000 (12:45 +0100)
A dead lock could occur at start up, where a loader thread would
get stuck on it's condition variable, while the main thread was
polling the loaders task results.

The vector to the dead lock is as follows:

main                         loader
DetectEngineMultiTenantSetup
-DetectLoaderSetupLoadTenant
--DetectLoaderQueueTask
---lock loader
---add task
---unlock loader
                        lock loader
                        check/exec tasks
                        unlock loader
---wake up threads
                        lock ctrl mutx
                        cond wait ctrl
                        unlock ctrl
-DetectLoadersSync
--lock loader
--check tasks
--unlock loader

Between the main thread unlocking the loader and waking up the
threads, it is possible that the loader has already moved ahead
but not yet entered its conditional wait. The main thread sends
its condition signal, but since the loader isn't yet waiting on
it the signal is ignored. Then when the loader does enter its
conditional wait, the signal is not sent again.

This patch updates the logic to send signals much more often.
It also makes sure that the signal is sent under lock, as the
API requires.

Bug: #6766.

src/detect-engine-loader.c
src/detect-engine-loader.h

index 1f5363c6f80c02dde46aa3f39535eef1aaadbccb..9073c1e9c29dd2a7586e554c8470da307884f231 100644 (file)
@@ -469,6 +469,12 @@ int DetectLoadersSync(void)
                 done = true;
             }
             SCMutexUnlock(&loader->m);
+            if (!done) {
+                /* nudge thread in case it's sleeping */
+                SCCtrlMutexLock(loader->tv->ctrl_mutex);
+                pthread_cond_broadcast(loader->tv->ctrl_cond);
+                SCCtrlMutexUnlock(loader->tv->ctrl_mutex);
+            }
         }
         SCMutexLock(&loader->m);
         if (loader->result != 0) {
@@ -524,7 +530,9 @@ static void TmThreadWakeupDetectLoaderThreads(void)
         while (tv != NULL) {
             if (strncmp(tv->name,"DL#",3) == 0) {
                 BUG_ON(tv->ctrl_cond == NULL);
+                SCCtrlMutexLock(tv->ctrl_mutex);
                 pthread_cond_broadcast(tv->ctrl_cond);
+                SCCtrlMutexUnlock(tv->ctrl_mutex);
             }
             tv = tv->next;
         }
@@ -568,6 +576,11 @@ static TmEcode DetectLoaderThreadInit(ThreadVars *t, const void *initdata, void
     /* pass thread data back to caller */
     *data = ftd;
 
+    DetectLoaderControl *loader = &loaders[ftd->instance];
+    SCMutexLock(&loader->m);
+    loader->tv = t;
+    SCMutexUnlock(&loader->m);
+
     return TM_ECODE_OK;
 }
 
index 7ffb8c8648a08c8cb4ebf0edaf7db7fc32de4a74..8a6f7b8f17be77d8830627ac4ad1ab8391f6a384 100644 (file)
@@ -44,6 +44,7 @@ typedef struct DetectLoaderTask_ {
 typedef struct DetectLoaderControl_ {
     int id;
     int result;     /* 0 for ok, error otherwise */
+    ThreadVars *tv; /* loader threads threadvars - for waking them up */
     SCMutex m;
     TAILQ_HEAD(, DetectLoaderTask_) task_list;
 } DetectLoaderControl;