Compare commits

...
87 Commits
Author SHA1 Message Date
Antonio S. MusumeciandClaude Opus 4.7 e726610c6b thread_pool: add enqueue_task PToken overload + fill coverage gaps
Implementation:
- Add enqueue_task(PToken&, FuncType&&) overload so high-rate callers
  can use a dedicated producer token without losing the future-based
  return.  Mirrors the no-token version: try-enqueue with fast-path
  grow on pressure (autoscale pools), block-enqueue otherwise.

Tests (5 new, 87 total):
- tp_enqueue_task_with_ptoken: basic int/string/void futures via ptok
- tp_enqueue_task_ptoken_exception: exception forwarded through future
- tp_enqueue_task_ptoken_multiple_futures: 50 concurrent futures, all
  resolved correctly
- tp_construct_scaling_nonpositive_decline_throws: decline_threshold ≤ 0
  was the only ScalingConfig validation not covered by an explicit test
- tp_autoscale_fast_path_grow_on_pressure: verifies _maybe_grow_on_pressure
  fires (thread_count grows synchronously) when enqueue_work_autoscale
  finds the queue full

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-18 19:57:26 -04:00
Antonio S. MusumeciandClaude Opus 4.7 62b42c1c75 tests: add 12 new ThreadPool tests covering gaps
- tp_enqueue_task_broken_promise_on_destruction: verifies documented
  contract that unfulfilled promises throw broken_promise
- tp_thread_count_matches_threads_size: thread_count() == threads().size()
  after each synchronous mutation
- tp_thread_count_settles_after_churn: atomic mirror converges to
  threads().size() at rest after add/remove bursts
- tp_try_enqueue_work_for_ptoken_times_out_when_full: ptoken overload
  of try_enqueue_work_for was untested for the full-queue timeout path
- tp_tasks_pending_zero_at_construction: sanity check at init
- tp_tasks_pending_zero_after_all_complete: pending drains to 0
- tp_autoscale_verifiable_growth: deterministically asserts thread_count
  exceeds min after sustained queue pressure
- tp_autoscale_verifiable_shrink_to_min: threads shed to min_threads
  after idle_threshold_usecs
- tp_shutdown_ecanceled: smoke test that racing add/remove/set_threads
  against the destructor does not crash or deadlock
- tp_queue_depth_zero_at_idle: queue_depth() == 0 when no work pending
- tp_queue_capacity_unchanged_after_resize: set_threads does not alter
  the queue's max depth
- tp_tasks_enqueued_increments_before_completion: tasks_enqueued() leads
  tasks_completed() under a blocked worker

All 82 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-18 19:38:27 -04:00
Antonio S. Musumeci 0346f2557a thread_pool: document thread_count eventual consistency and -ECANCELED
Two doc-only follow-ups to the preceding cleanup series:

- thread_count() is now an atomic-mirror read rather than a locked
  read of _threads.size().  Note on the declaration that callers
  wanting an exact count after a scaling operation must quiesce the
  pool (or use the wait_until pattern from the unit tests).

- add_thread / remove_thread / set_threads now return -ECANCELED
  during shutdown (_stop set), where they previously silently
  no-oped in some code paths.  Extend the class-level shutdown
  contract comment so callers treat the non-zero return as
  expected, not as a failure.
2026-04-18 18:01:00 -04:00
Antonio S. Musumeci 07bd0d1098 thread_pool: add atomic mirror of _threads.size() for hot-path reads
_maybe_grow_on_pressure and the monitor hill-climber called
thread_count() on every full-queue enqueue and every sample cycle,
which under the hood locked _threads_mutex.  Under sustained load
that serialized producers against every scaling operation.

Add _thread_count_atomic, maintained under _threads_mutex at each
push_back/erase site (constructor, _add_thread_scaling_locked,
_try_claim_and_remove_self, _try_remove_self_if_above).  Switch
thread_count() to an acquire-load of the mirror.  Authoritative
bound enforcement still lives inside _add/_remove_thread_scaling_locked
where _threads.size() is read under the lock, so the approximation
only affects the pre-lock hints that already expect staleness.
2026-04-18 17:56:08 -04:00
Antonio S. Musumeci 0f319ee93c thread_pool: fast-path _process_func when no removal is pending
Every task completion called _try_claim_and_remove_self, which
acquires _scaling_mutex and _threads_mutex even when _remove_count
is zero (the steady-state common case).  Short tasks paid full
mutex overhead per completion.  Add a relaxed _remove_count.load
before the locks; if it reads zero, skip the claim entirely.  A stale
zero just defers the claim to a later completion on any worker,
matching the existing design where the claim is a cooperative race.
2026-04-18 17:52:23 -04:00
Antonio S. Musumeci f7a9fbd0cf thread_pool: replace goto claimed with explicit bool in claim path
The RAII-guarded locks at function top survive the goto correctly, but
the pattern reads awkwardly — reviewers have to verify that nothing in
the claimed path relies on state that the goto skipped over.  Use a
bool flag and a straight-line fall-through instead.  No behavior
change.
2026-04-18 17:51:29 -04:00
Antonio S. Musumeci e55bf1829b thread_pool: skip _remove_thread_scaling_locked during shutdown
_add_thread_scaling_locked already bails out with -ECANCELED when
_stop is set.  Mirror that check in _remove_thread_scaling_locked so
the monitor cannot enqueue a wakeup poison pill after shutdown begins.
The pill would be benign (workers break on _stop before claiming), but
it would sit in the queue until BoundedQueue is destroyed, and skipping
it also avoids an unnecessary semaphore signal on the teardown path.
2026-04-18 17:50:30 -04:00
Antonio S. Musumeci 1b158549b8 thread_pool: document shutdown contract
Spell out the "producers stop before destruction" requirement at the
class level so callers know that enqueue_task returns a future that
will throw broken_promise, and enqueue_work* variants silently drop
work, if they race the destructor.
2026-04-18 17:49:43 -04:00
Antonio S. Musumeci fbf69071d2 thread_pool: use acquire ordering in tasks_pending for weak memory
The prior relaxed loads defeated the documented "bias toward zero"
invariant on ARM and POWER: the hardware can reorder two relaxed loads
against program order, allowing _tasks_completed to be observed before
_tasks_enqueued and producing a delta larger than any single atomic
snapshot.  Upgrade both loads to acquire and put an acquire fence
between them so source-order ordering is preserved on weak memory.
2026-04-18 17:48:46 -04:00
Antonio S. Musumeci 17c0ffaddc 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.
2026-04-18 17:47:49 -04:00
Antonio S. Musumeci b8b97ffeca checkpoint 2026-04-11 17:17:26 -04:00
Antonio S. Musumeci 118f2e767e checkpoint 2026-04-11 15:06:41 -04:00
Antonio S. Musumeci 5f2972278a checkpoint 2026-04-11 14:15:44 -04:00
Antonio S. Musumeci 8ef23782f6 checkpoint 2026-04-11 13:46:57 -04:00
Antonio S. Musumeci adca612e43 checkpoint 2026-04-11 12:30:30 -04:00
Antonio S. Musumeci 9446f784a9 checkpoint 2026-04-11 00:59:30 -04:00
Antonio S. MusumeciandClaude Opus 4.6 890b899219 fix stale line-number reference in monitor comment
The comment referenced "line 632" for the pre-lock thread count
read, but prior edits shifted that line.  Replace the hard-coded
line number with a positional description ("the earlier unlocked
read") so the comment stays accurate across future edits.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 22:31:00 -04:00
Antonio S. MusumeciandClaude Opus 4.6 81e8f6281a fix data race on _threads between constructor and autoscale workers
The constructor's spawn loop pushed to _threads without holding
_threads_mutex.  A just-spawned autoscale worker could time out on
the empty queue (especially with a small idle_threshold_usecs) and
call _try_remove_self_if_above, which acquires _threads_mutex and
modifies _threads (find + erase).  Concurrent unlocked push_back
and locked erase on the same vector is undefined behavior per the
C++ memory model, even when reserve() prevents reallocation.

The same race applied to the _threads.empty() and _threads.size()
reads after the spawn loop.

Fix: lock _threads_mutex around push_back in the spawn loop and
around the post-loop size check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 22:24:14 -04:00
Antonio S. MusumeciandClaude Opus 4.6 26c10d8a2e fix hill-climb monitor expected_count after shrink
After a successful _remove_thread_scaling_locked, the monitor set
expected_count via thread_count(), which returns the pre-shrink
count because the removal is deferred (only _remove_count is
incremented; the actual _threads erase happens asynchronously when
a worker claims the removal).  This caused two problems:

1. On the next cycle, the deferred removal makes thread_count()
   return count-1, which differs from expected_count (count),
   triggering the external-scaling-interference detector and
   wasting the cycle.

2. If the worker hasn't exited yet, thread_count() still equals
   expected_count, so the monitor evaluates throughput with the
   pre-shrink thread count still in effect, potentially reversing
   direction before the shrink's effect is measurable.

Fix: compute expected_count arithmetically (count-1 for shrink,
count+1 for grow) using the authoritative pre-operation count
read under _scaling_mutex, reflecting the intended outcome rather
than the racy current state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 22:21:20 -04:00
Antonio S. MusumeciandClaude Opus 4.6 5f50f76b66 fix idle self-exit allowing pool to drain to 0 threads
_remove_thread_scaling_locked floors min_allowed at 1, preventing
remove_thread/set_threads from ever reducing the pool below 1 thread.
But start_routine_autoscale passed raw _scaling_config.min_threads
to _try_remove_self_if_above without the same floor.  With
min_threads == 0, all idle workers could self-exit, draining the
pool to zero threads and permanently stalling any work enqueued
via non-autoscale enqueue_work (which doesn't call
_maybe_grow_on_pressure).

Apply the same max(min_threads, 1) floor in start_routine_autoscale
to keep the two shrink paths consistent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 22:18:23 -04:00
Antonio S. MusumeciandClaude Opus 4.6 f3b4703ee2 fix constructor resource leak and monitor condvar lost-wakeup race
Two fixes:

1. Constructor: when no threads can be spawned the constructor throws,
   but the destructor never runs for a partially-constructed object.
   The pthread_cond_t and pthread_mutex_t initialized earlier in the
   constructor body were leaked.  Destroy them before throwing.

2. Monitor: the _stop check was outside _monitor_mutex, so the
   destructor could set _stop and signal between the check and
   pthread_cond_timedwait, losing the wakeup and delaying shutdown
   by up to one sample interval (~100ms).  Move the _stop check
   under _monitor_mutex so it is serialized with the destructor's
   signal, closing the race.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 22:15:35 -04:00
Antonio S. MusumeciandClaude Opus 4.6 12a3a689ed fix tasks_pending() load ordering to bias toward zero as intended
The previous ordering loaded completed before enqueued, which meant a
completion between the two loads would be missed (stale lower c),
inflating the delta and over-reporting pending tasks. This is the
opposite of what the comment claimed. Swapping to load enqueued first
means a completion between the two loads yields a higher c, shrinking
the delta — biasing toward zero, which is the safe direction for
scaling heuristics.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 22:12:02 -04:00
Antonio S. Musumeci 911d31c04e checkpoint 2026-04-10 22:09:11 -04:00
Antonio S. Musumeci ea2d08ae4d checkpoint 2026-04-10 22:00:43 -04:00
Antonio S. Musumeci 37bb632524 checkpoint 2026-04-10 21:58:00 -04:00
Antonio S. Musumeci aff4ea1f01 checkpoint 2026-04-10 21:32:14 -04:00
Antonio S. Musumeci b210680bad checkpoint 2026-04-10 20:37:44 -04:00
Antonio S. Musumeci 51f803ca9c checkpoint 2026-04-10 19:44:23 -04:00
Antonio S. Musumeci 36a16249ac checkpoint 2026-04-10 19:31:10 -04:00
Antonio S. Musumeci bd56969399 checkpoint 2026-04-10 19:17:04 -04:00
Antonio S. MusumeciandClaude Opus 4.6 7ce5364c35 thread_pool: extract _try_claim_removal to deduplicate CAS loop
_process_func contained two identical compare_exchange_weak loops for
claiming a _remove_count decrement — one in the poison-pill handler
and one after task completion.  Duplicating the loop risks the two
copies diverging during future edits and makes the function harder
to follow.

Extract the loop into _try_claim_removal() which returns true when a
decrement is successfully claimed.  Both call sites now delegate to
the shared helper.  The relaxed memory ordering rationale (the
subsequent _remove_self acquires _threads_mutex, providing the
happens-before with the scaling-side increment) is documented on the
helper.  No functional change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 18:46:48 -04:00
Antonio S. MusumeciandClaude Opus 4.6 2d3528431b thread_pool: use producer token on fast path of enqueue_work_autoscale
The PToken overload of enqueue_work_autoscale passed ptok_ to the
blocking fallback enqueue but not to the try_enqueue fast path.
BoundedQueue has a try_enqueue(PToken&, ...) overload, so the token
was silently ignored on the common (non-full) path, forgoing the
per-producer thread-local allocation that moodycamel provides.

Pass ptok_ through to try_enqueue so the fast path benefits from the
same producer-token locality as the fallback path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 18:20:14 -04:00
Antonio S. MusumeciandClaude Opus 4.6 375f1f9303 thread_pool: measure cooldown from completion in _maybe_grow_on_pressure
The monitor stores _last_scale_time_usecs after _add_thread_scaling_locked
returns (i.e. after pthread_create), but _maybe_grow_on_pressure stored
a timestamp captured before the add.  Under resource pressure
pthread_create can take non-trivial time, making the effective cooldown
shorter than configured on the fast-path grow.

Move the store after _add_thread_scaling_locked and use a fresh
_now_usecs() call, matching the monitor's behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 17:34:22 -04:00
Antonio S. MusumeciandClaude Opus 4.6 159a5c7b71 thread_pool: skip spurious wakeups in hill-climb monitor
pthread_cond_timedwait can return early due to spurious wakeups.
When this happens the throughput delta covers a shorter-than-expected
interval, producing an artificially low sample.  Two consecutive
spurious wakes would cause the EMA to decline twice, crossing the
decline_threshold and triggering a false direction reversal — the
monitor would shrink the pool when throughput was actually stable.

Record the wall-clock time before sleeping and discard any sample
where the elapsed time is less than half the target interval.  The
_stop check runs first so shutdown is never delayed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 17:32:51 -04:00
Antonio S. MusumeciandClaude Opus 4.6 c43a14f269 thread_pool: close TOCTOU in set_threads by reading _remove_count under lock
set_threads read _threads.size() under _threads_mutex then read
_remove_count outside the lock.  Between the two reads a worker could
claim a _remove_count decrement and erase itself from _threads via
_remove_self, causing set_threads to double-count that shrink: once
from the reduced _threads.size() (had the lock been re-acquired) and
once from the stale _remove_count that was already claimed.  This
could cause a spurious over-shrink and a -EINVAL return even though
the pool was at the target size.

Move the _remove_count load inside the _threads_mutex scope so both
values are read atomically with respect to worker self-removal.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 17:29:07 -04:00
Antonio S. MusumeciandClaude Opus 4.6 06a2ba2f35 thread_pool: use acquire ordering in tasks_pending for ARM/POWER
tasks_pending() loads _tasks_completed then _tasks_enqueued and
returns the delta.  The bias-toward-zero property (safe for scaling
heuristics) depends on completed being observed no later than
enqueued.  With relaxed ordering on both loads, a weakly-ordered
CPU (ARM, POWER) can reorder the enqueued load before the completed
load, potentially observing a stale (lower) completed with a fresh
(higher) enqueued — over-reporting pending work.

Use acquire on the first load (_tasks_completed) so that subsequent
loads in program order cannot be reordered before it.  On x86 (TSO)
this compiles to the same instruction as relaxed, so there is zero
overhead on the primary target architecture.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 16:48:19 -04:00
Antonio S. MusumeciandClaude Opus 4.6 05413b86fd thread_pool: serialize idle self-exit with scaling operations
_try_remove_self_if_above only held _threads_mutex, while set_threads
and the monitor thread hold _scaling_mutex.  An idle worker calling
_try_remove_self_if_above could shrink the pool concurrently with
set_threads reading _threads.size(), causing set_threads to
miscalculate the delta and overshoot (add too many or remove too few
threads).

Fix by acquiring _scaling_mutex before _threads_mutex in
_try_remove_self_if_above.  This matches the existing lock ordering
used by _maybe_grow_on_pressure and the monitor/set_threads paths
(_scaling_mutex always acquired before _threads_mutex), so no
deadlock is introduced.

Now all paths that change the thread count — add_thread,
remove_thread, set_threads, monitor hill-climb, pressure grow, and
idle self-exit — are serialized under _scaling_mutex.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 16:46:01 -04:00
Antonio S. MusumeciandClaude Opus 4.6 dbb7c5e785 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>
2026-04-10 16:44:53 -04:00
Antonio S. MusumeciandClaude Opus 4.6 f9e36b3889 thread_pool: replace assert with runtime check in enqueue_work_autoscale
The assert guarding against BoundedQueue::try_enqueue moving the
callable on failure is compiled out in release builds (-DNDEBUG).
If the queue implementation ever changed to move-from on failure,
the empty Func would be enqueued as a poison pill, silently causing
a worker thread to exit — a subtle, hard-to-diagnose pool corruption.

Replace both assert() calls with a std::logic_error throw that
remains active in all build configurations.  This makes the
invariant violation immediately visible rather than silently
dangerous.

Also remove the now-unused <cassert> include.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 16:44:18 -04:00
Antonio S. MusumeciandClaude Opus 4.6 bc3a0ed8d6 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>
2026-04-10 16:38:39 -04:00
Antonio S. MusumeciandCopilot 13f56e6ea1 thread_pool: store scaling config by value
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-10 16:23:08 -04:00
Antonio S. MusumeciandCopilot d1bcde3753 Fix thread pool shutdown race
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-10 14:18:02 -04:00
Antonio S. MusumeciandClaude Opus 4.6 462020d19a Make _scaling_config const to enforce write-once invariant
_scaling_config was documented as read-only after construction but
nothing prevented accidental mutation.  Move its initialization from
the constructor body into the member initializer list (using a
ternary to handle the nullptr case) and mark it const.

_autoscale_enabled cannot be made const because the constructor
clears it if the monitor thread fails to spawn.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 13:44:27 -04:00
Antonio S. MusumeciandClaude Opus 4.6 0a0c82c224 Document thread-safety of _scaling_config and _autoscale_enabled
These fields are written once in the constructor before any worker or
monitor threads are spawned, and are read-only thereafter.  Add an
explicit comment noting the write-once-then-read-only invariant so
future maintainers know that making them mutable would require
synchronization.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 13:42:49 -04:00
Antonio S. MusumeciandClaude Opus 4.6 65e136c667 Document why relaxed ordering is correct for _remove_count CAS
The CAS loop on _remove_count uses memory_order_relaxed for both
success and failure, which is correct but non-obvious.  Add a comment
explaining that _remove_self acquires _threads_mutex immediately after
the successful CAS, and that mutex acquisition provides the necessary
happens-before edge between the scaling-side increment and the
worker-side erase/detach.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 13:41:06 -04:00
Antonio S. MusumeciandClaude Opus 4.6 02703ef239 Apply cooldown backoff on pthread_create failure
When pthread_create fails (e.g., EAGAIN from OS resource limits),
the monitor and fast-path pressure grow would retry on the next cycle
with no delay, potentially flooding syslog with identical warnings
every 100ms.

Fix: stamp _last_scale_time_usecs on creation failure so the existing
cooldown mechanism (200ms default) suppresses immediate retries.  This
reuses the scaling cooldown infrastructure rather than adding a
separate backoff timer.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 13:34:34 -04:00
Antonio S. MusumeciandClaude Opus 4.6 1dd979f955 Reset hill-climb warmup after idle period to re-seed EMA
When throughput dropped to zero, the monitor reset direction and
decline_streak but not warmup_samples.  On resumption, the first
meaningful throughput sample was compared against a stale pre-idle
EMA value, which could cause the hill-climber to make a scaling
decision based on a meaningless delta (new throughput vs. zero-decayed
EMA from potentially minutes ago).

Reset warmup_samples to 0 in the idle branch so the EMA is re-seeded
from two fresh samples before the hill-climber acts.  This ensures
the first scaling decision after an idle period is based on current
throughput trends rather than stale history.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 13:25:33 -04:00
Antonio S. MusumeciandClaude Opus 4.6 47a097ee96 Use condvar for monitor sleep to allow instant shutdown wakeup
The monitor thread used nanosleep for its sample interval, which meant
the destructor had to block for up to one full sample period (100ms by
default) waiting for the monitor to wake up and notice _stop.

Replace nanosleep with pthread_cond_timedwait on a dedicated condvar
(_monitor_cond) using CLOCK_MONOTONIC.  The destructor now signals
the condvar immediately after setting _stop, so the monitor wakes up
and exits without waiting for the interval to expire.

This also eliminates the EINTR retry loop — pthread_cond_timedwait
handles signals internally and returns when either the timeout expires
or the condvar is signaled.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 13:14:34 -04:00
Antonio S. MusumeciandClaude Opus 4.6 a90e13a19d Fix set_threads over-shrink when prior removals are still pending
set_threads computed its delta from _threads.size(), which does not
reflect _remove_count signals already in flight.  If called twice
before workers drained the first batch of removals, the second call
would see the original thread count and issue a full set of redundant
removal signals, potentially shrinking the pool well below the
requested target.

Fix: compute the effective thread count as _threads.size() minus
pending _remove_count, and use that as the baseline for the grow/shrink
delta.  This makes set_threads idempotent regardless of drain timing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 12:59:16 -04:00
Antonio S. MusumeciandClaude Opus 4.6 62d905bb29 Fix monitor/fast-path cooldown race in hill-climb scaling
The monitor thread checked _last_scale_time_usecs without holding
_scaling_mutex, then called add_thread()/remove_thread() which each
acquired the mutex independently.  The fast-path _maybe_grow_on_pressure
could scale between the monitor's cooldown check and its scaling action,
allowing two scaling events closer together than cooldown_usecs.

Fix: hold _scaling_mutex across both the cooldown check and the scaling
action in the monitor loop, using the _locked variants directly.  This
ensures the monitor and fast-path pressure grow are mutually exclusive
for the entire check-and-act sequence.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 12:56:51 -04:00
Antonio S. MusumeciandClaude Opus 4.6 4d35726fc6 Assert try_enqueue does not move-from callable on failure
enqueue_work_autoscale relies on BoundedQueue::try_enqueue leaving the
callable untouched when the semaphore check fails.  This is correct
today because tryWait() is evaluated before std::forward moves the
item, but the coupling is implicit — a future BoundedQueue change
that reorders these operations would silently introduce a use-after-
move bug on the blocking fallback path.

Add a debug assert after the failed try_enqueue to verify the Func is
still valid before using it on the blocking path.  Strengthen the
comment to explicitly name the invariant being relied upon.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 12:40:43 -04:00
Antonio S. MusumeciandClaude Opus 4.6 ea47934c63 Fix idle pool stall when shrinking via _remove_count
_remove_thread_scaling_locked increments _remove_count to signal a
worker to self-exit, but this counter is only checked after a worker
completes a real task.  Workers blocked on wait_dequeue (standard loop)
or try_wait_dequeue_for (autoscale loop) never see the removal request,
so shrinking an idle pool via remove_thread() or set_threads() stalls
indefinitely.

Fix: enqueue a wakeup poison pill alongside each _remove_count
increment so a blocked worker is guaranteed to wake up.

To prevent double-removal (one worker exits via the task-completion CAS
loop consuming _remove_count, another exits via the wakeup pill), the
poison pill handler now attempts to consume a _remove_count before
self-removing.  If the count is already zero (claimed by a worker that
finished a real task), the pill is treated as stale and the worker
continues processing.

Shutdown path is unaffected: the destructor sets _stop before enqueueing
its own pills, and the pill handler checks _stop first, returning BREAK
unconditionally during shutdown regardless of _remove_count.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 12:39:30 -04:00
Antonio S. MusumeciandClaude Opus 4.6 ec024b246a Load completed before enqueued in tasks_pending to bias low
The previous ordering (enqueued first, then completed) meant a
task completing between the two loads would inflate the pending
count.  For scaling heuristics, over-reporting pressure is worse
than under-reporting: it can trigger unnecessary thread creation.

Swap the load order so completed is read first.  A completion
between the two loads now reduces the delta toward zero, which
is the safer direction for autoscale decisions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 12:06:51 -04:00
Antonio S. MusumeciandClaude Opus 4.6 80af7ffcc0 Drain _remove_count in destructor before sending poison pills
If remove_thread() was called but no worker claimed the decrement
before shutdown, _remove_count remains positive.  Workers exiting
via poison pills would enter the _remove_count CAS loop in
_process_func, but since _stop is set, _remove_self is a no-op
anyway.  The stale counter is harmless at runtime but confusing
when debugging — it suggests pending work that will never complete.

Zero the counter after _stop is set and before enqueuing poison
pills so the shutdown path is clean and unambiguous.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 11:46:59 -04:00
Antonio S. MusumeciandClaude Opus 4.6 b9552758fd Fix TOCTOU race between _try_remove_self_if_above and destructor
The _stop check in _try_remove_self_if_above was outside the
_threads_mutex lock, creating a window where a worker thread could
read _stop==false, then the destructor could set _stop and snapshot
_threads (including that worker), then the worker could acquire the
lock, detach itself, and exit.  The destructor would then call
pthread_join on a detached thread — undefined behavior.

Move the _stop check inside the _threads_mutex lock so it is
serialized with the destructor's snapshot.  Now either the worker
holds the lock first (detaches and erases itself before the
snapshot) or the destructor sets _stop first (worker sees it and
bails out, remaining joinable in the snapshot).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 11:45:44 -04:00
Antonio S. MusumeciandClaude Opus 4.6 0466d7208f Remove unused <chrono> and <memory> includes
<chrono> was used by the std::chrono::seconds(5) future wait in the
old promise-based _remove_thread_scaling_locked, which was replaced
by the atomic _remove_count counter.  <memory> was used for
std::make_shared in the same code path.  Neither header is used
anywhere else in thread_pool.hpp.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 01:16:24 -04:00
Antonio S. MusumeciandClaude Opus 4.6 4b5b102e6f Re-check cooldown under lock in _maybe_grow_on_pressure
The pre-lock cooldown check used a stale _last_scale_time_usecs read.
If another thread (monitor hill-climb, concurrent enqueue) updated
the timestamp between the pre-lock check and the lock acquisition,
the function would proceed to acquire _scaling_mutex and contend
unnecessarily.  The CAS that followed would correctly reject the
stale value, so this was not a correctness bug, but it caused
avoidable lock contention on the hot enqueue path.

Add an authoritative cooldown re-check after acquiring _scaling_mutex
with a fresh timestamp read.  Replace the CAS with a plain store
since the _scaling_mutex already serializes writers.  Keep the
pre-lock speculative check as a fast-path filter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 01:15:21 -04:00
Antonio S. MusumeciandClaude Opus 4.6 11fafba6ae Detect external scaling in hill-climb to avoid misattribution
The hill-climb monitor samples throughput, perturbs thread count by
+/-1, and checks whether throughput improved.  But between samples,
other mechanisms (fast-path pressure grow, idle self-exit) can also
change the thread count.  The monitor had no way to distinguish its
own perturbation's effect from these external changes, which could
cause it to reinforce or reverse direction based on unrelated scaling.

Track the expected thread count after each perturbation.  At the next
sample, compare the actual count to the expected one.  If they differ,
external scaling occurred — skip the throughput attribution, reset the
decline streak, and re-baseline from the current count.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 01:13:32 -04:00
Antonio S. MusumeciandClaude Opus 4.6 20acdfce19 Log actual thread count after hill-climb scaling
The monitor thread read the thread count before calling add_thread or
remove_thread, then logged count+1 or count-1 as the new size.  If
concurrent scaling (fast-path grow, idle self-exit) changed the count
between the read and the action, the logged value was wrong.

Read thread_count() after the scaling action for the log message so
it reflects reality.  The pre-action read is kept for the bounds
check (direction guard) since that only needs to be approximately
correct.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 01:03:49 -04:00
Antonio S. MusumeciandClaude Opus 4.6 78d320a64c Clamp set_threads to autoscale bounds
set_threads did not enforce ScalingConfig::min_threads / max_threads,
so a caller could push the pool outside the range that the monitor
thread and idle self-exit logic expect.  For example, set_threads(1)
with min_threads=2 would be rejected by _remove_thread_scaling_locked
partway through, leaving the pool in a partially-shrunk state.
Growing above max_threads had no guard at all.

Clamp the requested count to [min_threads, max_threads] when
autoscaling is enabled, mirroring the constructor's clamping logic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 00:58:16 -04:00
Antonio S. MusumeciandClaude Opus 4.6 8f41b40692 Replace thread-removal lambda+promise with atomic counter
_remove_thread_scaling_locked previously enqueued a heap-allocated
lambda (capturing a shared_ptr<promise>) to signal a specific worker
to exit.  This had two problems:

1. Bug: if the 5-second future::wait_for timed out, the function
   returned -EBUSY but the lambda remained in the queue.  When a
   worker eventually executed it, it would unconditionally set
   _tl_should_exit and call _remove_self, silently killing an
   arbitrary thread with no compensating add.

2. Held _scaling_mutex for up to 5 seconds while blocking on the
   future, serializing all concurrent scaling operations (add_thread,
   remove_thread, _maybe_grow_on_pressure, monitor hill-climb).

Replace the entire mechanism with an atomic _remove_count counter.
_remove_thread_scaling_locked simply increments the counter and
returns immediately.  After each task, _process_func does a
CAS-decrement; the winning worker detaches itself via _remove_self
and exits.

This eliminates:
- All heap allocations (shared_ptr, promise, lambda captures)
- The thread-local _tl_should_exit flag
- The 5-second blocking window under _scaling_mutex
- The stale-lambda bug entirely (no lambda to go stale)
- The <memory> include (no longer needed)

During shutdown, _remove_self is a no-op (_stop is set), so a worker
that wins the CAS stays in _threads and is joined normally by the
destructor.  Any outstanding _remove_count credits are harmlessly
abandoned.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-10 00:50:25 -04:00
Antonio S. MusumeciandClaude Opus 4.6 af2419ff71 Extract _process_func to deduplicate worker loop logic
start_routine and start_routine_autoscale shared ~30 lines of identical
post-dequeue logic: poison-pill detection, exception-guarded execution,
func destruction, tasks_completed bookkeeping, and the _tl_should_exit
exit protocol. Any bug fix to this logic had to be applied in both
places.

Extract the shared code into _process_func which returns a WorkerAction
enum (CONTINUE / BREAK / EXIT). Both worker loops now call this helper,
reducing the maintenance surface and eliminating the duplication.

Also document the BoundedQueue::try_enqueue no-move-on-failure contract
in enqueue_work_autoscale, since the correctness of the try-then-block
pattern depends on it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:50:54 -04:00
Antonio S. MusumeciandClaude Opus 4.6 c2da86d6da Document tasks_pending() as an approximate metric
tasks_pending() reads _tasks_enqueued and _tasks_completed in two
separate relaxed loads. Between the loads, additional tasks can
complete, making the result transiently inconsistent (the (e > c)
guard prevents underflow but not staleness).

Add a comment clarifying that this is intentionally approximate and
should not be used for synchronization decisions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:43:43 -04:00
Antonio S. MusumeciandClaude Opus 4.6 db784e00a2 Document detached-thread lifetime safety in autoscale self-exit
After _try_remove_self_if_above detaches and erases the thread, the
destructor's snapshot will not include it — so pthread_join is never
called on it, and the ThreadPool object can be destroyed while this
thread is still returning.

This is safe because the thread only accesses local variables after
detach (name is a std::string copy made at function entry), but the
reasoning is subtle. Add an explicit safety comment explaining why
there is no use-after-free.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:42:02 -04:00
Antonio S. MusumeciandClaude Opus 4.6 96824eab31 Document _tl_should_exit protocol in worker loops
The _tl_should_exit mechanism in start_routine (and
start_routine_autoscale) is correct but the control flow is non-obvious:
_remove_thread_scaling_locked enqueues a lambda that sets the
thread-local flag and calls _remove_self (detach + erase), then the
worker checks the flag after running the func and exits.

Replace the terse one-line comments with an explanation of the full
protocol so future readers don't mistake it for dead code.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:40:47 -04:00
Antonio S. MusumeciandClaude Opus 4.6 2333f6078a Reset hill-climb direction after idle period
When throughput dropped below 0.5 (idle), decline_streak was reset but
direction was preserved. When work resumed, the monitor continued in
whatever direction it held before the idle period — often a shrink
direction that was correct for the previous workload but wrong for the
new burst. This could cause the pool to shrink immediately when it
should be growing.

Reset direction to +1 (grow) when entering the idle branch so the pool
starts the next active period biased toward growth, which is the safer
default under new load.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:32:47 -04:00
Antonio S. MusumeciandClaude Opus 4.6 b150f2c685 Retry nanosleep on EINTR in monitor thread
nanosleep returns -1 with errno EINTR when interrupted by a signal.
The remaining time was discarded (second arg was NULL), causing the
monitor to wake early and sample a shortened interval. This produced
artificially low throughput numbers that corrupted the EMA and could
trigger unnecessary scale-down decisions.

Loop on EINTR using the remaining time output parameter so the full
sample interval is always honored. Also check _stop on each retry
so the monitor exits promptly during shutdown.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:30:13 -04:00
Antonio S. MusumeciandClaude Opus 4.6 beb6961769 Fix _maybe_grow_on_pressure burning cooldown at max threads
The CAS on _last_scale_time_usecs was performed before checking whether
the pool was already at max_threads. When the pool was at capacity, the
cooldown window was consumed but no thread was added. This suppressed
legitimate scaling attempts from the monitor thread for the entire
cooldown period.

Move the CAS after the max-threads check and the _scaling_mutex
acquisition so the cooldown is only claimed when a thread will actually
be added. The mutex serializes concurrent callers, and the CAS still
deduplicates concurrent pressure-grow attempts that pass the check
simultaneously.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 23:28:28 -04:00
Antonio S. MusumeciandClaude Opus 4.6 111bedf2c9 Clamp initial thread count to ScalingConfig bounds in constructor
When auto-scaling is enabled, the constructor did not enforce that the
initial thread_count falls within [min_threads, max_threads]. Starting
with fewer threads than min_threads meant the monitor had to
immediately grow the pool, and starting above max_threads created
threads that the monitor would then immediately try to shrink.

Clamp the count before spawning so the pool starts within its
configured operating range.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 22:00:25 -04:00
Antonio S. MusumeciandClaude Opus 4.6 d166418675 Replace usleep with nanosleep in monitor thread
usleep is POSIX-obsolescent and its behavior on signal interruption is
under-specified. Replace with nanosleep which is the current POSIX
standard and has well-defined EINTR semantics. Pre-compute the
timespec once outside the loop since the interval is constant.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:58:43 -04:00
Antonio S. MusumeciandClaude Opus 4.6 f8ac649e6f Rename _add/_remove_thread_locked to _scaling_locked
The _locked suffix conventionally means the caller already holds the
lock being named. These functions expect _scaling_mutex to be held,
but the old name was ambiguous — it looked like _threads_mutex.

Rename to _add_thread_scaling_locked and _remove_thread_scaling_locked
to make the precondition explicit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:56:26 -04:00
Antonio S. MusumeciandClaude Opus 4.6 0bef28d154 Bound _remove_thread_locked wait to avoid indefinite _scaling_mutex hold
_remove_thread_locked enqueues a self-exit task and blocks on the
future while holding _scaling_mutex. If all workers are busy with
long-running tasks, this blocks indefinitely — starving the monitor
thread (and any concurrent add_thread/set_threads) since they all
contend on _scaling_mutex.

Replace the unbounded wait with a 5-second timed wait. On timeout,
return -EBUSY so callers can retry or report the condition. The
enqueued self-exit task will still be picked up eventually and the
thread will remove itself when a worker becomes free.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:51:50 -04:00
Antonio S. MusumeciandClaude Opus 4.6 8baf274cde Replace two-statement mutex_lockguard macro with single RAII declaration
The old macro expanded to two statements (mutex_lock + DEFER unlock).
In an unbraced if/else only the lock would be conditional while the
DEFER always executed — a latent double-unlock / unbalanced-unlock bug.

Add ScopedMutexLock to both debug and ndebug headers:
 - debug: stores __FILE__/__func__/__LINE__ from the call site and
   passes them to _mutex_lock/_mutex_unlock for diagnostics
 - ndebug: lightweight wrapper around mutex_lock/mutex_unlock

Redefine mutex_lockguard to create a single uniquely-named
ScopedMutexLock variable (via __COUNTER__), making it safe in any
statement context. Remove the now-unused scope_guard include.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:42:24 -04:00
Antonio S. MusumeciandClaude Opus 4.6 13fc1b27df Enforce cooldown_usecs in monitor hill-climb loop
cooldown_usecs was only checked in _maybe_grow_on_pressure (the
fast-path enqueue grow). The monitor thread scaled every
sample_interval_usecs unconditionally, so a recent fast-path grow
could be immediately followed by a monitor grow, defeating the
cooldown.

Check _last_scale_time_usecs against cooldown_usecs before applying
any monitor scaling decision, making the cooldown global across both
scaling paths.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:22:47 -04:00
Antonio S. MusumeciandClaude Opus 4.6 2aa2311a55 Add EMA smoothing and decline-streak to hill-climb monitor
A single noisy throughput sample would immediately flip the scaling
direction, causing the monitor to oscillate under bursty workloads —
continuously adding and removing threads every sample interval.

Replace raw throughput comparison with an exponential moving average
(alpha=0.3) and require two consecutive declining samples before
reversing direction. This filters out transient dips and keeps the
hill-climber from thrashing.

Also use a 2-sample warmup period to seed the EMA before making any
scaling decisions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:20:42 -04:00
Antonio S. MusumeciandClaude Opus 4.6 055a2d4abe Propagate errors from set_threads instead of ignoring them
set_threads silently ignored failures from _add_thread_locked (e.g.
pthread_create failure) and _remove_thread_locked (e.g. at min_threads).
The caller had no way to know the pool was undersized or that shrinking
stopped early.

Stop on the first error and return it so callers can detect partial
completion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:05:32 -04:00
Antonio S. MusumeciandClaude Opus 4.6 82e7980ec9 Fix TOCTOU race in _maybe_grow_on_pressure
Multiple concurrent enqueue_work_autoscale callers could all pass the
cooldown check and each add a thread, overshooting max_threads and
bypassing the intended rate limit.

Use compare_exchange_strong on _last_scale_time_usecs so only one
caller wins the cooldown window. Then hold _scaling_mutex for the
thread-count check and _add_thread_locked call so the grow is atomic
with respect to other scaling operations (monitor, set_threads).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 21:04:37 -04:00
Antonio S. MusumeciandClaude Opus 4.6 38fee4dd5f Fix use-after-free in start_routine_autoscale idle self-exit
After _try_remove_self_if_above detaches the thread, the destructor
will not join it. If the destructor runs before the detached thread
reaches the syslog call, btp->_name is a dangling reference.

Copy _name into a local string at thread start so the syslog after
self-removal uses stack-local data that outlives the detach.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:59:31 -04:00
Antonio S. MusumeciandClaude Opus 4.6 9222a4ff62 Move _tp_now_usecs into ThreadPool as private static _now_usecs
The file-scope function _tp_now_usecs had a leading underscore
followed by a lowercase letter, which is reserved at file scope
by the C++ standard. Move it to a private static member function
ThreadPool::_now_usecs where leading underscores on member names
are permitted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:46:42 -04:00
Antonio S. MusumeciandClaude Opus 4.6 e87acf0433 Add sane defaults to ScalingConfig members
ScalingConfig was value-initialized to all zeros when no config
is provided. While the non-autoscale path gates all access, a
zero idle_threshold_usecs would cause try_wait_dequeue_for to
return immediately if the autoscale path were ever reached by
mistake.

Add default member initializers: min_threads=2, max_threads=64,
sample_interval=100ms, cooldown=200ms, idle_threshold=5s. These
match the values recommended in the scaling policy design and
provide safe behavior even if a caller constructs ScalingConfig
without setting all fields.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:43:21 -04:00
Antonio S. MusumeciandClaude Opus 4.6 73b6bb3cdf Move _tasks_enqueued increment to after successful enqueue
The counter was incremented before the enqueue call, which can
block indefinitely for bounded queues. If tasks_pending() is used
as a drain-complete signal (pending == 0), the pre-increment
would overcount while a producer is blocked waiting for a slot.

Move the increment to after the enqueue returns in all paths:
enqueue_work, enqueue_work_autoscale, and enqueue_task. The
try_enqueue variants already incremented after success.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:39:32 -04:00
Antonio S. MusumeciandClaude Opus 4.6 3bcf66cdaa Fix monitor and idle-exit racing past min_threads
The hill-climb monitor and idle self-exit paths could both shrink
the pool past min_threads. The monitor read thread count, released
the lock, then called remove_thread - but between those steps an
idle worker could also self-exit, dropping below the threshold.

Three changes:
- _remove_thread_locked now checks min_threads (from ScalingConfig)
  under the same lock as the size check, refusing removal if at or
  below the bound.
- Idle self-exit replaced with _try_remove_self_if_above which
  atomically checks thread count and erases under a single lock
  acquisition, closing the TOCTOU window.
- The previous _remove_self is retained for the non-scaling paths
  (poison pill exit during set_threads/remove_thread).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:36:48 -04:00
Antonio S. MusumeciandClaude Opus 4.6 6a52d7b1a2 Fix semaphore drift from enqueue_unbounded in BoundedQueue
enqueue_unbounded bypasses the semaphore so control messages
(poison pills, thread removal tasks) can never block on a full
queue. But wait_dequeue always signals a slot after consuming,
so each unbounded item that gets dequeued inflated the semaphore
by +1, permanently raising the effective max queue depth.

Fix by calling tryWait() in enqueue_unbounded before enqueueing.
When the queue has room (common case), this consumes a slot and
the dequeue-side signal restores balance perfectly. When the queue
is genuinely full (tryWait fails), the item still enqueues but
temporarily exceeds max_depth by one until consumed - a transient
overshoot bounded by control messages in flight.

A CAS-based approach that absorbed extra signals on the dequeue
side was also considered but rejected: it can starve blocked
bounded producers by swallowing the signal they are waiting for.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 20:21:36 -04:00
Antonio S. MusumeciandClaude Opus 4.6 cb6f444e45 Fix double std::forward in enqueue_work_autoscale
The try-enqueue and fallback block-enqueue both called
std::forward<FuncType>(func_) on the same parameter. If
try_enqueue's internals ever change to move from the argument
before failing, the second forward would operate on a moved-from
object.

Fix by materializing into a Func up front and using std::move
for both attempts. Also extract the duplicated scaling logic into
_maybe_grow_on_pressure(), eliminating the copy-paste between the
PToken and non-PToken overloads.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 19:39:18 -04:00
Antonio S. MusumeciandClaude Opus 4.6 c0b4ea3fae Fix destructor race with _remove_self during shutdown
The destructor and autoscale idle-exit path could race: a worker
calling _remove_self would erase itself from _threads and detach
while the destructor was iterating _threads to enqueue poison pills
and join. This could cause too few poison pills (deadlocking
remaining workers) or pthread_join on a detached thread (UB).

Two changes:
- _remove_self now checks _stop and returns immediately during
  shutdown, ensuring no thread detaches after destruction begins.
- Destructor snapshots _threads under the mutex before enqueueing
  poison pills and joining, so it operates on a stable copy.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 19:35:35 -04:00
Antonio S. Musumeci 9ae145b7e9 checkpoint 2026-04-09 19:31:47 -04:00
Antonio S. Musumeci 9dc461c96d checkpoint 2026-04-09 19:13:58 -04:00
6 changed files with 2494 additions and 132 deletions
+1157 -6
View File
File diff suppressed because it is too large Load Diff
+38 -1
View File
@@ -29,6 +29,12 @@
// - Producers wait (decrement) before enqueuing, blocking when full.
// - Consumers signal (increment) after dequeuing, freeing a slot.
// This co-locates the invariant so the pairing cannot drift apart.
//
// enqueue_unbounded bypasses the semaphore for control messages that must
// never block. Each such item that is later dequeued via wait_dequeue
// causes a +1 semaphore inflation, slightly raising the effective max
// depth. This is bounded by the number of control messages in flight
// (thread removals, shutdown pills) and is acceptable for correctness.
template<typename T, typename Traits = moodycamel::ConcurrentQueueDefaultTraits>
class BoundedQueue
{
@@ -41,12 +47,14 @@ private:
Queue _queue;
moodycamel::LightweightSemaphore _slots;
std::size_t const _max_depth;
public:
explicit
BoundedQueue(std::size_t max_depth_)
: _queue(),
_slots(max_depth_)
_slots(max_depth_),
_max_depth(max_depth_)
{
}
@@ -125,6 +133,10 @@ public:
void
enqueue_unbounded(U &&item_)
{
// Try to consume a slot so the dequeue-side signal stays balanced.
// If the queue is full (tryWait fails), enqueue anyway - the item
// will temporarily exceed max_depth by one until it is consumed.
_slots.tryWait();
_queue.enqueue(std::forward<U>(item_));
}
@@ -137,6 +149,31 @@ public:
_slots.signal();
}
// -- Timed dequeue (waits up to timeout, signals slot if successful) -----
bool
try_wait_dequeue_for(CToken &ctok_, T &item_, std::int64_t timeout_usecs_)
{
if(!_queue.wait_dequeue_timed(ctok_, item_, timeout_usecs_))
return false;
_slots.signal();
return true;
}
// -- Introspection -------------------------------------------------------
std::size_t
max_depth() const
{
return _max_depth;
}
std::size_t
size_approx() const
{
return _queue.size_approx();
}
// -- Token creation -----------------------------------------------------
PToken
+14 -7
View File
@@ -6,8 +6,6 @@
#include "mutex_ndebug.hpp"
#endif
#include "scope_guard/scope_guard.hpp"
typedef pthread_mutex_t mutex_t;
// Simple mutex_t wrapper to provide RAII
@@ -53,9 +51,18 @@ public:
}
};
// Single-line mutex guard using scope_guard
// Expands to mutex_lock() followed by DEFER mutex_unlock()
// Both lock and unlock capture __FILE__/__func__/__LINE__ from the call site
// RAII mutex guard macro. Expands to a single variable declaration,
// so it is safe in unbraced if/else/for/while.
// In debug builds the call-site __FILE__/__func__/__LINE__ are
// forwarded to _mutex_lock/_mutex_unlock for diagnostics.
#define MUTEX_LOCKGUARD_CONCAT_(a,b) a##b
#define MUTEX_LOCKGUARD_CONCAT(a,b) MUTEX_LOCKGUARD_CONCAT_(a,b)
#ifdef DEBUG
#define mutex_lockguard(m) \
mutex_lock(m); \
DEFER { mutex_unlock(m); }
ScopedMutexLock MUTEX_LOCKGUARD_CONCAT(_mtx_guard_,__COUNTER__) \
((m),__FILE__,__func__,__LINE__)
#else
#define mutex_lockguard(m) \
ScopedMutexLock MUTEX_LOCKGUARD_CONCAT(_mtx_guard_,__COUNTER__)(m)
#endif
+27
View File
@@ -130,3 +130,30 @@ _mutex_unlock(pthread_mutex_t &mutex_,
if(rv != 0)
_print_error_and_abort(__func__,rv,file_,func_,linenum_);
}
class ScopedMutexLock
{
private:
pthread_mutex_t &_mutex;
const char *_file;
const char *_func;
int _line;
public:
ScopedMutexLock(pthread_mutex_t &mutex_,
const char *file_,
const char *func_,
int line_)
: _mutex(mutex_), _file(file_), _func(func_), _line(line_)
{
_mutex_lock(_mutex,_file,_func,_line);
}
~ScopedMutexLock()
{
_mutex_unlock(_mutex,_file,_func,_line);
}
ScopedMutexLock(const ScopedMutexLock&) = delete;
ScopedMutexLock& operator=(const ScopedMutexLock&) = delete;
};
+21
View File
@@ -62,3 +62,24 @@ mutex_destroy(pthread_mutex_t &mutex_)
if(rv != 0)
std::abort();
}
class ScopedMutexLock
{
private:
pthread_mutex_t &_mutex;
public:
ScopedMutexLock(pthread_mutex_t &mutex_)
: _mutex(mutex_)
{
mutex_lock(_mutex);
}
~ScopedMutexLock()
{
mutex_unlock(_mutex);
}
ScopedMutexLock(const ScopedMutexLock&) = delete;
ScopedMutexLock& operator=(const ScopedMutexLock&) = delete;
};
File diff suppressed because it is too large Load Diff