Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pool shutdown deadlock #108

Closed
wants to merge 1 commit into from
Closed

Fix pool shutdown deadlock #108

wants to merge 1 commit into from

Conversation

alugowski
Copy link

Fix deadlock.

The root cause is missed notifications. Condition variable wait/notify is instantaneous, so if a notification is sent while nobody is listening then it will be missed.

The fix is to send the notification while the tasks_mutex is acquired. This, combined with the predicate version of wait already in use, ensures that there is no way to miss the notification.

Ran against all three reported deadlock bugs and none demonstrate the issue anymore.

Fixes #107
Fixes #100
Fixes #93

@bshoshany
Copy link
Owner

Thanks for the PR! This is pretty much what I had in mind when looking into solving this bug (based on your previous analysis in #93). In my version of this solution I'm using a const std::scoped_lock, but that's just because I'm OCD about const. I will incorporate this into v3.4.0, which should be released tomorrow (along with some other bug fixes and new features I've been working on).

There will also be two tests added to the test suite in this release, which check for deadlocks - one upon object destruction and another upon pool reset (both of which call destroy_threads()). These checks will only be added to the non-light version, since they require a new feature to prevent the deadlock from locking the test itself, and that feature is not available in the light version.

Thank you again for your help with this bug, and sorry I didn't consider it high priority at first! I am now closing this PR and all the related issues since they have been resolved in v3.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Threadpool create/destroy deadlock [BUG] deadlock[BUG] [BUG] Deadlock on pool exit
2 participants