From bfd51f249e7884e5166c6fa317cb47e446f3a563 Mon Sep 17 00:00:00 2001 From: Jason Ginchereau Date: Mon, 28 Aug 2023 15:48:47 -1000 Subject: [PATCH] Catch exceptions in tunnel host async void method --- cs/build/build.props | 2 +- cs/src/Connections/TunnelHost.cs | 4 + cs/src/Connections/TunnelRelayTunnelHost.cs | 84 ++++++++++++--------- ts/package-lock.json | 12 +-- ts/package.json | 4 +- ts/src/connections/package.json | 4 +- 6 files changed, 64 insertions(+), 46 deletions(-) diff --git a/cs/build/build.props b/cs/build/build.props index b10b1f82..58435233 100644 --- a/cs/build/build.props +++ b/cs/build/build.props @@ -48,7 +48,7 @@ 4.8.13 4.7.2 15.5.31 - 3.11.22 + 3.11.26 2.4.0 2.4.0 diff --git a/cs/src/Connections/TunnelHost.cs b/cs/src/Connections/TunnelHost.cs index 4432dddc..8caa69e4 100644 --- a/cs/src/Connections/TunnelHost.cs +++ b/cs/src/Connections/TunnelHost.cs @@ -137,6 +137,10 @@ internal async Task ForwardPortAsync( // Treat it as equivalent to the client rejecting the forwarding request. forwarder = null; } + catch (ObjectDisposedException) + { + forwarder = null; + } if (forwarder == null) { diff --git a/cs/src/Connections/TunnelRelayTunnelHost.cs b/cs/src/Connections/TunnelRelayTunnelHost.cs index 24082455..1c3e2105 100644 --- a/cs/src/Connections/TunnelRelayTunnelHost.cs +++ b/cs/src/Connections/TunnelRelayTunnelHost.cs @@ -556,49 +556,63 @@ private async void OnSshClientReconnected(object? sender, EventArgs e) => private async Task StartForwardingExistingPortsAsync( SshSession session, bool removeUnusedPorts = false) { - // Send port-forward request messages concurrently. The client may still handle the - // requests sequentially but at least there is no network round-trip between them. - var forwardTasks = new List(); - - var tunnelPorts = Tunnel!.Ports ?? Enumerable.Empty(); - var pfs = session.ActivateService(); - pfs.ForwardConnectionsToLocalPorts = this.ForwardConnectionsToLocalPorts; - foreach (TunnelPort port in tunnelPorts) + try { - // ForwardPortAsync() catches and logs most exceptions that might normally occur. - forwardTasks.Add(ForwardPortAsync(pfs, port, CancellationToken.None)); - } + // Send port-forward request messages concurrently. The client may still handle the + // requests sequentially but at least there is no network round-trip between them. + var forwardTasks = new List(); - await Task.WhenAll(forwardTasks); + var tunnelPorts = Tunnel!.Ports ?? Enumerable.Empty(); + var pfs = session.ActivateService(); + pfs.ForwardConnectionsToLocalPorts = this.ForwardConnectionsToLocalPorts; + foreach (TunnelPort port in tunnelPorts) + { + // ForwardPortAsync() catches and logs most exceptions that might normally occur. + forwardTasks.Add(ForwardPortAsync(pfs, port, CancellationToken.None)); + } + + await Task.WhenAll(forwardTasks); - // If a tunnel client reconnects, its SSH session Port Forwarding service may - // have remote port forwarders for the ports no longer forwarded. - // Remove such forwarders. - if (removeUnusedPorts && session.SessionId != null) - { - tunnelPorts = Tunnel!.Ports ?? Enumerable.Empty(); - var unusedlocalPorts = new HashSet( - pfs.LocalForwardedPorts - .Select(p => p.LocalPort) - .Where(localPort => localPort.HasValue && !tunnelPorts.Any(tp => tp.PortNumber == localPort)) - .Select(localPort => localPort!.Value)); - - var remoteForwardersToDispose = RemoteForwarders - .Where((kvp) => - ((kvp.Key.SessionId == null && session.SessionId == null) || - ((kvp.Key.SessionId != null && session.SessionId != null) && - Enumerable.SequenceEqual(kvp.Key.SessionId, session.SessionId))) && - unusedlocalPorts.Contains(kvp.Value.LocalPort)) - .Select(kvp => kvp.Key); - - foreach (SessionPortKey key in remoteForwardersToDispose) + // If a tunnel client reconnects, its SSH session Port Forwarding service may + // have remote port forwarders for the ports no longer forwarded. + // Remove such forwarders. + if (removeUnusedPorts && session.SessionId != null) { - if (RemoteForwarders.TryRemove(key, out var remoteForwarder)) + tunnelPorts = Tunnel!.Ports ?? Enumerable.Empty(); + var unusedLocalPorts = new HashSet( + pfs.LocalForwardedPorts + .Select(p => p.LocalPort) + .Where(localPort => localPort.HasValue && + !tunnelPorts.Any(tp => tp.PortNumber == localPort)) + .Select(localPort => localPort!.Value)); + + var remoteForwardersToDispose = RemoteForwarders + .Where((kvp) => + ((kvp.Key.SessionId == null && session.SessionId == null) || + ((kvp.Key.SessionId != null && session.SessionId != null) && + Enumerable.SequenceEqual(kvp.Key.SessionId, session.SessionId))) && + unusedLocalPorts.Contains(kvp.Value.LocalPort)) + .Select(kvp => kvp.Key); + + foreach (SessionPortKey key in remoteForwardersToDispose) { - remoteForwarder?.Dispose(); + if (RemoteForwarders.TryRemove(key, out var remoteForwarder)) + { + remoteForwarder?.Dispose(); + } } } } + catch (Exception ex) + { + // Catch unexpected exceptions because this method is called from async void methods. + TraceSource trace = session.Trace; + trace.TraceEvent( + TraceEventType.Error, + 0, + "Unhandled exception when forwarding ports.\n{1}", + ex); + } } private void OnSshChannelOpening(object? sender, SshChannelOpeningEventArgs e) diff --git a/ts/package-lock.json b/ts/package-lock.json index 6437929d..943ab48c 100644 --- a/ts/package-lock.json +++ b/ts/package-lock.json @@ -76,9 +76,9 @@ "dev": true }, "@microsoft/dev-tunnels-ssh": { - "version": "3.11.22", - "resolved": "https://registry.npmjs.org/@microsoft/dev-tunnels-ssh/-/dev-tunnels-ssh-3.11.22.tgz", - "integrity": "sha512-v3oaZfEJpQm3ZIddWP0kGJm7UABAs83tVb6B2W69S1wXo5U3RmzuijxGmmL82PhgiLmxdC8LA8Gv1XqVZbyRvA==", + "version": "3.11.26", + "resolved": "https://registry.npmjs.org/@microsoft/dev-tunnels-ssh/-/dev-tunnels-ssh-3.11.26.tgz", + "integrity": "sha512-vuS0l0DVSK0bdY6cL3WdVgRjnNwhvulWq6KMwWv0GWU5PMv8w9m9itD3f2vx7X0pwnQmiEMBNOpZnpAiFt9nhg==", "requires": { "buffer": "^5.2.1", "debug": "^4.1.1", @@ -87,9 +87,9 @@ } }, "@microsoft/dev-tunnels-ssh-tcp": { - "version": "3.11.22", - "resolved": "https://registry.npmjs.org/@microsoft/dev-tunnels-ssh-tcp/-/dev-tunnels-ssh-tcp-3.11.22.tgz", - "integrity": "sha512-phWbdmk3IqB1USF4Avog+Kg/AST1Dwets4JZvDwH30W6sQDQl6um6SS91qQ9v2XN+y/DnrLlD6pJ6gjAe6QmWg==", + "version": "3.11.26", + "resolved": "https://registry.npmjs.org/@microsoft/dev-tunnels-ssh-tcp/-/dev-tunnels-ssh-tcp-3.11.26.tgz", + "integrity": "sha512-BhqJ8NTxqSv9rIg9molOi9Jy2jZVmURdlyUxn8R3t+/BJaATKkEOpi6+jR4ANlc7mDCORIXzTWGaf10y4Ci31Q==", "requires": { "@microsoft/dev-tunnels-ssh": "~3.11" } diff --git a/ts/package.json b/ts/package.json index 1a684026..bfe238db 100644 --- a/ts/package.json +++ b/ts/package.json @@ -21,8 +21,8 @@ "build-pack-publish": "npm run build && npm run pack && npm run publish" }, "dependencies": { - "@microsoft/dev-tunnels-ssh": "^3.11.21", - "@microsoft/dev-tunnels-ssh-tcp": "^3.11.21", + "@microsoft/dev-tunnels-ssh": "^3.11.26", + "@microsoft/dev-tunnels-ssh-tcp": "^3.11.26", "await-semaphore": "^0.1.3", "axios": "^0.21.1", "buffer": "^5.2.1", diff --git a/ts/src/connections/package.json b/ts/src/connections/package.json index 7dde6cbb..6d778510 100644 --- a/ts/src/connections/package.json +++ b/ts/src/connections/package.json @@ -20,8 +20,8 @@ "vscode-jsonrpc": "^4.0.0", "@microsoft/dev-tunnels-contracts": "^1.0.0", "@microsoft/dev-tunnels-management": "^1.0.0", - "@microsoft/dev-tunnels-ssh": "^3.11.22", - "@microsoft/dev-tunnels-ssh-tcp": "^3.11.22", + "@microsoft/dev-tunnels-ssh": "^3.11.26", + "@microsoft/dev-tunnels-ssh-tcp": "^3.11.26", "uuid": "^3.3.3", "await-semaphore": "^0.1.3", "websocket": "^1.0.28",