[release/v7.5] Close pipe client handles after creating the child ssh process#26822
Conversation
daxian-dbw
added
the
CL-General
There was a problem hiding this comment.
Pull request overview
Backport to release/v7.5 of the Windows SSH remoting hang fix by ensuring the parent process closes the client-side pipe handles after spawning ssh.exe, so the stdout/stderr reader threads can terminate when ssh.exe exits.
Changes:
- Dispose Win32
STARTUPINFOafterCreateProcessto close inherited pipe client handles in the parent process. - Minor cleanup to remove redundant namespace qualifications in SSH transport code paths.
- Add Pester coverage to validate SSH remoting cmdlets don’t hang when connecting to a non-existent host.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/powershell/engine/Remoting/SSHRemotingCmdlets.Tests.ps1 | Adds CI test cases intended to catch regressions where SSH remoting could hang on failed connections. |
| src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs | Small refactors/cleanup for process lookup and error-prefix handling in SSH transport read loops. |
| src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs | Core fix: closes parent pipe client handles by disposing startup info after creating the ssh.exe child process; includes minor cleanup and a dispose bugfix in an exception path. |
| Describe "No hangs when host doesn't exist" -Tags "CI" { | ||
| $testCases = @( | ||
| @{ | ||
| Name = 'Verifies no hang for New-PSSession with non-existing host name' | ||
| ScriptBlock = { New-PSSession -HostName "test-notexist" -UserName "test" -ErrorAction Stop } | ||
| FullyQualifiedErrorId = 'PSSessionOpenFailed' | ||
| }, | ||
| @{ | ||
| Name = 'Verifies no hang for Invoke-Command with non-existing host name' | ||
| ScriptBlock = { Invoke-Command -HostName "test-notexist" -UserName "test" -ScriptBlock { 1 } -ErrorAction Stop } | ||
| FullyQualifiedErrorId = 'PSSessionStateBroken' | ||
| } | ||
| ) | ||
|
|
||
| It "<Name>" -TestCases $testCases { | ||
| param ($ScriptBlock, $FullyQualifiedErrorId) | ||
|
|
||
| $ScriptBlock | Should -Throw -ErrorId $FullyQualifiedErrorId -ExceptionType 'System.Management.Automation.Remoting.PSRemotingTransportException' | ||
| } |
There was a problem hiding this comment.
The new hang-regression tests rely on a specific -ErrorId value, but earlier in this same file the SSH open failure error id is noted as platform-dependent (matched via -Match 'PSSessionOpenFailed'). To avoid flaky CI failures, consider using a wildcard/regex match for the error id (or asserting on exception type/message instead), and add an explicit short timeout around the SSH invocation so a reintroduced hang fails quickly rather than stalling the whole test run. Also consider using a reserved non-existent domain (e.g. *.invalid) instead of test-notexist to reduce the chance of accidental DNS resolution in some environments.
Backport of #26491 to release/v7.5
Triggered by @daxian-dbw on behalf of @daxian-dbw
Original CL Label: CL-General
/cc @PowerShell/powershell-maintainers
Impact
REQUIRED: Choose either Tooling Impact or Customer Impact (or both). At least one checkbox must be selected.
Tooling Impact
Customer Impact
Fixes hang when attempting SSH connections to non-existent hosts on Windows. Affects users using Invoke-Command and New-PSSession with SSH.
Regression
REQUIRED: Check exactly one box.
This is not a regression.
Testing
New tests added to verify no hang occurs. Successfully tested in 7.6 release. Fixes reported customer issues #25203 and #26228.
Risk
REQUIRED: Check exactly one box.
Fixes critical hang issue in SSH remoting on Windows. Changes are scoped to SSH connection setup. Successfully backported to 7.6 branch.