SIGN IN SIGN UP

Treat a write losing its connection as a closure, not an internal fault (#3167) (#3189)

Shutdown() discards the output writer, and it cannot wait for the write lock: teardown is
usually spotted on the read loop, which must not block. So a writer already inside the lock
can have the pipe pulled out from under it at any point. That is an ordinary closure, but the
write path reported it as InternalFailure wrapping InvalidOperationException("Output pipe not
initialized") - which reads as a client bug, is not a recognised connection failure, and also
gets announced through ConnectionMultiplexer.InternalError.

IdentifyFailureType already maps ObjectDisposedException to SocketClosed; the write path just
never consulted it. Changes:

- PhysicalConnection keeps an _isShutdown flag, set before the output is discarded, so a writer
  that finds it gone can tell a closure from a genuinely uninitialised pipe: the former now
  throws ObjectDisposedException, the latter still InvalidOperationException. This also removes
  a duplicate copy of that throw in Flush().
- New PhysicalConnection.ClassifyWriteFailure centralises the decision: honour an inner
  RedisConnectionException's own failure type, defer to IdentifyFailureType, and only fall back
  to InternalFailure when there is nothing better. PhysicalBridge (both write-failure paths) and
  Message.WriteTo now use it, and only report OnInternalError when it really was internal.

Two further faults found while reproducing this, both in the same window:

- ProcessBridgeBacklog re-read the nullable `physical` field to pass to WriteMessageInsideLock
  (whose parameter is not nullable) after testing it in the loop guard. OnDisconnected nulls that
  field, so the drain could hand it null and NRE - surfacing as InternalFailure "Failed to write".
  Now snapshotted per message, matching the idiom used elsewhere in the file.
- OperationCanceledException was not classified. RESPite already documents it as expected
  teardown noise alongside ObjectDisposedException, and the read loop treats a cancelled read as
  SocketClosed; the write path now matches, gated on our own OutputCancel having fired so a
  caller's cancellation is not mislabelled.

Tests: a deterministic test that shuts the output down before the write and asserts the failure
is not badged internal, plus an explicit (opt-in) stress test that races real teardowns against
concurrent writers - the latter is what found all three faults, but it has to saturate the box to
hit the window, so it is not part of the default run.
M
Marc Gravell committed
de93da1e35d98416d80dc453f91194e8a3eca547
Parent: 3ecc614
Committed by GitHub <noreply@github.com> on 8/20/2026, 10:39:59 AM