Fix for Linux platform PowerShell exit on error during SSH remoting connection #4993
Conversation
| @@ -1488,18 +1488,23 @@ private void CloseConnection() | |||
| var stdErrReader = _stdErrReader; | |||
| if (stdErrReader != null) { stdErrReader.Dispose(); } | |||
|
|
|||
| try | |||
| var sshProcessId = _sshProcessId; | |||
There was a problem hiding this comment.
Can't you use _sshProcessId directly instead of creating a new variable?
There was a problem hiding this comment.
CloseConnection can be called from multiple places (state machine, error handling) and this is just to prevent going through this code path multiple times. I am not concerned about race conditions since Process type is thread safe ... I just want to prevent running the code path after the process has been terminated.
I'll add a comment to clarify.
| @@ -1488,18 +1488,23 @@ private void CloseConnection() | |||
| var stdErrReader = _stdErrReader; | |||
| if (stdErrReader != null) { stdErrReader.Dispose(); } | |||
|
|
|||
| try | |||
| var sshProcessId = _sshProcessId; | |||
| _sshProcessId = 0; | |||
There was a problem hiding this comment.
Perhaps initialize it to 0 in the constructor instead of resetting it here?
There was a problem hiding this comment.
CLR guarantees all fields are initialized to zero. I am setting it back to zero here to prevent the code path running twice (see comment above).
This fixes addresses issue #4940.
The problem is that the connection clean up code in SSHClientSessionTransportManager would try to terminate a process with id == 0, which is the initial value for the _sshProcessId field when no SSH process has been created yet. For both Linux and Windows platforms process id of zero is a special process and should not be terminated. Fortunately those processes are not terminated by the existing code but on Linux platforms the PowerShell parent process exits immediately.
The fix is to add checks that the SSH process has been created and is valid before terminating it during connection cleanup.