Fix for SSH remoting when SSH client abruptly terminates #4123
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
iSazonov
reviewed
Jun 28, 2017
| @@ -1636,4 +1636,7 @@ All WinRM sessions connected to Windows PowerShell session configurations, such | |||
| <data name="InvalidRoleCapabilityFileExtension" xml:space="preserve"> | |||
| <value>The provided role capability file {0} does not have the required .psrc extension.</value> | |||
| </data> | |||
| <data name="SSHTerminated" > | |||
There was a problem hiding this comment.
Maybe SSHAbruptlyTerminated ?
There was a problem hiding this comment.
Sure, will change.
mirichmo
reviewed
Jun 28, 2017
| if (string.IsNullOrEmpty(error) || | ||
| if (error == null) | ||
| { | ||
| return error; |
There was a problem hiding this comment.
Why did you decide to return null here and throw in the calling function? Why not just throw here and make the ReadError() function always return non-null strings?
There was a problem hiding this comment.
No special reason except that I think of ReadError() helper method as a wrapper to StreamReader. But I agree that it would be cleaner to just throw in ReadError()
mirichmo
approved these changes
Jun 28, 2017
|
@iSazonov Do you have any additional comments or concerns? |
|
LGTM. |
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.
This PR is for Issue #4122
If the SSH client process that PowerShell is using for the SSH transport terminates abruptly the StreamReader will return null instead of closing the pipe for a normal process exit.
The current error stream reading code ignores null StreamReader values resulting in a hang where the remote session never ends.
Fix is to throw an error when this occurs.