thread_pool: close TOCTOU race in _remove_thread_scaling_locked

The size check and _remove_count increment were not atomic —
_threads_mutex was released between checking _threads.size() and
incrementing _remove_count.  A concurrent idle self-exit (which
only holds _threads_mutex, not _scaling_mutex) could shrink the
pool between the two operations, allowing _remove_count to signal
a removal that would take the pool below min_allowed threads.

Fix by holding _threads_mutex across both the size check and the
_remove_count increment.  The enqueue_unbounded call is kept
outside the lock since it only needs to wake a worker and does
not need to be atomic with the count check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Antonio S. Musumeci 2026-04-10 16:44:53 -04:00
parent f9e36b3889
commit dbb7c5e785

View file

@ -864,6 +864,10 @@ inline
int
ThreadPool::_remove_thread_scaling_locked()
{
// Hold _threads_mutex across both the size check and the
// _remove_count increment so that a concurrent removal (idle
// self-exit or another _remove_thread call) cannot shrink the
// pool below min_allowed between the check and the signal.
{
mutex_lockguard(_threads_mutex);
std::size_t min_allowed = 1;
@ -871,15 +875,16 @@ ThreadPool::_remove_thread_scaling_locked()
min_allowed = _scaling_config.min_threads;
if(_threads.size() <= min_allowed)
return -EINVAL;
// Increment the atomic counter while still holding the lock.
// A worker will claim the decrement either:
// (a) in the poison-pill handler after dequeuing the wakeup, or
// (b) in the task-completion CAS loop in _process_func.
// In case (b) the wakeup pill becomes stale and is harmlessly
// ignored (the pill handler sees _remove_count == 0 and continues).
_remove_count.fetch_add(1,std::memory_order_relaxed);
}
// Increment the atomic counter and enqueue a wakeup pill.
// A worker will claim the decrement either:
// (a) in the poison-pill handler after dequeuing the wakeup, or
// (b) in the task-completion CAS loop in _process_func.
// In case (b) the wakeup pill becomes stale and is harmlessly
// ignored (the pill handler sees _remove_count == 0 and continues).
_remove_count.fetch_add(1,std::memory_order_relaxed);
_queue.enqueue_unbounded(Func{});
return 0;