From dbb7c5e785ced3002d02f8c8e25ae86e0556b976 Mon Sep 17 00:00:00 2001 From: "Antonio S. Musumeci" Date: Fri, 10 Apr 2026 16:44:53 -0400 Subject: [PATCH] thread_pool: close TOCTOU race in _remove_thread_scaling_locked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- vendored/libfuse/include/thread_pool.hpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/vendored/libfuse/include/thread_pool.hpp b/vendored/libfuse/include/thread_pool.hpp index 8b9ba024..d3c1639d 100644 --- a/vendored/libfuse/include/thread_pool.hpp +++ b/vendored/libfuse/include/thread_pool.hpp @@ -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;