From 5e231ad1ec12d3d2dbaea6de421d2a600d2f9d96 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Wed, 19 Aug 2026 07:34:47 +0100 Subject: [PATCH 1/2] Transport mode: start reading before the handshake is written A transport is already connected when ConnectedAsync adopts it, and OnConnectedAsync sends the handshake. Until now the receiver was attached afterwards -- BeginConnectAsync reaches StartReading only once ConnectedAsync has returned -- so the reply could be on the wire before Start, and the transport had no way to know that would happen. It is not a theoretical window. Against a transport-backed multiplexer every SUBSCRIBE failed with "no connection became available": the subscription connection lost that race every time, while the interactive connection happened to win it, so ordinary command traffic looked perfectly healthy. The bytes were the handshake reply. StartTransportReading moves up to immediately after InitTransportOutput and becomes idempotent, since StartReading still calls it on the path every connection takes. This also makes DuplexTransport.Start's contract true as written -- "exactly one receiver, set once, before any data is expected" -- rather than a promise each implementer discovers is conditional and has to work around by staging bytes. Verified cross-repo against a real transport implementation (SocketSet's tunnel) and a real redis-server, as a discriminating pair: with that implementation's own staging workaround REMOVED, pub/sub through the tunnel passes with this change and fails without it. Nothing in this repo's tests touches the transport API, so there is no unit test to add here. Claude-Session: https://claude.ai/code/session_01QHTFkVFxokbBCukuUnkhGe --- .../PhysicalConnection.Transport.cs | 9 +++++++++ src/StackExchange.Redis/PhysicalConnection.cs | 12 ++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/StackExchange.Redis/PhysicalConnection.Transport.cs b/src/StackExchange.Redis/PhysicalConnection.Transport.cs index a0258166a..9b7a53dc9 100644 --- a/src/StackExchange.Redis/PhysicalConnection.Transport.cs +++ b/src/StackExchange.Redis/PhysicalConnection.Transport.cs @@ -37,8 +37,17 @@ private void InitTransportOutput(DuplexTransport transport) #endif } + private bool _transportReadingStarted; + + /// Attach the receiver and begin inbound delivery. Idempotent because it is now called + /// from TWO places: as soon as the transport is adopted (before the handshake is written, which is + /// the point), and from , which every connection still goes through and + /// which must not start a second receiver. private void StartTransportReading(DuplexTransport transport) { + if (_transportReadingStarted) return; + _transportReadingStarted = true; + _readStatus = ReadStatus.Init; _readState = default; _readBuffer = CycleBuffer.Create(pool: ReaderBufferPool); diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 0d93eefeb..54af29951 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -1053,6 +1053,18 @@ internal async ValueTask ConnectedAsync(Socket? socket, ILogger? log) } InitTransportOutput(transport); + + // Start inbound delivery BEFORE anything is written. OnConnectedAsync below sends the + // handshake, and a transport is already connected by the time we get here, so if the + // receiver were attached afterwards (as it was until now, via StartReading once this + // method returned) the reply could be on the wire first. That is not theoretical: it + // cost every SUBSCRIBE on a transport-backed multiplexer, because the subscription + // connection lost that race every time while the interactive one happened to win it. + // Starting first also makes DuplexTransport.Start's contract - a receiver set "before + // any data is expected" - true, rather than something each implementer must discover + // and work around by buffering. + StartTransportReading(transport); + log?.LogInformationTransportConnected(bridge.Name, transport.IsEncrypted); await bridge.OnConnectedAsync(this, log).ForAwait(); return true; From ea4d76dcc3419eac2527f7777b1611d08c3227a0 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Wed, 19 Aug 2026 07:35:21 +0100 Subject: [PATCH 2/2] A tunnel that refuses a transport should say why ConnectTransportAsync is called before the try in BeginConnectAsync is entered, and that method runs as BeginConnectAsync(log).RedisFireAndForget() -- so an exception out of a tunnel went nowhere at all. The caller waited out the full connect budget and got the generic "It was not possible to connect", with the tunnel's own explanation lost. That explanation is the whole value of throwing there: a transport refuses a dial when the configuration asks for something it cannot do, and the message names it. The case that showed this up was a tunnel told Ssl=true with no TLS provider configured; the diagnosis it produced instead read as a hang. Wrapping the acquisition and recording it via RecordConnectionFailed puts the reason in the connect log and the ConnectionFailed event, where the socket path's failures already appear. It does not make the refusal faster -- the bridge still retries to its budget -- it makes it explicable. Verified as a pair against a tunnel that refuses: with this change the tunnel's own sentence is in the connect log; without it the log carries only the timeout. Claude-Session: https://claude.ai/code/session_01QHTFkVFxokbBCukuUnkhGe --- src/StackExchange.Redis/PhysicalConnection.cs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 54af29951..17caff418 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -184,7 +184,23 @@ internal async Task BeginConnectAsync(ILogger? log) // connectTo=null no-socket pattern): connect and TLS belong to the tunnel, and the // stream/SslStream machinery below never runs. The TLS intent goes with it precisely // because TLS is now the tunnel's job: it cannot honour an intent it cannot see. - var transport = await tunnel.ConnectTransportAsync(endpoint, bridge.ConnectionType, new(rawConfig), CancellationToken.None).ForAwait(); + RESPite.Transports.DuplexTransport? transport; + try + { + transport = await tunnel.ConnectTransportAsync(endpoint, bridge.ConnectionType, new(rawConfig), CancellationToken.None).ForAwait(); + } + catch (Exception ex) + { + // A tunnel refuses a dial by throwing, and the message is the useful part: it is where + // "this configuration asks for something this transport cannot do" gets said. This + // method runs fire-and-forget (PhysicalBridge calls it as BeginConnectAsync(log) + // .RedisFireAndForget()) and the try below has not been entered yet, so an escaping + // exception was reported nowhere - the caller saw a bare connect timeout, and the + // reason was lost. + RecordConnectionFailed(ConnectionFailureType.UnableToConnect, ex, isInitialConnect: true); + return; + } + if (transport is not null) { _transport = transport;