multi-tenant: fix coverity warning #10395
Conversation
Rework locking logic to avoid the following coverity warning.
** CID 1591966: Concurrent data access violations (MISSING_LOCK)
/src/detect-engine-loader.c: 475 in DetectLoadersSync()
474 SCCtrlMutexLock(loader->tv->ctrl_mutex);
>>> CID 1591966: Concurrent data access violations (MISSING_LOCK)
>>> Accessing "loader->tv" without holding lock "DetectLoaderControl_.m". Elsewhere, "DetectLoaderControl_.tv" is written to with "DetectLoaderControl_.m" held 1 out of 1 times (1 of these accesses strongly imply that it is necessary).
475 pthread_cond_broadcast(loader->tv->ctrl_cond);
476 SCCtrlMutexUnlock(loader->tv->ctrl_mutex);
The warning is harmless.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10395 +/- ##
=======================================
Coverage 82.52% 82.52%
=======================================
Files 978 978
Lines 272158 272158
=======================================
+ Hits 224591 224598 +7
+ Misses 47567 47560 -7
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Coverity confirmed to be happy. |
|
Information: ERROR: QA failed on SURI_TLPW2_autofp_suri_time.
Pipeline 18398 |
| @@ -44,9 +44,9 @@ 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; | |||
| SCMutex m; /**< mutex protectx result and task_list */ | |||
There was a problem hiding this comment.
minor typo: protectx
Q: Why did you do this reordering of struct items? To have the cacheline boundary fall outside of task_list?
There was a problem hiding this comment.
It's not a perf critical struct, so I reordered things to have the mutex and the members it protects grouped together
There was a problem hiding this comment.
ok. Thank you. This PR looks good to me. If you're fixing the typo in comment, I'll wait for the next PR to approve.
|
replaced by #10412 |
Rework locking logic to avoid the following coverity warning.
** CID 1591966: Concurrent data access violations (MISSING_LOCK)
/src/detect-engine-loader.c: 475 in DetectLoadersSync()
The warning is harmless.