Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Catch exceptions in tunnel host async void method #297

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Catch exceptions in tunnel host async void method #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cs/build/build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
<ReportGeneratorVersion>4.8.13</ReportGeneratorVersion>
<SystemTextEncodingsWebPackageVersion>4.7.2</SystemTextEncodingsWebPackageVersion>
<VisualStudioValidationVersion>15.5.31</VisualStudioValidationVersion>
<DevTunnelsSshPackageVersion>3.11.22</DevTunnelsSshPackageVersion>
<DevTunnelsSshPackageVersion>3.11.26</DevTunnelsSshPackageVersion>
<XunitRunnerVisualStudioVersion>2.4.0</XunitRunnerVisualStudioVersion>
<XunitVersion>2.4.0</XunitVersion>
</PropertyGroup>
Expand Down
4 changes: 4 additions & 0 deletions cs/src/Connections/TunnelHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
84 changes: 49 additions & 35 deletions cs/src/Connections/TunnelRelayTunnelHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Task>();

var tunnelPorts = Tunnel!.Ports ?? Enumerable.Empty<TunnelPort>();
var pfs = session.ActivateService<PortForwardingService>();
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<Task>();

await Task.WhenAll(forwardTasks);
var tunnelPorts = Tunnel!.Ports ?? Enumerable.Empty<TunnelPort>();
var pfs = session.ActivateService<PortForwardingService>();
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<TunnelPort>();
var unusedlocalPorts = new HashSet<int>(
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<TunnelPort>();
var unusedLocalPorts = new HashSet<int>(
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)
Expand Down
12 changes: 6 additions & 6 deletions ts/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ts/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions ts/src/connections/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading