mirror of
https://github.com/trapexit/mergerfs.git
synced 2026-10-26 19:01:56 -04:00
Assert try_enqueue does not move-from callable on failure
enqueue_work_autoscale relies on BoundedQueue::try_enqueue leaving the callable untouched when the semaphore check fails. This is correct today because tryWait() is evaluated before std::forward moves the item, but the coupling is implicit — a future BoundedQueue change that reorders these operations would silently introduce a use-after- move bug on the blocking fallback path. Add a debug assert after the failed try_enqueue to verify the Func is still valid before using it on the blocking path. Strengthen the comment to explicitly name the invariant being relied upon. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ea47934c63
commit
4d35726fc6
1 changed files with 13 additions and 3 deletions
|
|
@ -24,6 +24,7 @@
|
|||
#include "mutex.hpp"
|
||||
|
||||
#include <algorithm>
|
||||
#include <cassert>
|
||||
#include <atomic>
|
||||
#include <cerrno>
|
||||
#include <csignal>
|
||||
|
|
@ -955,8 +956,11 @@ ThreadPool::enqueue_work_autoscale(ThreadPool::PToken &ptok_,
|
|||
// Materialize into a Func up front so we own the value across
|
||||
// the try/block-enqueue boundary without a double std::forward.
|
||||
//
|
||||
// Safe: BoundedQueue::try_enqueue checks the semaphore before
|
||||
// forwarding, so f is not moved-from on failure.
|
||||
// INVARIANT: BoundedQueue::try_enqueue checks the semaphore
|
||||
// (tryWait) *before* forwarding the item into the inner queue.
|
||||
// On failure the item is never moved-from, so f remains valid.
|
||||
// The assert below guards against future BoundedQueue changes
|
||||
// that could violate this ordering.
|
||||
Func f(std::forward<FuncType>(func_));
|
||||
|
||||
if(_queue.try_enqueue(std::move(f)))
|
||||
|
|
@ -965,6 +969,9 @@ ThreadPool::enqueue_work_autoscale(ThreadPool::PToken &ptok_,
|
|||
return;
|
||||
}
|
||||
|
||||
assert(static_cast<bool>(f) &&
|
||||
"BoundedQueue::try_enqueue moved the callable despite returning false");
|
||||
|
||||
_maybe_grow_on_pressure();
|
||||
|
||||
// Block-enqueue (will succeed once a worker frees a slot)
|
||||
|
|
@ -978,7 +985,7 @@ inline
|
|||
void
|
||||
ThreadPool::enqueue_work_autoscale(FuncType &&func_)
|
||||
{
|
||||
// See ptok_ overload for safety note on try_enqueue + move.
|
||||
// See ptok_ overload for invariant and safety note.
|
||||
Func f(std::forward<FuncType>(func_));
|
||||
|
||||
if(_queue.try_enqueue(std::move(f)))
|
||||
|
|
@ -987,6 +994,9 @@ ThreadPool::enqueue_work_autoscale(FuncType &&func_)
|
|||
return;
|
||||
}
|
||||
|
||||
assert(static_cast<bool>(f) &&
|
||||
"BoundedQueue::try_enqueue moved the callable despite returning false");
|
||||
|
||||
_maybe_grow_on_pressure();
|
||||
|
||||
// Block-enqueue (will succeed once a worker frees a slot)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue