From bc3a0ed8d61e72963ef510a03229384b8422d3bf Mon Sep 17 00:00:00 2001 From: "Antonio S. Musumeci" Date: Fri, 10 Apr 2026 16:38:39 -0400 Subject: [PATCH] thread_pool: snapshot min_threads before _try_remove_self_if_above call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In start_routine_autoscale, btp->_scaling_config.min_threads was read as an argument to _try_remove_self_if_above. If that method returns true, the calling thread is detached and removed from _threads — at which point the destructor may run concurrently and destroy the pool object. Although the dereference technically occurs before entering the method (and thus before the detach), it happens before the _threads_mutex lock that serializes with the destructor, making the ordering fragile. Snapshot min_threads into a local variable before the call so the argument evaluation cannot race with pool destruction. This mirrors the existing pattern of copying _name into a local for the post-detach syslog call. Co-Authored-By: Claude Opus 4.6 (1M context) --- vendored/libfuse/include/thread_pool.hpp | 25 ++++++++++++------------ 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/vendored/libfuse/include/thread_pool.hpp b/vendored/libfuse/include/thread_pool.hpp index 45fe6d09..00c915aa 100644 --- a/vendored/libfuse/include/thread_pool.hpp +++ b/vendored/libfuse/include/thread_pool.hpp @@ -505,20 +505,19 @@ ThreadPool::start_routine_autoscale(void *arg_) if(btp->_stop.load(std::memory_order_acquire)) break; - // Copy state needed for the log message BEFORE detaching, - // because _try_remove_self_if_above detaches the thread and - // the destructor won't wait for us — btp may be destroyed - // by the time we reach the syslog call. + // Snapshot the minimum thread count BEFORE calling + // _try_remove_self_if_above. If that method returns true + // the thread is detached and removed from _threads — the + // destructor's snapshot won't include it, so ~ThreadPool + // may run concurrently on another thread. Reading + // btp->_scaling_config as an argument to the call would + // dereference the pool object before the lock inside + // _try_remove_self_if_above serializes with the destructor. // - // Safety: after _try_remove_self_if_above returns true the - // thread is detached and removed from _threads. The - // destructor's snapshot won't include it, so pthread_join - // is never called. From this point we touch only locals - // (name is a std::string copy made at function entry), so - // there is no use-after-free even if ~ThreadPool runs - // concurrently on another thread. - if(btp->_try_remove_self_if_above(pthread_self(), - btp->_scaling_config.min_threads)) + // name was already copied into a local at function entry, + // so the syslog below is also safe. + std::size_t const min_thr = btp->_scaling_config.min_threads; + if(btp->_try_remove_self_if_above(pthread_self(),min_thr)) { syslog(LOG_DEBUG, "threadpool (%s): idle self-exit",