Fix race condition with deferred connections #12
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
MiKom
force-pushed
the
fix-connection-evalutor
branch
from
7ac1c49 to
fec12d4
Compare
Lazy creation of ConnectionEvaluator in event loop was not guarded and it may happen from a secondary thread (or multiple of those as was the case with tests). This is actually quite probable as the idea behind deferred connections is that they may be connected to from threads other than main. In "Multiple Signals with Evaluator" test two threads tried to do a deferred connection at the same time and apparently this caused race condition in creation of the evaluator. Now we unconditionally create the connection evaluator in the event loop. This way, we know that the evaluator is fully created by the end of the event loop construction. Our assumption is that user shouldn't use the evaluator from another thread before the loop is fully constructed.
phyBrackets
approved these changes
Jun 28, 2024
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.
Lazy creation of ConnectionEvaluator in event loop was not guarded and
it may happen from a secondary thread (or multiple of those as was the
case with tests). This is actually quite probable as the idea behind
deferred connections is that they may be connected to from threads other
than main.
In "Multiple Signals with Evaluator" test two threads tried to do a deferred
connection at the same time and apparently this caused race condition in
creation of the evaluator.
Now we unconditionally create the connection evaluator in the event
loop. This way, we know that the evaluator is fully created by the end
of the event loop construction. Our assumption is that the user shouldn't
use the evaluator from another thread before the loop is fully
constructed.