SSH: open a shell off the caller's thread, and stop waiting when the caller gives up

src/DodoSSH.Client.Ssh/SshNetConnectionFactory.cs

@@ -468,7 +468,21 @@ internal sealed class SshNetConnection(
     public string Cipher { get; } = client.ConnectionInfo.CurrentServerEncryption ?? string.Empty;
 
     /// <inheritdoc />
-    public Task<ISshShellSession> OpenShellAsync(TerminalSize size, CancellationToken cancellationToken)
+    /// <remarks>
+    /// <para>
+    /// On the thread pool, because <c>CreateShellStream</c> is synchronous and is three round trips to the
+    /// server — open a channel, ask for a terminal, ask for a shell — and this used to make them on whichever
+    /// thread called it, which is the UI thread. On a distant or overloaded server the window stopped
+    /// painting for as long as they took, and the cancellation token it was given was read once, before any
+    /// of them.
+    /// </para>
+    /// <para>
+    /// SSH.NET's call cannot itself be cancelled, so a cancellation stops the wait rather than the call. A
+    /// shell that arrives after nobody is waiting for it is closed as it lands rather than left holding a
+    /// channel on the server.
+    /// </para>
+    /// </remarks>
+    public async Task<ISshShellSession> OpenShellAsync(TerminalSize size, CancellationToken cancellationToken)
     {
         cancellationToken.ThrowIfCancellationRequested();
 
@@ -476,15 +490,53 @@ internal sealed class SshNetConnection(
 
         // 4 KiB read buffer inside SSH.NET. Output is coalesced a layer up, so a larger buffer here
         // only delays the first byte reaching the screen.
-        var shell = client.CreateShellStream(
+        var opening = Task.Run(
+            () => client.CreateShellStream(
                 "xterm-256color",
                 effective.Columns,
                 effective.Rows,
                 effective.PixelWidth,
                 effective.PixelHeight,
-            4096);
+                4096),
+            CancellationToken.None);
 
-        return Task.FromResult<ISshShellSession>(new SshNetShellSession(shell, client));
+        try
+        {
+            var shell = await opening.WaitAsync(cancellationToken).ConfigureAwait(false);
+
+            return new SshNetShellSession(shell, client);
+        }
+        catch (OperationCanceledException)
+        {
+            _ = CloseWhenItLandsAsync(opening);
+
+            throw;
+        }
+    }
+
+    /// <summary>Closes a shell nobody is waiting for any more, or lets its failure go unremarked.</summary>
+    /// <remarks>
+    /// A shell that failed to open after its caller gave up has nobody to tell, and the connection it was
+    /// on reports its own death through the session that owns it. The failures are the ones SSH.NET raises
+    /// for a channel or a connection that went away, named so that anything else still surfaces.
+    /// </remarks>
+    private static async Task CloseWhenItLandsAsync(Task<ShellStream> opening)
+    {
+        ShellStream shell;
+
+        try
+        {
+            shell = await opening.ConfigureAwait(false);
+        }
+        catch (Exception exception) when (exception is SshException
+            or System.Net.Sockets.SocketException
+            or ObjectDisposedException
+            or InvalidOperationException)
+        {
+            return;
+        }
+
+        await shell.DisposeAsync().ConfigureAwait(false);
     }
 
     /// <inheritdoc />

tests/DodoSSH.Client.Ssh.Tests/KeyAuthenticationTests.cs

@@ -28,6 +28,44 @@ public sealed class KeyAuthenticationTests(SshServerFixture fixture)
 {
     private static CancellationToken Token => TestContext.Current.CancellationToken;
 
+    /// <remarks>
+    /// Opening a shell is three round trips to the server, and SSH.NET makes them synchronously. They used to
+    /// be made on the caller's thread — the UI thread — behind a <c>Task.FromResult</c>, so the task this
+    /// returned was already finished and the window had stopped painting while it was being made.
+    /// </remarks>
+    [Fact]
+    public async Task OpeningAShell_LeavesTheCallingThreadFree()
+    {
+        await using var connection = await ConnectTrustedAsync(
+            new SshPrivateKeyCredential(Pkcs1(fixture.ClientKey), Passphrase: null));
+
+        var opening = connection.OpenShellAsync(TerminalSize.Default, Token);
+
+        opening.IsCompleted.ShouldBeFalse("three round trips to the server cannot have been made on this thread");
+
+        await using var shell = await opening;
+    }
+
+    /// <remarks>
+    /// The cancellation token used to be read once, before any of those round trips. A caller that gives up
+    /// now stops waiting at once, and the connection is left able to open the next shell.
+    /// </remarks>
+    [Fact]
+    public async Task AShellGivenUpOn_StopsTheWait_AndTheConnectionOpensAnother()
+    {
+        await using var connection = await ConnectTrustedAsync(
+            new SshPrivateKeyCredential(Pkcs1(fixture.ClientKey), Passphrase: null));
+
+        using var giveUp = CancellationTokenSource.CreateLinkedTokenSource(Token);
+
+        var opening = connection.OpenShellAsync(TerminalSize.Default, giveUp.Token);
+        await giveUp.CancelAsync();
+
+        await Should.ThrowAsync<OperationCanceledException>(opening);
+
+        await using var shell = await connection.OpenShellAsync(TerminalSize.Default, Token);
+    }
+
     [Fact]
     public async Task AKeyHeldAsBytes_AuthenticatesAndOpensAShell()
     {