Fix race condition in MonitorConnectionClient #160
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
lvfxx
changed the title
lvfxx
force-pushed
the
heal-client-eventCh-refactor
branch
2 times, most recently
from
2e6b4e3
to
b1dddf0
Compare
alex-yust
reviewed
Mar 17, 2020
alex-yust
reviewed
Mar 17, 2020
alex-yust
reviewed
Mar 17, 2020
lvfxx
force-pushed
the
heal-client-eventCh-refactor
branch
from
42a1051
to
f383b78
Compare
… fanoutEventCh closing Signed-off-by: Sergey Semenov <[email protected]>
lvfxx
force-pushed
the
heal-client-eventCh-refactor
branch
from
f383b78
to
b41b364
Compare
This was referenced Mar 17, 2020
Signed-off-by: Sergey Semenov <[email protected]>
nsmbot
pushed a commit
that referenced
this pull request
Jun 27, 2023
…i@main PR link: networkservicemesh/api#160 Commit: 9785eac Author: Danil Uzlov Date: 2023-06-27 21:36:11 +0700 Message: - Add RESELECT_REQUESTED connection state (#160) * add reselect connection state Signed-off-by: Danil Uzlov <[email protected]> * fix ci Signed-off-by: Danil Uzlov <[email protected]> --------- Signed-off-by: NSMBot <[email protected]>
denis-tingaikin
pushed a commit
that referenced
this pull request
Jun 27, 2023
…i@main (#1477) PR link: networkservicemesh/api#160 Commit: 9785eac Author: Danil Uzlov Date: 2023-06-27 21:36:11 +0700 Message: - Add RESELECT_REQUESTED connection state (#160) * add reselect connection state * fix ci --------- Signed-off-by: NSMBot <[email protected]> Co-authored-by: NSMBot <[email protected]>
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.
Problem:
Undefined order of execution between
monitorConnectionClient.eventLoop()goroutine that passes events tofanoutEventChand goroutine that's waiting<--context.Done()and closesfanoutEventChleads to different number oftestOnHeal.Request()calls in/pkg/networkservice/common/heal/client_test.goTestNewClient_MissingConnectionsOnInittest that leads to different number (2 or 3)testOnHeal.Request()invocations. Explanation diagramSolutions i see:
time.Sleep(100 * time.Millisecond)after sendingINITIAL_STATE_TRANSFERevent to ensure that this event will be handled earlier than context cancellation.Guarantee that chain context cancellation will be handled after connection event, if it was cancelled after connection event sending. One way to do that i see is to emulate the server falling down by closing
eventCh, not chain context cancelling. Chain context cancelling is rather about client's intention to shut down and logically may be async, but server's falling down is not.Tried to implement the second one in this PR :)