From e9033c3b868fee6dbadadd4fddbc5c250d064aca Mon Sep 17 00:00:00 2001 From: Ralf Becher Date: Thu, 3 Sep 2026 11:39:53 +0200 Subject: [PATCH] fix: don't let one unreachable server stall the whole listener AcceptConnection constructed the TDSConnection inline and only re-armed BeginAcceptTcpClient afterwards. Constructing a connection dials the far server with a blocking Connect, so a server that swallows SYNs held the single in-flight accept for the OS connect timeout - over two minutes on Linux - and every other client sat in the backlog for exactly as long. Observed against a legacy SQL Server that is blackholed nightly: one connection served per 135 seconds, for every client, all night. Three changes: - Re-arm the listener before setting up the connection, so a slow or dead far server no longer serializes accepts. - Bound the dial with a 10 second timeout rather than the OS SYN retry sequence, so a thread is not parked for minutes per attempt. - Roll back construction when the dial fails. The Stopping subscription is what keeps a TDSConnection alive, and it is taken before the dial, so a failed attempt lingered - still holding the accepted client socket - until the service stopped. That is the origin of the hundreds of NullReferenceExceptions from Dispose at shutdown, one per stranded attempt; _insideStream is null-guarded there as well. Keep-alive (#6) does not cover this case: it needs a connection that was established, and here none ever is. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019FMY262wiaqabAsPuQ9b8G --- src/TDSProxy/TDSConnection.cs | 65 +++++++++++++++++++++++++++++++++-- src/TDSProxy/TDSListener.cs | 49 +++++++++++++++++++++----- 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/src/TDSProxy/TDSConnection.cs b/src/TDSProxy/TDSConnection.cs index 1aa3eca..e74cfbe 100644 --- a/src/TDSProxy/TDSConnection.cs +++ b/src/TDSProxy/TDSConnection.cs @@ -492,7 +492,21 @@ public TDSConnection(TDSProxyService service, _insideEP = insideEndPoint; _insideClient = new TcpClient(_insideEP.AddressFamily) {NoDelay = false}; - _insideClient.Connect(insideEndPoint); + + try + { + ConnectWithTimeout(_insideClient, insideEndPoint); + } + catch + { + // The far leg never came up. Undo what this constructor has already done: the + // Stopping subscription above is what keeps the instance alive, so without this + // the failed attempt lingers - still holding the client's socket - until the + // service stops. An hour of an unreachable server strands an hour of attempts. + AbandonBeforeConnected(); + throw; + } + EnableKeepAlive(_insideClient.Client); _insideStream = _insideClient.GetStream(); _insideActiveStream = _insideStream; // Start with plain stream, may upgrade to SSL later @@ -501,6 +515,53 @@ public TDSConnection(TDSProxyService service, _processingTask = ProcessConnection(); } + /// + /// How long to wait for the far server to answer a connection attempt. Left to the OS this + /// is a fixed sequence of SYN retries - over two minutes on Linux - which a blackholed route + /// runs through in full: longer than any client waits, and a parked thread throughout. + /// + static readonly TimeSpan ConnectTimeout = TimeSpan.FromSeconds(10); + + static void ConnectWithTimeout(TcpClient client, IPEndPoint endPoint) + { + using (var cts = new CancellationTokenSource(ConnectTimeout)) + { + try + { + client.ConnectAsync(endPoint.Address, endPoint.Port, cts.Token) + .AsTask() + .GetAwaiter() + .GetResult(); + } + catch (OperationCanceledException) when (cts.IsCancellationRequested) + { + throw new TimeoutException( + $"Timed out after {ConnectTimeout.TotalSeconds:0} seconds connecting to {endPoint}."); + } + } + } + + /// + /// Roll back the part of construction that ran before the far leg was up, so an attempt that + /// never became a connection does not count as active and does not keep itself alive. The + /// outside client belongs to the caller, which closes it. + /// + void AbandonBeforeConnected() + { + _state = StateEnum.Closed; + Interlocked.Decrement(ref ActiveConnectionCount); + _service.Stopping -= service_Stopping; + + try + { + _insideClient.Close(); + } + catch (Exception e) + { + log.Error($"Error closing the inside client for connection from {_outsideEP}", e); + } + } + /// /// Turn on TCP keep-alive so a peer that disappears without FIN or RST is noticed. /// A link that is cut rather than closed leaves a blocking read waiting forever: @@ -560,7 +621,7 @@ void IDisposable.Dispose() try { - _insideStream.Close(); + _insideStream?.Close(); } catch (Exception e) { diff --git a/src/TDSProxy/TDSListener.cs b/src/TDSProxy/TDSListener.cs index 5aff06e..f22d6e7 100644 --- a/src/TDSProxy/TDSListener.cs +++ b/src/TDSProxy/TDSListener.cs @@ -203,9 +203,11 @@ private SslProtocols ParseSslProtocols(string protocols) private void AcceptConnection(IAsyncResult result) { + TcpClient readClient; + try { - TcpClient readClient = ((TcpListener)result.AsyncState).EndAcceptTcpClient(result); + readClient = ((TcpListener)result.AsyncState).EndAcceptTcpClient(result); log.InfoFormat("Accepted connection from {0} on {1}, will forward to {2}", readClient.Client.RemoteEndPoint, readClient.Client.LocalEndPoint, ForwardTo); @@ -215,25 +217,56 @@ private void AcceptConnection(IAsyncResult result) readClient.Close(); return; } - - new TDSConnection(_service, this, readClient, ForwardTo); } - catch (ObjectDisposedException) { /* We're shutting down, ignore */ } + catch (ObjectDisposedException) + { + /* We're shutting down, ignore */ + return; + } catch (Exception e) { - log.Fatal("Error in AcceptConnection.", e); + log.Fatal("Error accepting connection.", e); + return; + } + finally + { + ResumeAccepting(); } - if (!_stopped) + // Setting up the connection dials the far server, and that dial blocks. It has to happen + // after the listener is accepting again: a server that swallows SYNs takes the connect + // timeout to fail, and until this method returned, that was equally how long every other + // client sat in the backlog waiting to be accepted. + try { + new TDSConnection(_service, this, readClient, ForwardTo); + } + catch (Exception e) + { + log.Fatal("Error in AcceptConnection.", e); try { - _tcpListener?.BeginAcceptTcpClient(AcceptConnection, _tcpListener); + readClient.Close(); + } + catch (Exception closeError) + { + log.Error("Error closing a connection that could not be set up.", closeError); } - catch (ObjectDisposedException) { /* We're shutting down, ignore */ } } } + void ResumeAccepting() + { + if (_stopped) + return; + + try + { + _tcpListener?.BeginAcceptTcpClient(AcceptConnection, _tcpListener); + } + catch (ObjectDisposedException) { /* We're shutting down, ignore */ } + } + public void Dispose() { if (!_stopped)