fix(connections): close SSH tunnel when a pending connection is cancelled (#1369)#1370
Merged
Merged
Conversation
This file contains hidden or 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
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.
Fixes #1369.
Cancelling a tunnelled connection while it was connecting could leave the SSH tunnel (local forward port + keep-alive task) open if the user did not retry.
cancelEnsureConnectedremoved the session but never closed the tunnel, and the cancelled connect task deliberately leaves the connectionId-keyed tunnel alone so it cannot tear down a retry's tunnel (per #1367).Change
cancelEnsureConnectednow closes the SSH tunnel when the cancelled session has no driver, i.e. no live connection is using it.Race safety
SSHTunnelManageris an actor andcloseTunnelruns as a single atomic step (removeValue+close(), no suspension). The cancel path is scheduled the moment the connection is cancelled, before any retry starts its slower tunnel handshake, and a retry'screateTunnelcloses and replaces the tunnel for the same id anyway. So the cancel closes only the abandoned attempt's tunnel and cannot drop a newer one.Tests
The teardown is a side effect on the
SSHTunnelManageractor singleton with no injection seam, so it is not unit-tested here, consistent with the existing SSH tunnel code. Verified by build. Session-removal behaviour is unchanged and still covered by the #1358 tests.