Do not assume GetSpan honoured the size hint - #3220

Fixes the intermittent ArgumentOutOfRangeException from MessageWriter.WriteUnifiedSpan reported in
this gist — and, while there, a
silent buffer overrun on a neighbouring path.

The bug

IBufferWriter<T>.GetSpan(sizeHint) guarantees one element, not sizeHint. The "should be at least
this size" wording in the docs is left over from when the parameter was called minSize; it is advisory,
and consuming code is expected to test what it got and fall back — which is exactly what the BCL's own
BuffersExtensions does, looping and copying in slices rather than demanding one contiguous block. See
CommunityToolkit/dotnet#1208.

MessageWriter treated it as a guarantee throughout: take the span, write the full amount into it,
advance by that amount.

The looser contract exists for transports with page limits, which can honour reasonable requests and
refuse excessive ones. CycleBuffer is one — it caps the hint at 1k, sizes a fresh segment from the
committed total rather than from the hint, and, the case with no floor at all, hands back a dangling
recycled segment exactly as it is, whatever length that happens to be:

var spare = endSegment.Next;
if (spare is not null) { ... return spare; }   // whatever size it happens to be

All legitimate. The callers were wrong.

That intermittency is specific to CycleBuffer — the recycled-spare path is what makes the reported
failure show up under concurrency rather than every time. It is worth separating out, because the other
under-delivering writer is not intermittent at all. BlockBuffer.GetBuffer clamps the hint to [16, 128]
and then returns whatever is left in the block, and says so itself:

// note this isn't an actual 'max', just a max of what we guarantee;
// we give the caller whatever is left in the buffer

So on BlockBuffer this is a straightforward capacity bug: any request above 128 is under-served whenever
the block has between 128 and 527 bytes left, which covers the whole quick-span path. BlockBufferUnderDeliversWithoutAnyContrivedWriter exercises it through TestHarness.Write with no synthetic writer
involved at all.

On main, that test does not throw — the test host dies with exit code 139 (SIGSEGV), reproduced
twice. That is the "silent overrun" characterisation, executed rather than reasoned.

The reported exception is the benign case

Every fixed-size write had the same shape, and one of them is worse than an exception:

var span = writer.GetSpan(expectedLength);
fixed (byte* bPtr = &MemoryMarshal.GetReference(span))
    Encoding.UTF8.GetBytes(cPtr, value.Length, bPtr, expectedLength);
writer.Advance(expectedLength);

expectedLength is passed to the encoder as the destination capacity rather than being derived from the
span, so against a short span that writes past the end of memory it does not own — silently. It now
falls through to the encoder loop immediately below it, which sizes every write from span.Length and
already copes with whatever it is given.

Shape of the fix

The hot sites check the length inline and hand the rare case to a separate NoInlining method that
composes in a stack buffer and writes through BuffersExtensions.Write.

That split is load-bearing rather than stylistic. A stackalloc in the caller's cold branch changes
codegen for the whole method: writing the fallbacks inline cost 20% on the simplest command shape. It
is the same "inline-optimized for it-fits, pathological case doesn't inline" structure the BCL uses, and
the issue above describes. The genuinely cold sites (WriteUnifiedUInt64, WriteUnifiedDouble,
WriteInteger, the SHA1 path) keep a PrefixScratch helper, which slices both spans to the requested
length so it bounds-checks in Release rather than relying on a Debug.Assert.

Sites: both WriteHeader overloads, both WriteMultiBulkHeader overloads, WriteUnifiedPrefixedString,
WriteUnifiedPrefixedBlob, WriteUnifiedInt64, WriteUnifiedUInt64, WriteUnifiedDouble,
WriteInteger, WriteUnifiedSequenceIterator, the SHA1 bulk-string path, WriteUnifiedSpan and
WriteCrlf. The encoder loop was already correct; the commented-out WriteUnifiedSequence is untouched.

Also fixed while in there

• Length prefixes are sized for an int64. WriteMultiBulkHeader(long), WriteUnifiedPrefixedBlob
  (prefix.LongLength + value.LongLength) and WriteUnifiedSequenceIterator all format a long but asked
  for 3 + MaxInt32TextLen = 14, against a possible 23. The commented-out WriteUnifiedSequence had it
  right. Now covered by a test — but defensively, not as a live bug: the test fails on main only
  because it calls WriteMultiBulkHeader(long.MaxValue) directly, and no caller can reach that, since
  argument counts come from array lengths. The sizing follows the parameter type rather than today's
  callers; the test comment says the same.
• Large binary values keep their length prefix in place. WriteUnifiedSpan sends everything over
  MaxQuickSpanSize (512) down the piecewise path because no single span could hold it — so for a 4KB
  byte[] that path is the normal one, not a fallback, yet it called WriteCountPrefixSlow
  unconditionally. That composed $4096\r\n into a stack buffer and handed it to a hintless
  BuffersExtensions.Write: an extra copy, and the same length-prefix-split-across-segments
  fragmentation this PR fixes two hunks earlier in WriteCrlf. The string path never had it
  (WriteUnifiedPrefixedString calls the checked WriteCountPrefix), so it was asymmetric for no reason.
  It now calls WriteCountPrefix too, and is renamed WriteUnifiedSpanPiecewise — …Slow described only
  one of its two entry reasons. The two header fallbacks got the same treatment: the ask that was declined
  was larger than 23, so the smaller one may well still land in place. WriteCountPrefixSlow is now
  reached only from WriteCountPrefix.
• WriteUnifiedInt64 passed the wrong scratch constant - MaxPrefixScratch (30) while requesting 27.
  It fit by three bytes, guarded only by a Debug.Assert that is gone in Release.
• WriteCrlf keeps its hint of 2. Falling through to BuffersExtensions.Write would ask with no hint
  at all, and CycleBuffer's hint <= 0 branch returns anything non-empty - so a 1-byte segment tail
  would split a CRLF across segments rather than rolling to a fresh one. Protocol-correct either way, but
  a fragmentation change on a per-bulk-string path is not one to make by accident.

Cost

Formatting only, no server, via the added MessageWriterBenchmarks:

That is the honest price of checking, and it is reproducible — measured twice independently, at +3.1% and
+3.0%.

Only that figure is claimed. The mixed-value and 4KB cases swung by several percent between runs in
both directions (main's own 4KB figure moved between 187ns and 204ns across sessions), so they are not
reported: BenchmarkDotNet's error bars measure within-run precision, not between-run reproducibility. The
filter set also matters — including the fallback benchmark in the same run moved SET key value to
70.6ns, so before-and-after runs must measure the same set.

The benchmark has two writers, because they answer different questions. Reusable always returns the
remainder of a 16KB buffer, so the hint is always honoured and the table above measures what the check
costs. Stingy never returns more than 8 bytes, so every burst takes the fallback; that one cannot be
compared against a pre-fix build, because a pre-fix build faults.

8 is deliberate, and an earlier 64 was useless: the largest ask in SET user:1 marc is 23 (WriteHeader
wants 9 framed command bytes + 3 + MaxInt32TextLen; WriteCountPrefix wants 3 + MaxInt64TextLen), so a
64-byte cap declines nothing and measures the direct path twice. 16 declines the two 23-byte asks but still
does not fault on main, whose WriteHeader writes only 13 of the 23 it asked for. 8 exercises every
fallback and faults on main — confirmed by running ShortSpansStillProduceCorrectFrames(max: 8), the
same command shape, against main.

Tests

BufferWriterHintTests uses a writer that honours at most N bytes per span and places a canary past the
span it hands out, verified on Advance, on the next hand-out, and on reading the result — all three,
because a span written past and then abandoned without advancing would slip through a check on Advance
alone.

That detail matters: my first version of this harness used a plain growing buffer and passed against the
unfixed code — an overrun lands harmlessly in the slack that follows, while the real writer corrupts the
next thing in the pool.

Direct coverage for the paths no message shape reaches: WriteSha1AsHex (the tightest fit in the file -
47 requested, exactly 47 written, no slack), keyspace-isolation prefixes, the prefixed multi-bulk header,
WriteInteger, and a long-width count.

34 tests. Against main, 16 of the 31 synthetic-writer tests fail — and the 3 BlockBuffer tests cannot
be counted alongside them, because they take the test host down with SIGSEGV before the run finishes.

Verification

Full suite against the docker topology: 6184 passed, 150 skipped, 1 failed. The failure is
FailoverTests.SubscriptionsSurvivePrimarySwitchAsync, which fails identically on main.
CommandTimeoutTests.DefaultHeartbeatLowTimeout and DatabaseTests.CountKeys appeared in one run, were
absent from the next, and pass consistently in isolation — flaky, not related. Full
dotnet build Build.csproj -c Release /p:CI=true passes.

Not addressed

CycleBuffer is unchanged, deliberately — it is within its rights, and the callers were the problem.

Separately noticed while investigating: ScriptReadOnlyCommandTests, ScriptEvalRespNoScriptTests and
RenderedArgsLeakTests fail rather than skip when no server is present, because they hand-roll their
connection to get a command-map hook (TestBase.Create has none) and so lose the
AbortOnConnectFail = false plus Assert.SkipUnless(conn.IsConnected, ...) that TestBase applies. Not
touched here, but it reports "broken" instead of "skipped" on any machine without Redis, which is a
misleading signal when you are trying to tell whether your own change broke something.