Fix freeze of worker #53
Merged
Conversation
This file contains 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
…tup of worker fixes worker freeze from two sequential waits when a task is emplaced right before the worker calls `wait()` in the start-barrier
psychocoderHPC
approved these changes
Dec 15, 2023
psychocoderHPC
merged commit cfe32f0
into
ComputationalRadiationPhysics:dev
1 check passed
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 race condition on the
cvcondition-variable inWorkercaused a freeze in theWorkerUtilization-unittest.This freeze happens because of two sequential calls to
wait()without proper synchronization ofnotify()in the case when the worker is assigned a new task before the worker starts its work-loop. There, first the Worker must progress to the start-barrier and enter the outer while-condition to wait for the start signal. Beforewait()returns, bothWorker::start()and the task-emplace sent theirnotify(), but thenotify()corresponding to the emplacement of the new task is lost in ambiguity with the start-signal.Then the worker thread will wake up and jump to to
work_loop()where it will wait again but thenotify()which should wake the worker up was already sent and thus the worker will not wake up and the task will not be consumed.In a 'real' scenario this might not be as apparent, because the worker will only be falsely inactive until the next task is emplaced which will wake the worker up and everything will continue normally.
This PR fixes this bug by eliminating the
CondVar-based barrier beforework_loop()which is also not required anymore because of recent refactorings.