thread_pool: snapshot min_threads before _try_remove_self_if_above call

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) <noreply@anthropic.com>
This commit is contained in:
Antonio S. Musumeci 2026-04-10 16:38:39 -04:00
parent 13f56e6ea1
commit bc3a0ed8d6

View file

@ -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",