Fix async thread-pool deadlock from semaphore-blocked B-team workers - #9372
Merged
Conversation
A worker that found a job it could run but for an unavailable semaphore went to sleep on one of the two idle worker condition variables (the A/B teams), chosen by pool-size bookkeeping unrelated to why it slept. A semaphore release only ever broadcast wake_a_team, so a semaphore-blocked worker demoted to the B team was never woken by the release that made its job runnable, and the pipeline could deadlock with every thread parked. The A/B teams model idle capacity (no runnable work), which is a different state from being blocked on an external event. Give blocked-on-semaphore workers their own wait channel and wake it on every release, alongside stalled owners. Genuinely-idle A/B-team workers are never waiting on a semaphore, so they are left undisturbed. This also removes the old 0 -> 1-transition gate on the release wakeup, which was independently unsound for acquires of count > 1 satisfied by several count-1 releases in a row. Adds test/correctness/async_deadlock.cpp, pinned to a low thread count (where the pool reshuffles teams and reliably exposes the bug). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
abadams
approved these changes
Aug 20, 2026
abadams
left a comment
Member
There was a problem hiding this comment.
I think in future we could use a parallel deadlock fuzzer that tries random nestings of .parallel and .async and runs under tsan.
5 tasks
mcourteaux
requested changes
Aug 20, 2026
mcourteaux
left a comment
Contributor
There was a problem hiding this comment.
The idea of the separate channel looks great, but I think there is still a race condition.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9372 +/- ##
==========================================
- Coverage 70.08% 70.06% -0.02%
==========================================
Files 259 260 +1
Lines 79158 79287 +129
Branches 19293 19327 +34
==========================================
+ Hits 55477 55553 +76
- Misses 17886 17916 +30
- Partials 5795 5818 +23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mcourteaux
approved these changes
Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A worker that found a job it could run but for an unavailable semaphore went to sleep on one of the two idle worker condition variables (the A/B teams), chosen by pool-size bookkeeping unrelated to why it slept. A semaphore release only ever broadcast wake_a_team, so a semaphore-blocked worker demoted to the B team was never woken by the release that made its job runnable, and the pipeline could deadlock with every thread parked.
The A/B teams model idle capacity (no runnable work), which is a different state from being blocked on an external event. Give blocked-on-semaphore workers their own wait channel and wake it on every release, alongside stalled owners. Genuinely-idle A/B-team workers are never waiting on a semaphore, so they are left undisturbed.
This also removes the old 0 -> 1-transition gate on the release wakeup, which was independently unsound for acquires of count > 1 satisfied by several count-1 releases in a row.
Adds test/correctness/async_deadlock.cpp, pinned to a low thread count (where the pool reshuffles teams and reliably exposes the bug).
Checklist