ForwardedPortLocal can stall queued connections behind a keep-alive connection

未關閉
#1,858 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視

維護者通常 3 天內回覆

還沒有人認領這個 Issue。

評估

難度
3/5
預估耗時
1-2 天
新手友好度
68/100
Issue 類型
缺陷
描述清晰度
描述清楚
活躍度
活躍
技術堆疊
csharp
領域
networking

研究方向

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.

由索引模型根據 Issue 內容生成。

描述

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.");
        }
    }
}
主要語言
C#
星號
4.4k
分支
987
平均合併
2 天 6 小時
30 天內合併 PR
4

環境準備

  • 沒有 Dockerfile 或 Docker Compose 檔案
  • 沒有 Pull Request 範本
  • 閱讀貢獻指南

從這裡開始

  1. 先讀完整個 Issue,再讀專案的貢獻指南。
  2. 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
  3. Fork 儲存庫,在一個分支上完成修改。
  4. 送出 Pull Request,並在描述裡引用這個 Issue 編號。

sshnet/SSH.NET 的其他 Issue

查看 sshnet/SSH.NET 的全部 Issue

相似的 Issue

更多 C# Issue

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。