From 8170df46d15342cd6ff635f67de0e97d4d75a238 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Tue, 17 Mar 2026 14:17:37 -0700 Subject: [PATCH 1/3] Fix leaking ssh connections with VS Attach to Process This PR address the issue where every SSH PortSupplier AD7Port will always create a new SSH connection on the remote machine when the Attach to Process window opens and closes. This causes ports to be kept open until VS completly closes. The changes here are to cache an existing connection if VS opens and closes the attach to process window. Adds a ref count mechanism to ensure if you are debugging multiple processes on the same machine, it does not Clean the connection until all processes are done. --- src/MIDebugEngine/AD7.Impl/AD7Engine.cs | 1 + ...tudio.Debugger.Interop.UnixPortSupplier.cs | 9 +++- src/SSHDebugPS/AD7/AD7Port.cs | 47 ++++++++++++++----- src/SSHDebugPS/SSH/SSHHelper.cs | 3 ++ src/SSHDebugPS/SSH/SSHPortSupplier.cs | 37 +++++++++++++-- 5 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/MIDebugEngine/AD7.Impl/AD7Engine.cs b/src/MIDebugEngine/AD7.Impl/AD7Engine.cs index b9e60a869..ffb8146b8 100755 --- a/src/MIDebugEngine/AD7.Impl/AD7Engine.cs +++ b/src/MIDebugEngine/AD7.Impl/AD7Engine.cs @@ -229,6 +229,7 @@ public int Attach(IDebugProgram2[] portProgramArray, IDebugProgramNode2[] progra if (port is IDebugUnixShellPort) { _unixPort = (IDebugUnixShellPort)port; + (_unixPort as IDebugPortCleanup)?.AddSessionRef(); } StartDebugging(launchOptions); } diff --git a/src/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier.cs b/src/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier.cs index 87f9da2d6..7ee30a393 100644 --- a/src/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier.cs +++ b/src/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier/Microsoft.VisualStudio.Debugger.Interop.UnixPortSupplier.cs @@ -73,7 +73,7 @@ public interface IDebugUnixShellPort } /// - /// Interface implemented by a port that supports explicit cleanup + /// Interface implemented by a port that supports explicit cleanup of shared connections. /// [ComImport()] [ComVisible(true)] @@ -82,9 +82,14 @@ public interface IDebugUnixShellPort public interface IDebugPortCleanup { /// - /// Clean up debugging resources + /// Decrement the session reference count and close the connection when it reaches zero. /// void Clean(); + + /// + /// Increment the session reference count on this port. + /// + void AddSessionRef(); } /// diff --git a/src/SSHDebugPS/AD7/AD7Port.cs b/src/SSHDebugPS/AD7/AD7Port.cs index 819f63eda..fe89e8f93 100644 --- a/src/SSHDebugPS/AD7/AD7Port.cs +++ b/src/SSHDebugPS/AD7/AD7Port.cs @@ -21,6 +21,7 @@ internal abstract class AD7Port : IDebugPort2, IDebugUnixShellPort, IDebugPortCl private Connection _connection; private readonly Dictionary _eventCallbacks = new Dictionary(); private uint _lastCallbackCookie; + private int _sessionRefCount; protected string Name { get; private set; } @@ -37,16 +38,19 @@ public AD7Port(AD7PortSupplier portSupplier, string name, bool isInAddPort) protected Connection GetConnection() { - if (_connection == null) + lock (_lock) { - _connection = GetConnectionInternal(); - if (_connection != null) + if (_connection == null) { - Name = _connection.Name; + _connection = GetConnectionInternal(); + if (_connection != null) + { + Name = _connection.Name; + } } - } - return _connection; + return _connection; + } } protected abstract Connection GetConnectionInternal(); @@ -60,7 +64,10 @@ public bool IsConnected { get { - return _connection != null; + lock (_lock) + { + return _connection != null; + } } } @@ -241,14 +248,32 @@ public bool IsLinux() return GetConnection().IsLinux(); } + public void AddSessionRef() + { + Interlocked.Increment(ref _sessionRefCount); + } + public void Clean() { - try + if (Interlocked.Decrement(ref _sessionRefCount) > 0) { - _connection?.Close(); + return; + } + + lock (_lock) + { + try + { + _connection?.Close(); + } + // Dev15 632648: Liblinux sometimes throws exceptions on shutdown - we are shutting down anyways, so ignore to not crash + catch (Exception) { } + finally + { + _connection = null; + Interlocked.Exchange(ref _sessionRefCount, 0); + } } - // Dev15 632648: Liblinux sometimes throws exceptions on shutdown - we are shutting down anyways, so ignore to not crash - catch (Exception) { } } } } diff --git a/src/SSHDebugPS/SSH/SSHHelper.cs b/src/SSHDebugPS/SSH/SSHHelper.cs index de1e0d3ea..e30edb5ce 100644 --- a/src/SSHDebugPS/SSH/SSHHelper.cs +++ b/src/SSHDebugPS/SSH/SSHHelper.cs @@ -49,16 +49,19 @@ internal static SSHConnection CreateSSHConnectionFromConnectionInfo(ConnectionIn } else { + remoteSystem.Dispose(); return null; } } else { + remoteSystem.Dispose(); throw new InvalidOperationException("Why is IVsConnectionManager null?"); } } catch (Exception ex) { + remoteSystem.Dispose(); VsShellUtilities.ShowMessageBox(ServiceProvider.GlobalProvider, ex.Message, null, OLEMSGICON.OLEMSGICON_CRITICAL, OLEMSGBUTTON.OLEMSGBUTTON_OK, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST); return null; diff --git a/src/SSHDebugPS/SSH/SSHPortSupplier.cs b/src/SSHDebugPS/SSH/SSHPortSupplier.cs index 349292ee8..ef60efb59 100644 --- a/src/SSHDebugPS/SSH/SSHPortSupplier.cs +++ b/src/SSHDebugPS/SSH/SSHPortSupplier.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; using System.IO; using System.Runtime.InteropServices; using liblinux; @@ -20,6 +21,8 @@ internal class SSHPortSupplier : AD7PortSupplier { private const string _Name = "SSH"; private readonly Guid _Id = new Guid("3FDDF14E-E758-4695-BE0C-7509920432C9"); + private readonly object _portCacheLock = new object(); + private readonly Dictionary _portCache = new Dictionary(StringComparer.OrdinalIgnoreCase); protected override Guid Id { get { return _Id; } } protected override string Name { get { return _Name; } } @@ -33,11 +36,26 @@ public override int AddPort(IDebugPortRequest2 request, out IDebugPort2 port) string name; HR.Check(request.GetPortName(out name)); - AD7Port newPort = new SSHPort(this, name, isInAddPort: true); + SSHPort sshPort; + lock (_portCacheLock) + { + if (!_portCache.TryGetValue(name, out sshPort)) + { + sshPort = new SSHPort(this, name, isInAddPort: true); + if (sshPort.IsConnected) + { + _portCache[name] = sshPort; + } + } + else + { + sshPort.EnsureConnected(); + } + } - if (newPort.IsConnected) + if (sshPort.IsConnected) { - port = newPort; + port = sshPort; return HR.S_OK; } @@ -53,7 +71,18 @@ public override int EnumPorts(out IEnumDebugPorts2 ppEnum) for (int i = 0; i < store.Connections.Count; i++) { ConnectionInfo connectionInfo = (ConnectionInfo)store.Connections[i]; - ports[i] = new SSHPort(this, GetFormattedSSHConnectionName(connectionInfo), isInAddPort: false); + string name = GetFormattedSSHConnectionName(connectionInfo); + + lock (_portCacheLock) + { + if (!_portCache.TryGetValue(name, out SSHPort port)) + { + port = new SSHPort(this, name, isInAddPort: false); + _portCache[name] = port; + } + + ports[i] = port; + } } ppEnum = new AD7PortEnum(ports); From b7a07a20ac37c8ac5e9be0737ddd81cad9c3c52a Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 25 Mar 2026 19:53:15 -0700 Subject: [PATCH 2/3] Address PR Comments --- src/SSHDebugPS/AD7/AD7Port.cs | 120 ++++++++++++++++++++++++++++---- src/SSHDebugPS/SSH/SSHHelper.cs | 73 ++++++++++--------- 2 files changed, 146 insertions(+), 47 deletions(-) diff --git a/src/SSHDebugPS/AD7/AD7Port.cs b/src/SSHDebugPS/AD7/AD7Port.cs index fe89e8f93..3923ff3b8 100644 --- a/src/SSHDebugPS/AD7/AD7Port.cs +++ b/src/SSHDebugPS/AD7/AD7Port.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Threading; @@ -22,6 +23,7 @@ internal abstract class AD7Port : IDebugPort2, IDebugUnixShellPort, IDebugPortCl private readonly Dictionary _eventCallbacks = new Dictionary(); private uint _lastCallbackCookie; private int _sessionRefCount; + private int _activeAsyncCommands; protected string Name { get; private set; } @@ -95,6 +97,8 @@ private AD7Process[] EnumProcessesInternal() result = processList.Select((proc) => new AD7Process(this, proc)).ToArray(); }); + CloseConnectionIfIdle(); + return result; } @@ -162,7 +166,92 @@ void IDebugUnixShellPort.ExecuteSyncCommand(string commandDescription, string co void IDebugUnixShellPort.BeginExecuteAsyncCommand(string commandText, bool runInShell, IDebugUnixShellCommandCallback callback, out IDebugUnixShellAsyncCommand asyncCommand) { - GetConnection().BeginExecuteAsyncCommand(commandText, runInShell, callback, out asyncCommand); + var wrappedCallback = new AsyncCommandCallback(this, callback); + var connection = GetConnection(); + lock (_lock) + { + _activeAsyncCommands++; + } + connection.BeginExecuteAsyncCommand(commandText, runInShell, wrappedCallback, out asyncCommand); + asyncCommand = new AsyncCommandWrapper(this, asyncCommand); + } + + private class AsyncCommandWrapper : IDebugUnixShellAsyncCommand + { + private readonly AD7Port _port; + private readonly IDebugUnixShellAsyncCommand _inner; + private int _notified; + + public AsyncCommandWrapper(AD7Port port, IDebugUnixShellAsyncCommand inner) + { + _port = port; + _inner = inner; + } + + public void Write(string text) => _inner.Write(text); + public void WriteLine(string text) => _inner.WriteLine(text); + + public void Abort() + { + _inner.Abort(); + // If OnExit didn't fire (abort path), notify the port now + if (Interlocked.CompareExchange(ref _notified, 1, 0) == 0) + { + _port.OnAsyncCommandExited(); + } + } + } + + private void OnAsyncCommandExited() + { + lock (_lock) + { + _activeAsyncCommands--; + } + CloseConnectionIfIdle(); + } + + private void CloseConnectionIfIdle() + { + lock (_lock) + { + if (_sessionRefCount > 0 || _activeAsyncCommands > 0 || _connection == null) + { + return; + } + + var conn = _connection; + _connection = null; + try { conn.Close(); } catch (Exception) { } + } + } + + /// + /// Wraps an IDebugUnixShellCommandCallback to detect when an async command exits, + /// allowing the port to close idle connections for engines that do not call + /// IDebugPortCleanup.AddSessionRef/Clean (e.g., vsdbg). + /// + private class AsyncCommandCallback : IDebugUnixShellCommandCallback + { + private readonly AD7Port _port; + private readonly IDebugUnixShellCommandCallback _inner; + + public AsyncCommandCallback(AD7Port port, IDebugUnixShellCommandCallback inner) + { + _port = port; + _inner = inner; + } + + public void OnOutputLine(string line) + { + _inner.OnOutputLine(line); + } + + public void OnExit(string exitCode) + { + _inner.OnExit(exitCode); + _port.OnAsyncCommandExited(); + } } void IConnectionPointContainer.EnumConnectionPoints(out IEnumConnectionPoints ppEnum) @@ -250,29 +339,34 @@ public bool IsLinux() public void AddSessionRef() { - Interlocked.Increment(ref _sessionRefCount); + lock (_lock) + { + _sessionRefCount++; + } + EnsureConnected(); } public void Clean() { - if (Interlocked.Decrement(ref _sessionRefCount) > 0) - { - return; - } - lock (_lock) { + Debug.Assert(_sessionRefCount > 0, "Unbalanced call to Clean -- no matching AddSessionRef"); + _sessionRefCount--; + + if (_sessionRefCount > 0) + { + return; + } + + var conn = _connection; + _connection = null; + try { - _connection?.Close(); + conn?.Close(); } // Dev15 632648: Liblinux sometimes throws exceptions on shutdown - we are shutting down anyways, so ignore to not crash catch (Exception) { } - finally - { - _connection = null; - Interlocked.Exchange(ref _sessionRefCount, 0); - } } } } diff --git a/src/SSHDebugPS/SSH/SSHHelper.cs b/src/SSHDebugPS/SSH/SSHHelper.cs index e30edb5ce..f506d842b 100644 --- a/src/SSHDebugPS/SSH/SSHHelper.cs +++ b/src/SSHDebugPS/SSH/SSHHelper.cs @@ -23,56 +23,61 @@ internal static SSHConnection CreateSSHConnectionFromConnectionInfo(ConnectionIn if (connectionInfo != null) { UnixSystem remoteSystem = new UnixSystem(); - string name = SSHPortSupplier.GetFormattedSSHConnectionName(connectionInfo); - - while (true) + bool success = false; + try { - try - { - VSOperationWaiter.Wait( - StringResources.WaitingOp_Connecting.FormatCurrentCultureWithArgs(name), - throwOnCancel: false, - action: (cancellationToken) => - remoteSystem.Connect(connectionInfo)); - break; - } - catch (RemoteAuthenticationException) + string name = SSHPortSupplier.GetFormattedSSHConnectionName(connectionInfo); + + while (true) { - IVsConnectionManager connectionManager = (IVsConnectionManager)ServiceProvider.GlobalProvider.GetService(typeof(IVsConnectionManager)); - if (connectionManager != null) + try { - IConnectionManagerResult result = connectionManager.ShowDialog(StringResources.AuthenticationFailureHeader, StringResources.AuthenticationFailureDescription, connectionInfo); - - if (result != null && (result.DialogResult & ConnectionManagerDialogResult.Succeeded) == ConnectionManagerDialogResult.Succeeded) + VSOperationWaiter.Wait( + StringResources.WaitingOp_Connecting.FormatCurrentCultureWithArgs(name), + throwOnCancel: false, + action: (cancellationToken) => + remoteSystem.Connect(connectionInfo)); + break; + } + catch (RemoteAuthenticationException) + { + IVsConnectionManager connectionManager = (IVsConnectionManager)ServiceProvider.GlobalProvider.GetService(typeof(IVsConnectionManager)); + if (connectionManager != null) { - connectionInfo = result.ConnectionInfo; + IConnectionManagerResult result = connectionManager.ShowDialog(StringResources.AuthenticationFailureHeader, StringResources.AuthenticationFailureDescription, connectionInfo); + + if (result != null && (result.DialogResult & ConnectionManagerDialogResult.Succeeded) == ConnectionManagerDialogResult.Succeeded) + { + connectionInfo = result.ConnectionInfo; + } + else + { + return null; + } } else { - remoteSystem.Dispose(); - return null; + throw new InvalidOperationException("Why is IVsConnectionManager null?"); } } - else + catch (Exception ex) { - remoteSystem.Dispose(); - throw new InvalidOperationException("Why is IVsConnectionManager null?"); + VsShellUtilities.ShowMessageBox(ServiceProvider.GlobalProvider, ex.Message, null, + OLEMSGICON.OLEMSGICON_CRITICAL, OLEMSGBUTTON.OLEMSGBUTTON_OK, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST); + return null; } } - catch (Exception ex) + + success = true; + return new SSHConnection(remoteSystem); + } + finally + { + if (!success) { remoteSystem.Dispose(); - VsShellUtilities.ShowMessageBox(ServiceProvider.GlobalProvider, ex.Message, null, - OLEMSGICON.OLEMSGICON_CRITICAL, OLEMSGBUTTON.OLEMSGBUTTON_OK, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST); - return null; } } - - // NOTE: This will be null if connect is canceled - if (remoteSystem != null) - { - return new SSHConnection(remoteSystem); - } } return null; From da7871ec7306dad70b12abe6796746562b76435f Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Thu, 26 Mar 2026 17:35:06 -0700 Subject: [PATCH 3/3] Address more PR issues --- src/SSHDebugPS/AD7/AD7Port.cs | 88 +++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/src/SSHDebugPS/AD7/AD7Port.cs b/src/SSHDebugPS/AD7/AD7Port.cs index 3923ff3b8..951887bbd 100644 --- a/src/SSHDebugPS/AD7/AD7Port.cs +++ b/src/SSHDebugPS/AD7/AD7Port.cs @@ -22,8 +22,7 @@ internal abstract class AD7Port : IDebugPort2, IDebugUnixShellPort, IDebugPortCl private Connection _connection; private readonly Dictionary _eventCallbacks = new Dictionary(); private uint _lastCallbackCookie; - private int _sessionRefCount; - private int _activeAsyncCommands; + private int _refCount; protected string Name { get; private set; } @@ -167,24 +166,39 @@ void IDebugUnixShellPort.ExecuteSyncCommand(string commandDescription, string co void IDebugUnixShellPort.BeginExecuteAsyncCommand(string commandText, bool runInShell, IDebugUnixShellCommandCallback callback, out IDebugUnixShellAsyncCommand asyncCommand) { var wrappedCallback = new AsyncCommandCallback(this, callback); - var connection = GetConnection(); lock (_lock) { - _activeAsyncCommands++; + _refCount++; + } + try + { + var connection = GetConnection(); + connection.BeginExecuteAsyncCommand(commandText, runInShell, wrappedCallback, out asyncCommand); + asyncCommand = new AsyncCommandWrapper(wrappedCallback, asyncCommand); + } + catch + { + lock (_lock) + { + _refCount--; + } + CloseConnectionIfIdle(); + throw; } - connection.BeginExecuteAsyncCommand(commandText, runInShell, wrappedCallback, out asyncCommand); - asyncCommand = new AsyncCommandWrapper(this, asyncCommand); } + /// + /// Wraps IDebugUnixShellAsyncCommand so that Abort() triggers NotifyExited on the + /// callback, ensuring the ref count is decremented even when OnExit does not fire. + /// private class AsyncCommandWrapper : IDebugUnixShellAsyncCommand { - private readonly AD7Port _port; + private readonly AsyncCommandCallback _callback; private readonly IDebugUnixShellAsyncCommand _inner; - private int _notified; - public AsyncCommandWrapper(AD7Port port, IDebugUnixShellAsyncCommand inner) + public AsyncCommandWrapper(AsyncCommandCallback callback, IDebugUnixShellAsyncCommand inner) { - _port = port; + _callback = callback; _inner = inner; } @@ -194,11 +208,7 @@ public AsyncCommandWrapper(AD7Port port, IDebugUnixShellAsyncCommand inner) public void Abort() { _inner.Abort(); - // If OnExit didn't fire (abort path), notify the port now - if (Interlocked.CompareExchange(ref _notified, 1, 0) == 0) - { - _port.OnAsyncCommandExited(); - } + _callback.NotifyExited(); } } @@ -206,7 +216,8 @@ private void OnAsyncCommandExited() { lock (_lock) { - _activeAsyncCommands--; + Debug.Assert(_refCount > 0, "Underflowing _refCount"); + _refCount--; } CloseConnectionIfIdle(); } @@ -215,26 +226,29 @@ private void CloseConnectionIfIdle() { lock (_lock) { - if (_sessionRefCount > 0 || _activeAsyncCommands > 0 || _connection == null) + if (_refCount > 0 || _connection == null) { return; } var conn = _connection; _connection = null; - try { conn.Close(); } catch (Exception) { } + try { + conn.Close(); + } + // Dev15 632648: Liblinux sometimes throws exceptions on shutdown - we are shutting down anyways, so ignore to not crash + catch (Exception) { } } } /// - /// Wraps an IDebugUnixShellCommandCallback to detect when an async command exits, - /// allowing the port to close idle connections for engines that do not call - /// IDebugPortCleanup.AddSessionRef/Clean (e.g., vsdbg). + /// Wraps an IDebugUnixShellCommandCallback to track async command exits. /// private class AsyncCommandCallback : IDebugUnixShellCommandCallback { private readonly AD7Port _port; private readonly IDebugUnixShellCommandCallback _inner; + private int _notified; public AsyncCommandCallback(AD7Port port, IDebugUnixShellCommandCallback inner) { @@ -250,7 +264,15 @@ public void OnOutputLine(string line) public void OnExit(string exitCode) { _inner.OnExit(exitCode); - _port.OnAsyncCommandExited(); + NotifyExited(); + } + + public void NotifyExited() + { + if (Interlocked.CompareExchange(ref _notified, 1, 0) == 0) + { + _port.OnAsyncCommandExited(); + } } } @@ -341,7 +363,7 @@ public void AddSessionRef() { lock (_lock) { - _sessionRefCount++; + _refCount++; } EnsureConnected(); } @@ -350,24 +372,10 @@ public void Clean() { lock (_lock) { - Debug.Assert(_sessionRefCount > 0, "Unbalanced call to Clean -- no matching AddSessionRef"); - _sessionRefCount--; - - if (_sessionRefCount > 0) - { - return; - } - - var conn = _connection; - _connection = null; - - try - { - conn?.Close(); - } - // Dev15 632648: Liblinux sometimes throws exceptions on shutdown - we are shutting down anyways, so ignore to not crash - catch (Exception) { } + Debug.Assert(_refCount > 0, "Underflowing _refCount"); + _refCount--; } + CloseConnectionIfIdle(); } } }