From 17c0ffaddc246f09f6a58b132bfcd1750417f5c7 Mon Sep 17 00:00:00 2001 From: "Antonio S. Musumeci" Date: Sat, 18 Apr 2026 17:47:49 -0400 Subject: [PATCH] thread_pool: remove dead code and fix stale comment _remove_self and _try_claim_removal had no callers. The CAS loop from _try_claim_removal was inlined into _try_claim_and_remove_self when that function was introduced (7ce5364c extracted it, later commits merged it back under combined lock acquisition), and _remove_self was superseded by _try_remove_self_if_above / _try_claim_and_remove_self. The destructor comment referencing _remove_self pointed at a race that is now closed by those two functions instead. --- vendored/libfuse/include/thread_pool.hpp | 49 +++--------------------- 1 file changed, 5 insertions(+), 44 deletions(-) diff --git a/vendored/libfuse/include/thread_pool.hpp b/vendored/libfuse/include/thread_pool.hpp index 1f8a7302..4c894313 100644 --- a/vendored/libfuse/include/thread_pool.hpp +++ b/vendored/libfuse/include/thread_pool.hpp @@ -93,9 +93,7 @@ private: WorkerAction _process_func(Func &func_); int _add_thread_scaling_locked(const std::string &name_); int _remove_thread_scaling_locked(); - void _remove_self(pthread_t t_); bool _try_remove_self_if_above(pthread_t t_, std::size_t min_count_); - bool _try_claim_removal(); bool _try_claim_and_remove_self(pthread_t t_); void _maybe_grow_on_pressure(); static std::uint64_t _now_usecs(); @@ -453,8 +451,9 @@ ThreadPool::~ThreadPool() // we snapshot. Without _scaling_mutex a late push_back could // land after the snapshot, leaving the new thread unjoined and // accessing a destroyed pool. - // _threads_mutex closes the race with _remove_self, which only - // needs _threads_mutex — after _stop is set it becomes a no-op. + // _threads_mutex closes the race with _try_remove_self_if_above and + // _try_claim_and_remove_self — both check _stop under this lock, so + // after _stop is set they become no-ops and leave us authoritative. std::vector snapshot; { mutex_lockguard(_scaling_mutex); @@ -859,27 +858,9 @@ ThreadPool::monitor_routine(void *arg_) } -// Try to claim one pending removal. Returns true if a _remove_count -// decrement was successfully claimed, false if the count was zero. -inline -bool -ThreadPool::_try_claim_removal() -{ - std::size_t count = _remove_count.load(std::memory_order_relaxed); - while(count > 0) - { - if(_remove_count.compare_exchange_weak(count,count - 1, - std::memory_order_acq_rel)) - return true; - } - return false; -} - - // Atomically claim a pending removal AND remove self from the thread -// list under both _scaling_mutex and _threads_mutex. This closes the -// TOCTOU race between the old _try_claim_removal + _remove_self -// two-step: without atomicity, set_threads can observe the decremented +// list under both _scaling_mutex and _threads_mutex. Atomicity closes +// a TOCTOU race: without it, set_threads could observe the decremented // _remove_count before _threads is shrunk, miscalculate effective // count, and issue excess removals that drain the pool to zero. inline @@ -931,26 +912,6 @@ claimed: } -inline -void -ThreadPool::_remove_self(pthread_t t_) -{ - mutex_lockguard(_threads_mutex); - - // Check _stop under the lock so the destructor can't snapshot this - // thread for joining and then race with us detaching ourselves. - if(_stop.load(std::memory_order_acquire)) - return; - - auto it = std::find(_threads.begin(),_threads.end(),t_); - if(it != _threads.end()) - { - _threads.erase(it); - pthread_detach(t_); - } -} - - // Atomically check thread count and remove self if above min_count_. // Returns true if removed, false if at or below threshold. //