Better handle client connectivity state. #510
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
Motivation: Pull grpc#506 highlighted some issues with client reconnectivity, namely a race between an automatic reconnection and the client closing the connection. In this case the delegate would report changes of state after the terminal shutdown state. Modifications: - State changes are now thread safe. - Record when a user initiates shutdown, ignore state changes beyond this point and close any channels created after this point. Result: More correct channel reconnectivity.
MrMage
approved these changes
Jul 11, 2019
| /// - `makeBootstrap(configuration:)`, and | ||
| /// - `verifyTLS(channel:)`. | ||
| /// - Parameter channel: The channel that was set. | ||
| private func didSetChannel(to channel: EventLoopFuture<Channel>) { |
There was a problem hiding this comment.
Why does this logic need to be split into will and did set?
There was a problem hiding this comment.
willSet is preparing the channel so that it can be used, didSet is advertising that self.channel may be used (or not in the case of shutdown).
If we had the current didSet logic in willSet then we potentially have race problems when e.g. waiting on .ready (since it would be triggered before actually assigning the new value). If the willSet logic was in didSet then it would be possible to call self.channel and hit an error (closing the channel) before the reconnect callback was registered.
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.
Motivation:
Pull #506 highlighted some issues with client reconnectivity, namely
a race between an automatic reconnection and the client closing the
connection. In this case the delegate would report changes of state
after the terminal shutdown state.
Modifications:
this point and close any channels created after this point.
Result:
More correct channel reconnectivity.