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

[mgravell]

Fixes #3167.

The race

PhysicalConnection.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 on it. So a writer already inside the lock can have the pipe pulled out from under it at any point. Every _output read on the write path is therefore a TOCTOU; ProcessBridgeBacklog's HasOutputPipe guard only narrows the window to one message, which is why a 193-deep backlog drain found it in the field.

The race itself isn't fixable by locking. What's wrong is the reporting: a routine closure came out as InternalFailure wrapping InvalidOperationException("Output pipe not initialized") - which reads as a client bug, isn't a recognised connection failure, and is also announced through ConnectionMultiplexer.InternalError.

IdentifyFailureType already maps ObjectDisposedException to SocketClosed; the write path simply never consulted it - WriteMessageToServerInsideWriteLock, HandleWriteException and all three Message.WriteTo catches hardcoded InternalFailure.

Changes

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

All internal - no public API change.

Two further faults found while reproducing it

Both live in the same window, and both surfaced as the same InternalFailure:

1. A real NullReferenceException. 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 - 2-5 per stress run, reported as InternalFailure: "Failed to write". Now snapshotted per message, matching the idiom already used at three other sites in the file.
2. OperationCanceledException wasn't classified. Not speculative: RESPite already documents it as expected teardown noise alongside ObjectDisposedException (BufferedStreamWriter.Switchable.cs), 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 isn't mislabelled.

Behaviour change

Callers who previously saw InternalFailure for these races now see SocketClosed/SocketFailure, and no longer get a spurious InternalError event. Messages are still failed rather than retried - deliberately: the connection is genuinely gone, and per-message resurrection is WithRetry's job, not the bridge's.

Tests

tests/StackExchange.Redis.Tests/Issues/Issue3167Tests.cs:

• WriteLosingItsConnectionIsReportedAsAClosure - deterministic, runs in CI (~6ms). Shuts the output down before the write, then asserts the caller's failure isn't badged internal and no InternalError is raised. It deliberately doesn't demand one exact exception: the message can also be completed by the teardown our own Shutdown kicked off, carrying the underlying socket error.
• WritesRacingTeardownAreNeverInternalFailures - Explicit = true, so opt-in. Races real teardowns against 16 concurrent writers; this is what found all three faults. It has to saturate the box to hit the window reliably, which starves anything running alongside it, so it's kept out of the default run; the docs on it give the invocation.

Verified both fail against unmodified main (Expected: SocketClosed / Actual: InternalFailure, plus 28 output-pipe faults and 35 internal errors in a 10s window) and pass here.

dotnet build Build.csproj -c Release /p:CI=true green across all TFMs; full net10.0 and net8.0 suites green.

One thing worth knowing independently of this PR: the suite has an ambient flake rate of roughly 1 run in 6, spread across a load-sensitive family (DatabaseTests.CountKeys, PubSubKeyNotification*, UnroutableRedirectUnitTests). I confirmed that on unmodified main, not just here. Separately, during teardown storms both main and this branch log InvalidOperationException: Received Interactive/Resp3 response with no message waiting: Null from CommitAndParseFrames a couple of times per run - pre-existing, not chased here.