ForwardedPortLocal can stall queued connections behind a keep-alive connection

Aberta
#1,858 0 comentários 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 3 dias

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
3/5
Tempo estimado
1-2 dias
Facilidade para iniciantes
68/100
Tipo de issue
Bug
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
csharp
Domínio
networking

Direção de pesquisa

Read src/Renci.SshNet/ForwardedPortLocal.cs, especially AcceptCompleted and ProcessAccept, then run the supplied QueuedKeepAlive regression command against the unchanged code. Review the proposed dispatch and its connection-lifetime and resource behavior; done means queued connections begin processing before any keep-alive connection closes, with the regression and existing forwarding tests passing.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

Codex identified this problem and proposed a fix to it. I can provide more context if needed. Thank you.

Here is the issue body it generated:

ForwardedPortLocal can stall queued connections behind a keep-alive connection

Version: SSH.NET 2026.0.0, tag commit
7b2fd3dbf2c86a80a7b06cea020aa5f821c9902e. Linux x64. Live reproduction runs
on .NET 8. The source regression targets net8.0 and was executed with .NET 10.

When several TCP connections arrive together, some can remain unprocessed until
another connection closes. This shows up as intermittent browser asset-loading
timeouts. The SSH session remains connected and the forward remains started.
No SshClient.ErrorOccurred or ForwardedPort.Exception event was observed
during the failing live availability requests.

Suspected mechanism, reproduced in an isolated test

ForwardedPortLocal.AcceptCompleted captures a client socket, calls StartAccept,
then calls ProcessAccept. If the next Socket.AcceptAsync completes synchronously,
StartAccept calls AcceptCompleted inline. Several queued sockets therefore
produce nested callbacks. The deepest ProcessAccept blocks in channel.Bind
for the lifetime of its connection, so outer callbacks cannot process the sockets
they have already captured. A browser keeping that connection open can wait for
an asset on one of the unprocessed connections.

The attached ForwardedPortLocalTest_QueuedKeepAlive.cs prequeues three local
connections before starting the accept loop. Mocked IChannelDirectTcpip.Bind
signals that it has been reached and then waits for an explicit release. The test
requires all three channels to begin before any is released. It fails on the
unchanged library and passes with accept-loop.patch. No SSH server, authentication,
remote socket, browser, application lifecycle code or management watchdog is required.
Private-field setup in the test deliberately exercises synchronous acceptance
without depending on a probabilistic connection burst.

Candidate correction

Dispatch ProcessAccept(clientSocket) to the existing
ThreadAbstraction.ExecuteThreadLongRunning helper before calling StartAccept.
This keeps the blocking handler off the accept callback and avoids dependence on
worker-pool growth for persistent connections. This proposal allocates a worker
per accepted connection; please review lifecycle/resource behavior and whether
an asynchronous forwarding design is preferable. It changes no SSH wire protocol,
authentication, host-key validation or cryptographic code.

Validation
  • New regression: unchanged code fails; patched code passes, including five repeats.
  • Existing local-forwarding tests plus new regression: 83 passed.
  • Related forwarding/direct-TCP tests: 354 passed, two pre-existing skips.
  • Independent public-API HTTP test: eight batches of eight keep-alive requests,
    keeping successful connections open while awaiting the rest. Released package:
    all eight batches time out, with only 2–5 replies per batch. Candidate patch:
    all 64 replies arrive within their three-second batch deadlines.
  • Three fresh anonymous browser loads through the patched standalone forward
    complete in 2672, 2673 and 2646 ms. The previous unpatched browser test stalled
    intermittently. No server or application configuration was changed.
  • The temporary forward releases its listener on disposal.
Running the regression

Place the C# file below in test/Renci.SshNet.Tests/Classes/ in a checkout of the
2026.0.0 tag, then run:

dotnet test test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj -p:TargetFrameworks=net8.0 --filter FullyQualifiedName~QueuedKeepAlive

It should fail before the patch and pass afterward. The test uses private-field
setup specifically to exercise a nonempty accept backlog deterministically.
This report does not claim high-load qualification or exhaustive disposal/reconnect testing.

Candidate patch
diff --git a/src/Renci.SshNet/ForwardedPortLocal.cs b/src/Renci.SshNet/ForwardedPortLocal.cs
index 9bf5c058..137c793c 100644
--- a/src/Renci.SshNet/ForwardedPortLocal.cs
+++ b/src/Renci.SshNet/ForwardedPortLocal.cs
@@ -265,11 +265,13 @@ private void AcceptCompleted(object sender, SocketAsyncEventArgs e)
                 return;
             }
 
+            // Processing blocks for the connection lifetime. Never run it on the
+            // accept callback: a synchronous AcceptAsync completion can recurse
+            // and otherwise leave earlier sockets waiting behind a keep-alive.
+            _ = Abstractions.ThreadAbstraction.ExecuteThreadLongRunning(() => ProcessAccept(clientSocket));
+
             // accept new connection
             StartAccept(e);
-
-            // process connection
-            ProcessAccept(clientSocket);
         }
 
         private void ProcessAccept(Socket clientSocket)
Isolated regression test
using System;
using System.Collections.Concurrent;
using System.Net;
using System.Net.Sockets;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;

using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using Renci.SshNet.Channels;

namespace Renci.SshNet.Tests.Classes
{
    [TestClass]
    public class ForwardedPortLocalTest_QueuedKeepAlive
    {
        [TestMethod]
        public async Task QueuedConnectionsMustOpenBeforeAnyKeepAliveConnectionCloses()
        {
            // Prequeue sockets before invoking the accept loop: this deterministically
            // exercises AcceptAsync returning false (synchronous completion).
            const int count = 3;
            using var bound = new CountdownEvent(count);
            using var release = new ManualResetEventSlim();
            var errors = new ConcurrentQueue<Exception>();
            var info = new Mock<IConnectionInfo>();
            info.Setup(x => x.Timeout).Returns(TimeSpan.FromSeconds(2));
            var session = new Mock<ISession>();
            session.Setup(x => x.IsConnected).Returns(true);
            session.Setup(x => x.ConnectionInfo).Returns(info.Object);
            session.Setup(x => x.SessionLoggerFactory).Returns(NullLoggerFactory.Instance);
            session.Setup(x => x.CreateChannelDirectTcpip()).Returns(() =>
            {
                var channel = new Mock<IChannelDirectTcpip>();
                channel.Setup(x => x.Bind()).Callback(() =>
                {
                    bound.Signal();
                    release.Wait(TimeSpan.FromSeconds(10));
                });
                return channel.Object;
            });
            using var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            listener.Bind(new IPEndPoint(IPAddress.Loopback, 0));
            listener.Listen(8);
            var endpoint = (IPEndPoint)listener.LocalEndPoint;
            using var forward = new ForwardedPortLocal("127.0.0.1", (uint)endpoint.Port, "127.0.0.1", 80);
            forward.Session = session.Object;
            forward.Exception += (_, e) => errors.Enqueue(e.Exception);
            // Test-only private setup isolates the accept loop from listener startup timing.
            void Set(string name, object value) => typeof(ForwardedPortLocal)
                .GetField(name, BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(forward, value);
            Set("_listener", listener);
            Set("_status", ForwardedPortStatus.Started);
            Set("_pendingChannelCountdown", new CountdownEvent(1));
            var clients = new Socket[count];
            Task accept = null;
            bool allOpened;
            try
            {
                for (var i = 0; i < count; i++)
                {
                    clients[i] = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
                    clients[i].Connect(endpoint);
                }
                accept = Task.Run(() => typeof(ForwardedPortLocal)
                    .GetMethod("StartAccept", BindingFlags.Instance | BindingFlags.NonPublic)!
                    .Invoke(forward, new object[] { null }));
                allOpened = await Task.Run(() => bound.Wait(TimeSpan.FromSeconds(3)));
            }
            finally
            {
                release.Set();
                foreach (var client in clients) client?.Dispose();
                if (accept != null) await accept.WaitAsync(TimeSpan.FromSeconds(5));
                forward.Stop();
            }
            Assert.IsTrue(allOpened, "Queued connections waited for another connection to close; remaining=" + bound.CurrentCount);
            Assert.AreEqual(0, errors.Count, "Forwarding raised an exception.");
        }
    }
}
Linguagem predominante
C#
Estrelas
4.4k
Forks
987
Merge médio
2d 6h
PRs com merge (30d)
4

Preparar o ambiente

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de sshnet/SSH.NET

Todas as issues de sshnet/SSH.NET

Issues semelhantes

Mais issues de C#

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.