From d2670b490ddee5ad2cebab5fd22912a98fca83a6 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 29 Mar 2023 11:37:02 -0400 Subject: [PATCH] freebsd_network: Handle an edge-case race of callout_stop. If callout_stop() runs at the same time the callout_thread is about to process the callout, we can wind up with a situation where the callout is "active" but has not yet run due to waiting on the mutex. FreeBSD's documentation confirms that callout_stop must be called (when "safe" is 0, i.e. not callout_drain) with the callout's lock held, if it has one, and so we can prevent the callout from being invoked at this point, too. Additionally, fix return values of callout_reset. May further help with #18315. --- src/libs/compat/freebsd_network/callout.cpp | 28 +++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/libs/compat/freebsd_network/callout.cpp b/src/libs/compat/freebsd_network/callout.cpp index 5f6851a2e3..ec7eb7a033 100644 --- a/src/libs/compat/freebsd_network/callout.cpp +++ b/src/libs/compat/freebsd_network/callout.cpp @@ -57,20 +57,29 @@ callout_thread(void* /*data*/) // execute timer list_remove_item(&sTimers, c); - c->c_due = -1; + if (mutex == NULL) + c->c_due = -1; sCurrentCallout = c; mutex_unlock(&sLock); - if (mutex != NULL) + if (mutex != NULL) { mtx_lock(mutex); + if (c->c_due < 0) { + mtx_unlock(mutex); + goto done; + } + c->c_due = -1; + } + c->c_func(c->c_arg); if (mutex != NULL && (c->c_flags & CALLOUT_RETURNUNLOCKED) == 0) mtx_unlock(mutex); + done: if ((status = mutex_lock(&sLock)) != B_OK) continue; @@ -176,7 +185,7 @@ callout_init_mtx(struct callout *c, struct mtx *mtx, int flags) int callout_reset(struct callout *c, int _ticks, void (*func)(void *), void *arg) { - int canceled = callout_stop(c); + int cancelled = callout_stop(c); MutexLocker locker(sLock); @@ -197,7 +206,7 @@ callout_reset(struct callout *c, int _ticks, void (*func)(void *), void *arg) release_sem(sWaitSem); } - return canceled; + return (cancelled == -1) ? 0 : 1; } @@ -220,13 +229,22 @@ _callout_stop_safe(struct callout *c, int safe) int ret = -1; if (callout_active(c)) { + ret = 0; + if (!safe && c->c_mtx != NULL && c->c_due > 0) { + mtx_assert(c->c_mtx, MA_OWNED); + + // The callout is active, but c_due > 0 and we hold the locks: this + // means the callout thread has dequeued it and is waiting for c_mtx. + // Clear c_due to signal the callout thread. + c->c_due = -1; + ret = 1; + } if (safe) { locker.Unlock(); while (callout_active(c)) snooze(100); locker.Lock(); } - ret = 0; } if (c->c_due <= 0)