threading: don't pass locked flow between threads

Previously the flow manager would share evicted flows with the workers
while keeping the flows mutex locked. This reduced the number of unlock/
lock cycles while there was guaranteed to be no contention.

This turns out to be undefined behavior. A lock is supposed to be locked
and unlocked from the same thread. It appears that FreeBSD is stricter on
this than Linux.

This patch addresses the issue by unlocking before handing a flow off
to another thread, and locking again from the new thread.

Issue was reported and largely analyzed by Bill Meeks.

Bug: #4478
pull/6306/head
Victor Julien 3 years ago
parent cf21694ba6
commit 9551cd0535

@ -692,6 +692,7 @@ static inline void MoveToWorkQueue(ThreadVars *tv, FlowLookupStruct *fls,
f->fb = NULL;
f->next = NULL;
FlowQueuePrivateAppendFlow(&fls->work_queue, f);
FLOWLOCK_UNLOCK(f);
} else {
/* implied: TCP but our thread does not own it. So set it
* aside for the Flow Manager to pick it up. */

@ -333,9 +333,9 @@ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCount
FlowForceReassemblyNeedReassembly(f) == 1)
{
FlowForceReassemblyForFlow(f);
FLOWLOCK_UNLOCK(f);
/* flow ownership is passed to the worker thread */
/* flow remains locked */
counters->flows_aside_needs_work++;
continue;
}

@ -401,6 +401,7 @@ static inline void FlowForceReassemblyForHash(void)
RemoveFromHash(f, prev_f);
f->flow_end_flags |= FLOW_END_FLAG_SHUTDOWN;
FlowForceReassemblyForFlow(f);
FLOWLOCK_UNLOCK(f);
f = next_f;
continue;
}

@ -168,6 +168,7 @@ static void CheckWorkQueue(ThreadVars *tv, FlowWorkerThreadData *fw,
{
Flow *f;
while ((f = FlowQueuePrivateGetFromTop(fq)) != NULL) {
FLOWLOCK_WRLOCK(f);
f->flow_end_flags |= FLOW_END_FLAG_TIMEOUT; //TODO emerg
const FlowStateType state = f->flow_state;

Loading…
Cancel
Save