Resolve stalls in the example client. #52
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: When running the example client on extremely low latency connections it can occasionally stall, printing nothing to stdout before exiting. This is the result of a timing window where we receive the data before the pipe bootstrap has been configured and our glue handlers are not in place. There is also another timing window where we block reads on the SSH Child Channel because the glue handler asked if the partner was writable before the partner was inserted into the pipe channel. Modifications: - Cleanup both timing windows. Result: The SSH client example will be substantially more reliable.
Lukasa
added
the
patch-version-bump-only
Joannis
reviewed
Nov 20, 2020
| // It's possible our partner asked if we were writable, before, and we couldn't answer. | ||
| // Consider updating it. | ||
| if context.channel.isWritable { | ||
| self.partner?.partnerBecameWritable() |
There was a problem hiding this comment.
Is there a scenario where we became writable but the partner wasn't set yet? This also goes for channelWritabilityChanged.
There was a problem hiding this comment.
Seeing the current usages, and the fact that the class is internal, I think it's never going to happen. So it might be wise to make it implicitly unwrapped.
There was a problem hiding this comment.
In principle the partner should always be set, but we do nil it out explicitly on handlerRemoved so it’s likely better safe than sorry here.
artemredkin
approved these changes
Nov 20, 2020
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:
When running the example client on extremely low latency connections it
can occasionally stall, printing nothing to stdout before exiting. This
is the result of a timing window where we receive the data before the
pipe bootstrap has been configured and our glue handlers are not in
place.
There is also another timing window where we block reads on the SSH
Child Channel because the glue handler asked if the partner was writable
before the partner was inserted into the pipe channel.
Modifications:
Result:
The SSH client example will be substantially more reliable.