Fix race condition in AsyncPipelinedExecutor destructor #271
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
|
Builds 38700. |
Kh4L
reviewed
Nov 6, 2018
| @@ -47,6 +47,15 @@ class DLL_PUBLIC AsyncPipelinedExecutor : public PipelinedExecutor { | |||
| cpu_thread_.ForceStop(); | |||
| mixed_thread_.ForceStop(); | |||
| gpu_thread_.ForceStop(); | |||
| /* | |||
| * We need to call shutdown here and not relay on cpu_thread_ destructor | |||
Kh4L
reviewed
Nov 6, 2018
| /* | ||
| * We need to call shutdown here and not relay on cpu_thread_ destructor | ||
| * as when WorkerThread destructor is called conditional variables and mutexes | ||
| * from this class may no longer exists while work inside WorkerThread is still |
Kh4L
approved these changes
Nov 6, 2018
When AsyncPipelinedExecutor is destroyed WorkerThread destructor is called as well. Inside, it waits for the running job to be completed and then thread executing it is shut down as well. The problem arises because the job that is executed inside WorkerThread uses conditionals and mutexes that are destroyed during AsyncPipelinedExecutor destruction and before WorkerThread finishes. So WorkerThread may end up hanging inside work() it needs to complete before is able to shutdown, but this work it trying to use no longer existing conditional or mutex waiting for it infinitely. Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
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.
When AsyncPipelinedExecutor is destroyed WorkerThread destructor is called as well. Inside, it waits for the running job to be completed and then thread executing it is shut down as well. The problem arises because the job that is executed
inside WorkerThread uses conditionals and mutexes that are destroyed during AsyncPipelinedExecutor destruction and before WorkerThread finishes.
So WorkerThread may end up hanging inside work() it needs to complete before
is able to shutdown, but this work it trying to use no longer existing
conditional or mutex waiting for it infinitely.
Signed-off-by: Janusz Lisiecki jlisiecki@nvidia.com