Skip to content

fix(thread_pool): notify under the mutex in post#372

Merged
sgerbino merged 1 commit into
cppalliance:developfrom
sgerbino:fix/foreign-post-join
Jul 23, 2026
Merged

fix(thread_pool): notify under the mutex in post#372
sgerbino merged 1 commit into
cppalliance:developfrom
sgerbino:fix/foreign-post-join

Conversation

@sgerbino

Copy link
Copy Markdown
Collaborator

join() guarantees the work has drained, not that foreign posting threads have left post(). A cross-executor completion could enqueue its item, have it consumed, and see the pool join and destruct while the poster was still inside the condition variable signal (pthread_cond_signal racing pthread_cond_destroy, TSan-confirmed).

Notifying while holding the mutex orders the destroy after the poster's last touch of the condition variable: the consumer cannot pop until the poster releases the lock. Wait morphing keeps the held-lock notify cheap on Linux.

The regression test hops a task through a long-lived worker pool back into a short-lived pool that joins and dies immediately; under TSan the race reproduced on every run before the fix.

join() guarantees the work has drained, not that foreign posting
threads have left post(). A cross-executor completion could enqueue
its item, have it consumed, and see the pool join and destruct while
the poster was still inside the condition variable signal
(pthread_cond_signal racing pthread_cond_destroy, TSan-confirmed).

Notifying while holding the mutex orders the destroy after the
poster's last touch of the condition variable: the consumer cannot
pop until the poster releases the lock. Wait morphing keeps the
held-lock notify cheap on Linux.

The regression test hops a task through a long-lived worker pool
back into a short-lived pool that joins and dies immediately; under
TSan the race reproduced on every run before the fix.
@cppalliance-bot

Copy link
Copy Markdown

An automated preview of the documentation is available at https://372.capy.prtest3.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-07-23 16:07:10 UTC

@cppalliance-bot

Copy link
Copy Markdown

GCOVR code coverage report https://372.capy.prtest3.cppalliance.org/gcovr/index.html
LCOV code coverage report https://372.capy.prtest3.cppalliance.org/genhtml/index.html
Coverage Diff Report https://372.capy.prtest3.cppalliance.org/diff-report/index.html

Build time: 2026-07-23 16:20:23 UTC

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.33%. Comparing base (49b2db9) to head (34e4242).
⚠️ Report is 7 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #372      +/-   ##
===========================================
+ Coverage    98.18%   98.33%   +0.15%     
===========================================
  Files          152      152              
  Lines         8246     8288      +42     
===========================================
+ Hits          8096     8150      +54     
+ Misses         150      138      -12     
Flag Coverage Δ
linux 98.32% <ø> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ex/thread_pool.cpp 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49b2db9...34e4242. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sgerbino
sgerbino merged commit 56f152e into cppalliance:develop Jul 23, 2026
72 of 73 checks passed
@sgerbino
sgerbino deleted the fix/foreign-post-join branch July 23, 2026 17:06
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Beast2 Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants