From: Victor Julien Date: Sun, 11 Feb 2024 08:29:38 +0000 (+0100) Subject: multi-tenant: fix loader dead lock X-Git-Tag: suricata-7.0.4~34 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d93b21c5240c00657c350f47f0bbc02c29778ad5;p=thirdparty%2Fsuricata.git 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: #6768. Co-authored-by: Shivani Bhardwaj --- diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index e41f27779f..0cdb453388 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -456,6 +456,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) { @@ -511,7 +517,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; } @@ -555,6 +563,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 7ffb8c8648..f43ff9a549 100644 --- a/src/detect-engine-loader.h +++ b/src/detect-engine-loader.h @@ -43,9 +43,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, LoaderFreeFunc FreeFunc);