From 694feb37665483089be3b8c13d6112a3fc9283ed Mon Sep 17 00:00:00 2001 From: Harsh Date: Fri, 9 Oct 2026 19:40:22 +0000 Subject: [PATCH 1/2] Release SshCommand's session subscriptions and channel on completion (#1204) SshCommand subscribed to Session.Disconnected/ErrorOccured in its constructor and, since #1596, created its channel there too and only disposed it in SshCommand.Dispose. A command that is executed but not disposed (e.g. the result of SshClient.RunCommand) was therefore kept alive by the session for the session's lifetime, together with its ChannelSession, which itself stays subscribed to ~12 session events and holds 4 wait handles. With a long-lived SshClient this grows without bound and slows down message dispatch. Now: - the constructor neither creates the channel nor subscribes to the session; ExecuteAsync does both, - the session subscriptions are removed as soon as the execution completes (any completion path), - the channel is disposed as soon as it has been closed at both ends and no thread is blocked in a request on it. A use count around SendExecRequest/SendSignalRequest keeps the #1596 race fixed: if SSH_MSG_CHANNEL_CLOSE is processed before SendExecRequest returns, the dispose is deferred to the executing thread. - if the execution completes while the channel is still open (cancellation without a reply from the server, session error or disconnect), the channel is disposed on a thread-pool thread, since that sends SSH_MSG_CHANNEL_CLOSE and waits up to ChannelCloseTimeout. - if Open/SendExecRequest throws, the channel and subscriptions are released and the instance can be executed again. - the subscriptions and the channel are released, and the output streams disposed, before the task completes, so that this cleanup cannot race with a re-execution started on the task's completion. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FhcS92GWC4Aixeh8JxWMXy --- src/Renci.SshNet/SshCommand.cs | 312 +++++++++++++++--- .../Classes/SshCommandTest_NotDisposed.cs | 248 ++++++++++++++ 2 files changed, 505 insertions(+), 55 deletions(-) create mode 100644 test/Renci.SshNet.Tests/Classes/SshCommandTest_NotDisposed.cs diff --git a/src/Renci.SshNet/SshCommand.cs b/src/Renci.SshNet/SshCommand.cs index ce1042244..eb06d19b7 100644 --- a/src/Renci.SshNet/SshCommand.cs +++ b/src/Renci.SshNet/SshCommand.cs @@ -6,6 +6,7 @@ using System.Threading; using System.Threading.Tasks; +using Renci.SshNet.Abstractions; using Renci.SshNet.Channels; using Renci.SshNet.Common; using Renci.SshNet.Messages.Connection; @@ -21,7 +22,37 @@ public sealed class SshCommand : IDisposable private readonly ISession _session; private readonly Encoding _encoding; - private IChannelSession _channel; + /// + /// Guards , , + /// and . + /// + private readonly Lock _channelLock = new Lock(); + + /// + /// The channel of the current execution, or before the first execution + /// and after the channel of a completed execution has been released. + /// + private IChannelSession? _channel; + + /// + /// The number of threads currently blocked in a request on + /// ( or + /// ). The channel is not disposed + /// while this is non-zero, because disposing it would pull the wait handle out from under + /// the waiting thread (see #1596). + /// + private int _channelUseCount; + + /// + /// Whether has been closed at both ends. + /// + private bool _channelClosed; + + /// + /// Whether the execution that belongs to has completed. + /// + private bool _executionCompleted; + private TaskCompletionSource? _tcs; private CancellationTokenSource? _cts; private CancellationTokenRegistration _tokenRegistration; @@ -142,7 +173,9 @@ public int? ExitStatus /// public Stream CreateInputStream() { - if (!_channel.IsOpen) + var channel = _channel; + + if (channel is not { IsOpen: true }) { throw new InvalidOperationException("The input stream can be used only during execution."); } @@ -152,7 +185,7 @@ public Stream CreateInputStream() throw new InvalidOperationException("The input stream already exists."); } - _inputStream = new ChannelInputStream(_channel); + _inputStream = new ChannelInputStream(channel); return _inputStream; } @@ -224,9 +257,10 @@ internal SshCommand(ISession session, string commandText, Encoding encoding) CommandTimeout = Timeout.InfiniteTimeSpan; OutputStream = new PipeStream(); ExtendedOutputStream = new PipeStream(); - _session.Disconnected += Session_Disconnected; - _session.ErrorOccured += Session_ErrorOccurred; - _channel = _session.CreateChannelSession(); + + // Note: we deliberately do not subscribe to session events or create the channel here. + // Both are done in ExecuteAsync and undone when the execution completes, so that an + // SshCommand which is not disposed is not kept alive by a long-lived session (#1204). } /// @@ -258,7 +292,9 @@ public Task ExecuteAsync(CancellationToken cancellationToken = default) throw new InvalidOperationException("Asynchronous operation is already in progress."); } - UnsubscribeFromChannelEvents(dispose: true); + // The channel of the previous execution has been released when that execution + // completed; this is just a safety net. + ReleaseChannel(force: true); OutputStream.Dispose(); ExtendedOutputStream.Dispose(); @@ -268,7 +304,6 @@ public Task ExecuteAsync(CancellationToken cancellationToken = default) // so we just need to reinitialise them for subsequent executions. OutputStream = new PipeStream(); ExtendedOutputStream = new PipeStream(); - _channel = _session.CreateChannelSession(); } _exitStatus = default; @@ -283,16 +318,55 @@ public Task ExecuteAsync(CancellationToken cancellationToken = default) _cts = null; _cancellationRequested = false; + var channel = _session.CreateChannelSession(); + _tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _userToken = cancellationToken; - _channel.DataReceived += Channel_DataReceived; - _channel.ExtendedDataReceived += Channel_ExtendedDataReceived; - _channel.RequestReceived += Channel_RequestReceived; - _channel.Closed += Channel_Closed; - _channel.Open(); + lock (_channelLock) + { + _channel = channel; + _channelClosed = false; + _executionCompleted = false; + + // This thread is about to block in SendExecRequest; don't let the channel be + // disposed underneath it if SSH_MSG_CHANNEL_CLOSE is processed first (#1596). + _channelUseCount = 1; + } + + channel.DataReceived += Channel_DataReceived; + channel.ExtendedDataReceived += Channel_ExtendedDataReceived; + channel.RequestReceived += Channel_RequestReceived; + channel.Closed += Channel_Closed; + + // Subscribe after the channel has been created so that the channel's own + // session event handlers run before ours. + SubscribeToSessionEvents(); + + try + { + channel.Open(); + + _ = channel.SendExecRequest(CommandText); + } + catch (Exception ex) + { + // The command could not be started. Don't leave the channel and the session + // subscriptions behind, and don't leave this instance stuck "in progress". + _ = _tcs.TrySetException(ex); + _ = _tcs.Task.Exception; // Observed: the exception is rethrown to the caller below. + + UnsubscribeFromSessionEvents(); + EndChannelUse(channel); + ReleaseChannel(force: true); - _ = _channel.SendExecRequest(CommandText); + OutputStream.Dispose(); + ExtendedOutputStream.Dispose(); + + throw; + } + + EndChannelUse(channel); if (CommandTimeout != Timeout.InfiniteTimeSpan) { @@ -453,13 +527,24 @@ public void CancelAsync(bool forceKill = false, int millisecondsTimeout = 500) try { - // Try to send the cancellation signal. - if (_channel?.SendSignalRequest(forceKill ? "KILL" : "TERM") is null) + var channel = BeginChannelUse(); + + if (channel is null) { // Command has completed (in the meantime since the last check). return; } + try + { + // Try to send the cancellation signal. + _ = channel.SendSignalRequest(forceKill ? "KILL" : "TERM"); + } + finally + { + EndChannelUse(channel); + } + // Having sent the "signal" message, we expect to receive "exit-signal" // and then a close message. But since a server may not implement signals, // we can't guarantee that, so we wait a short time for that to happen and @@ -507,55 +592,71 @@ public string Execute(string commandText) private void Session_Disconnected(object? sender, EventArgs e) { - _ = _tcs?.TrySetException(new SshConnectionException("An established connection was aborted by the software in your host machine.", DisconnectReason.ConnectionLost)); - - SetAsyncComplete(setResult: false); + SetAsyncComplete(new SshConnectionException("An established connection was aborted by the software in your host machine.", DisconnectReason.ConnectionLost)); } private void Session_ErrorOccurred(object? sender, ExceptionEventArgs e) { - _ = _tcs?.TrySetException(e.Exception); - - SetAsyncComplete(setResult: false); + SetAsyncComplete(e.Exception); } - private void SetAsyncComplete(bool setResult = true) + private void SetAsyncComplete(Exception? exception = null) { - Interlocked.MemoryBarrier(); // ensure fresh read of _cancellationRequested (possibly unnecessary) + // Clean up before completing the task: once the task has completed, the instance may be + // executed again (e.g. on a continuation), and this cleanup must not then affect the + // session subscriptions or output streams of that new execution. - if (setResult) - { - Debug.Assert(_tcs is not null, "Should only be completing the task if we've started one."); + // The execution is over: stop listening to the session so that the session (which + // typically lives much longer than a command) no longer references this instance. + UnsubscribeFromSessionEvents(); - if (_userToken.IsCancellationRequested) - { - _ = _tcs.TrySetCanceled(_userToken); - } - else if (_cts?.Token.IsCancellationRequested == true) - { - _ = _tcs.TrySetException(new SshOperationTimeoutException($"Command '{CommandText}' timed out. ({nameof(CommandTimeout)}: {CommandTimeout}).")); - } - else if (_cancellationRequested) - { - _ = _tcs.TrySetCanceled(); - } - else - { - _ = _tcs.TrySetResult(null!); - } + // Release the channel unless a thread is blocked in a request on it, in which case that + // thread releases it when the request returns (see EndChannelUse). + lock (_channelLock) + { + _executionCompleted = true; } - // We don't dispose the channel here to avoid a race condition - // where SSH_MSG_CHANNEL_CLOSE arrives before _channel starts - // waiting for a response in _channel.SendExecRequest(). - UnsubscribeFromChannelEvents(dispose: false); + ReleaseChannel(force: false); OutputStream.Dispose(); ExtendedOutputStream.Dispose(); + + if (exception is not null) + { + _ = _tcs?.TrySetException(exception); + return; + } + + Interlocked.MemoryBarrier(); // ensure fresh read of _cancellationRequested (possibly unnecessary) + + Debug.Assert(_tcs is not null, "Should only be completing the task if we've started one."); + + if (_userToken.IsCancellationRequested) + { + _ = _tcs.TrySetCanceled(_userToken); + } + else if (_cts?.Token.IsCancellationRequested == true) + { + _ = _tcs.TrySetException(new SshOperationTimeoutException($"Command '{CommandText}' timed out. ({nameof(CommandTimeout)}: {CommandTimeout}).")); + } + else if (_cancellationRequested) + { + _ = _tcs.TrySetCanceled(); + } + else + { + _ = _tcs.TrySetResult(null!); + } } private void Channel_Closed(object? sender, ChannelEventArgs e) { + lock (_channelLock) + { + _channelClosed = true; + } + SetAsyncComplete(); } @@ -596,13 +697,96 @@ private void Channel_DataReceived(object? sender, ChannelDataEventArgs e) OutputStream.Write(e.Data.Array!, e.Data.Offset, e.Data.Count); } + private void SubscribeToSessionEvents() + { + // Guard against a double subscription should a previous execution not have unsubscribed. + UnsubscribeFromSessionEvents(); + + _session.Disconnected += Session_Disconnected; + _session.ErrorOccured += Session_ErrorOccurred; + } + + private void UnsubscribeFromSessionEvents() + { + _session.Disconnected -= Session_Disconnected; + _session.ErrorOccured -= Session_ErrorOccurred; + } + /// - /// Unsubscribes the current from channel events, and optionally, - /// disposes . + /// Marks the start of a blocking request on the current channel. /// - private void UnsubscribeFromChannelEvents(bool dispose) + /// + /// The channel, or if there is no channel or it has already been closed. + /// Each non-null return must be paired with a call to . + /// + private IChannelSession? BeginChannelUse() { - var channel = _channel; + lock (_channelLock) + { + if (_channel is null || _channelClosed) + { + return null; + } + + _channelUseCount++; + return _channel; + } + } + + /// + /// Marks the end of a blocking request on , and releases the channel + /// if the execution has completed in the meantime. + /// + /// The channel returned by . + private void EndChannelUse(IChannelSession channel) + { + lock (_channelLock) + { + if (!ReferenceEquals(channel, _channel)) + { + // That channel has already been released (forcibly, by Dispose or a subsequent + // execution); the count now belongs to another channel (or none). + return; + } + + Debug.Assert(_channelUseCount > 0, "EndChannelUse without matching use."); + + _channelUseCount--; + } + + ReleaseChannel(force: false); + } + + /// + /// Unsubscribes from the events of , disposes it and drops the reference to it. + /// + /// + /// to only release the channel when its execution has completed and no + /// thread is blocked in a request on it; to release it unconditionally + /// (and synchronously). + /// + private void ReleaseChannel(bool force) + { + IChannelSession? channel; + bool channelClosed; + + lock (_channelLock) + { + channel = _channel; + + if (channel is null) + { + return; + } + + if (!force && (!_executionCompleted || _channelUseCount > 0)) + { + return; + } + + channelClosed = _channelClosed; + _channel = null; + } // unsubscribe from events as we do not want to be signaled should these get fired // during the dispose of the channel @@ -611,10 +795,29 @@ private void UnsubscribeFromChannelEvents(bool dispose) channel.RequestReceived -= Channel_RequestReceived; channel.Closed -= Channel_Closed; - if (dispose) + // Disposing the channel unsubscribes it from the session's events and releases its + // wait handles. If the channel has been closed this does not send any message nor block. + if (force || channelClosed) { channel.Dispose(); + return; } + + // The execution completed while the channel is still open (cancellation without a reply + // from the server, session error/disconnect). Disposing it then sends SSH_MSG_CHANNEL_CLOSE + // and waits up to ConnectionInfo.ChannelCloseTimeout for the server to close it, which we + // don't want to do on the thread completing the execution (possibly the message loop). + ThreadAbstraction.ExecuteThread(() => + { + try + { + channel.Dispose(); + } + catch + { + // Swallow exceptions which would otherwise be unhandled. + } + }); } /// @@ -641,12 +844,11 @@ private void Dispose(bool disposing) { // unsubscribe from session events to ensure other objects that we're going to dispose // are not accessed while disposing - _session.Disconnected -= Session_Disconnected; - _session.ErrorOccured -= Session_ErrorOccurred; + UnsubscribeFromSessionEvents(); // unsubscribe from channel events to ensure other objects that we're going to dispose // are not accessed while disposing - UnsubscribeFromChannelEvents(dispose: true); + ReleaseChannel(force: true); _inputStream?.Dispose(); _inputStream = null; diff --git a/test/Renci.SshNet.Tests/Classes/SshCommandTest_NotDisposed.cs b/test/Renci.SshNet.Tests/Classes/SshCommandTest_NotDisposed.cs new file mode 100644 index 000000000..0eaf678ec --- /dev/null +++ b/test/Renci.SshNet.Tests/Classes/SshCommandTest_NotDisposed.cs @@ -0,0 +1,248 @@ +using System; +using System.Globalization; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading; + +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Moq; + +using Renci.SshNet.Channels; +using Renci.SshNet.Common; +using Renci.SshNet.Tests.Common; + +namespace Renci.SshNet.Tests.Classes +{ + /// + /// Regression tests for https://github.com/sshnet/SSH.NET/issues/1204: an + /// that completed but was not disposed must not be kept alive by the (long-lived) session, and + /// must not keep its channel subscribed to the session. + /// + [TestClass] + public class SshCommandTest_NotDisposed : TestBase + { + private Mock _sessionMock; + private Mock _channelSessionMock; + private string _commandText; + + protected override void OnInit() + { + base.OnInit(); + + _sessionMock = new Mock(MockBehavior.Strict); + _sessionMock.Setup(p => p.SessionLoggerFactory).Returns(NullLoggerFactory.Instance); + _channelSessionMock = new Mock(MockBehavior.Strict); + _commandText = new Random().Next().ToString(CultureInfo.InvariantCulture); + } + + [TestMethod] + public void ConstructorShouldNotCreateChannelNorSubscribeToSession() + { + // Strict mock: CreateChannelSession has not been set up, so calling it would throw. + _ = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + + _sessionMock.Verify(p => p.CreateChannelSession(), Times.Never); + } + + [TestMethod] + public void CompletedCommandShouldReleaseChannelWithoutDispose() + { + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()); + _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()); + + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + var asyncResult = sshCommand.BeginExecute(); + + _channelSessionMock.Verify(p => p.Dispose(), Times.Never); + + _channelSessionMock.Raise(c => c.Closed += null, new ChannelEventArgs(5)); + + _ = sshCommand.EndExecute(asyncResult); + + // The channel was released as soon as it was closed at both ends. + _channelSessionMock.Verify(p => p.Dispose(), Times.Once); + + // Disposing afterwards must not dispose the channel again. + sshCommand.Dispose(); + _channelSessionMock.Verify(p => p.Dispose(), Times.Once); + } + + [TestMethod] + public void ChannelClosedBeforeSendExecRequestReturnsShouldNotDisposeChannelUnderneathIt() + { + // Reproduces the race fixed by #1596: SSH_MSG_CHANNEL_CLOSE is processed on the + // message loop thread while the executing thread is still in SendExecRequest. + var disposedDuringSendExecRequest = false; + + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()); + _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)) + .Returns(true) + .Callback(() => + { + _channelSessionMock.Raise(c => c.Closed += null, new ChannelEventArgs(5)); + + disposedDuringSendExecRequest = _channelSessionMock.Invocations.Any(i => i.Method.Name == nameof(IDisposable.Dispose)); + }); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()); + + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + var asyncResult = sshCommand.BeginExecute(); + _ = sshCommand.EndExecute(asyncResult); + + Assert.IsFalse(disposedDuringSendExecRequest); + _channelSessionMock.Verify(p => p.Dispose(), Times.Once); + } + + [TestMethod] + public void CompletedCommandShouldNotBeKeptAliveBySession() + { + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()); + _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()); + + var weakReference = ExecuteAndForget(); + + // Moq records invocations (including the event handler delegates passed to add_/remove_ + // accessors), which would otherwise keep the command alive. The event handlers that are + // still subscribed are stored separately and are not cleared by this - which is exactly + // what this test is about. + _sessionMock.Invocations.Clear(); + _channelSessionMock.Invocations.Clear(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Assert.IsFalse(weakReference.IsAlive, "The completed SshCommand is still referenced (by the session)."); + GC.KeepAlive(_sessionMock.Object); + } + + [TestMethod] + public void FailureToStartShouldReleaseChannelAndAllowReexecution() + { + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()).Throws(new SshException("open failed")); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()); + + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + + _ = Assert.ThrowsExactly(sshCommand.BeginExecute); + + _channelSessionMock.Verify(p => p.Dispose(), Times.Once); + + // Previously the instance was left "in progress" forever. + var channelSessionBMock = new Mock(MockBehavior.Strict); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(channelSessionBMock.Object); + channelSessionBMock.InSequence(seq).Setup(p => p.Open()); + channelSessionBMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)).Returns(true); + + _ = sshCommand.BeginExecute(); + } + + [TestMethod] + public void CancelledCommandWhoseChannelIsNotClosedShouldReleaseChannel() + { + using var channelDisposed = new ManualResetEventSlim(); + + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()); + _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.SendSignalRequest("TERM")).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()).Callback(channelDisposed.Set); + + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + var executeTask = sshCommand.ExecuteAsync(); + + // The server does not reply to the signal and does not close the channel. + sshCommand.CancelAsync(millisecondsTimeout: 10); + + Assert.IsTrue(executeTask.IsCanceled); + Assert.IsTrue(channelDisposed.Wait(TimeSpan.FromSeconds(5)), "The channel was not released."); + } + + [TestMethod] + public void SessionErrorShouldReleaseChannel() + { + using var channelDisposed = new ManualResetEventSlim(); + var exception = new SshException("session error"); + + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()); + _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()).Callback(channelDisposed.Set); + + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + var executeTask = sshCommand.ExecuteAsync(); + + _sessionMock.Raise(s => s.ErrorOccured += null, new ExceptionEventArgs(exception)); + + Assert.AreSame(exception, executeTask.Exception?.InnerException); + Assert.IsTrue(channelDisposed.Wait(TimeSpan.FromSeconds(5)), "The channel was not released."); + } + + [TestMethod] + public void CancelledCommandWhoseChannelIsNotClosedShouldNotBeKeptAliveBySession() + { + using var channelDisposed = new ManualResetEventSlim(); + + var seq = new MockSequence(); + _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object); + _channelSessionMock.InSequence(seq).Setup(p => p.Open()); + _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText)).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.SendSignalRequest("TERM")).Returns(true); + _channelSessionMock.InSequence(seq).Setup(p => p.Dispose()).Callback(channelDisposed.Set); + + var weakReference = ExecuteCancelAndForget(); + + Assert.IsTrue(channelDisposed.Wait(TimeSpan.FromSeconds(5)), "The channel was not released."); + + // See CompletedCommandShouldNotBeKeptAliveBySession. + _sessionMock.Invocations.Clear(); + _channelSessionMock.Invocations.Clear(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Assert.IsFalse(weakReference.IsAlive, "The cancelled SshCommand is still referenced (by the session or its channel)."); + GC.KeepAlive(_sessionMock.Object); + GC.KeepAlive(_channelSessionMock.Object); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private WeakReference ExecuteCancelAndForget() + { + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + _ = sshCommand.ExecuteAsync(); + sshCommand.CancelAsync(millisecondsTimeout: 10); + + // Deliberately not disposed. + return new WeakReference(sshCommand); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private WeakReference ExecuteAndForget() + { + var sshCommand = new SshCommand(_sessionMock.Object, _commandText, Encoding.UTF8); + var asyncResult = sshCommand.BeginExecute(); + _channelSessionMock.Raise(c => c.Closed += null, new ChannelEventArgs(5)); + _ = sshCommand.EndExecute(asyncResult); + + // Deliberately not disposed. + return new WeakReference(sshCommand); + } + } +} From f5ac6879d2228cf0235308cf21fc6839798903b3 Mon Sep 17 00:00:00 2001 From: Harsh <24m0784@iitb.ac.in> Date: Sat, 10 Oct 2026 08:40:05 +0530 Subject: [PATCH 2/2] Fix two flaky integration tests Ssh_RemotePortForwarding: s.Expect("ABC"/"DEF") only matches the terminal echo of the line typed into telnet, not its delivery through the forwarded port. The shell (and with it telnet) was then closed immediately, and the listeners were only waited on afterwards, for 1s. Wait for each listener to receive its bytes before closing the shell that drives its telnet session. Common_DetectSessionKilledOnServer: Session.RaiseError raises ErrorOccurred on the message listener thread before it disconnects the session, so IsConnected can still be true right after the event is signaled. Wait (bounded) for IsConnected to become false. Both tests have also failed intermittently on unrelated branches. Co-Authored-By: Claude Opus 5.5 --- test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs | 5 ++++- test/Renci.SshNet.IntegrationTests/SshTests.cs | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs b/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs index 4791c188e..310810d46 100644 --- a/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs +++ b/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs @@ -404,7 +404,10 @@ public void Common_DetectSessionKilledOnServer() Assert.AreEqual(typeof(SshConnectionException), errorOccurred.GetType()); Assert.IsNull(errorOccurred.InnerException); Assert.AreEqual("An established connection was aborted by the server.", errorOccurred.Message); - Assert.IsFalse(client.IsConnected); + + // ErrorOccurred is raised on the message listener thread before the session disconnects, + // so IsConnected may still be true for a moment after it is signaled. + Assert.IsTrue(SpinWait.SpinUntil(() => !client.IsConnected, TimeSpan.FromSeconds(5))); } } diff --git a/test/Renci.SshNet.IntegrationTests/SshTests.cs b/test/Renci.SshNet.IntegrationTests/SshTests.cs index 2c9dc5655..6c57256ee 100644 --- a/test/Renci.SshNet.IntegrationTests/SshTests.cs +++ b/test/Renci.SshNet.IntegrationTests/SshTests.cs @@ -682,6 +682,10 @@ public void Ssh_RemotePortForwarding() s.WriteLine("ABC"); s.Flush(); s.Expect("ABC"); + + // The echo of "ABC" only means telnet has read the line: wait for it to reach the + // listener before closing the shell (which terminates telnet). + areBytesReceivedOnListener1 = bytesReceivedEventOnListener1.WaitOne(TimeSpan.FromSeconds(10)); s.Close(); } @@ -692,12 +696,11 @@ public void Ssh_RemotePortForwarding() s.WriteLine("DEF"); s.Flush(); s.Expect("DEF"); + + areBytesReceivedOnListener2 = bytesReceivedEventOnListener2.WaitOne(TimeSpan.FromSeconds(10)); s.Close(); } - areBytesReceivedOnListener1 = bytesReceivedEventOnListener1.WaitOne(1000); - areBytesReceivedOnListener2 = bytesReceivedEventOnListener2.WaitOne(1000); - forwardedPort1.Stop(); forwardedPort2.Stop(); }