multi-tenant: fix loader dead lock

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: #6767.

Co-authored-by: Shivani Bhardwaj <shivani@oisf.net>

diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c
index f83dbc076f1f78e8e4843dff696b0760a0fc7b54..3557d2c95e46c3055c2c2964e4820e285d206b7c 100644 (file)
--- a/src/detect-engine-loader.c
+++ b/src/detect-engine-loader.c
@@ -430,6 +430,12 @@ int DetectLoadersSync(void)
                 done = 1;
             }
             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) {
@@ -492,7 +498,9 @@ 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;
         }
@@ -544,6 +552,9 @@ static TmEcode DetectLoaderThreadInit(ThreadVars *t, const void *initdata, void
     /* pass thread data back to caller */
     *data = ftd;
 
+    DetectLoaderControl *loader = &loaders[ftd->instance];
+    loader->tv = t;
+
     return TM_ECODE_OK;
 }
 

diff --git a/src/detect-engine-loader.h b/src/detect-engine-loader.h
index 6b8aa5cf9e0542343faa807661a41187705157e4..65f255dd2a7fb75c4971ac0d9303d458a2961218 100644 (file)
--- a/src/detect-engine-loader.h
+++ b/src/detect-engine-loader.h
@@ -41,9 +41,14 @@ typedef struct DetectLoaderTask_ {
 
 typedef struct DetectLoaderControl_ {
     int id;
-    int result;     /* 0 for ok, error otherwise */
-    SCMutex m;
-    TAILQ_HEAD(, DetectLoaderTask_) task_list;
+    ThreadVars *tv; /**< loader threads threadvars - for waking them up */
+
+    /** struct to group members and mutex */
+    struct {
+        SCMutex m;  /**< mutex protects result and task_list */
+        int result; /**< 0 for ok, error otherwise */
+        TAILQ_HEAD(, DetectLoaderTask_) task_list;
+    };
 } DetectLoaderControl;
 
 int DetectLoaderQueueTask(int loader_id, LoaderFunc Func, void *func_ctx);