Scope checkpoint metadata I/O completions to their own operation - #2154

Fixes #2149

Problem

DeviceLogCommitCheckpointManager kept its completion semaphore and its error code on the instance, and passed context: null to every device I/O:

private readonly SemaphoreSlim semaphore;   // one per manager
private uint metadataIoErrorCode;           // one per manager
...
metadataIoErrorCode = 0;
device.ReadAsync(..., IOCallback, null);    // nothing identifies the operation
semaphore.Wait();

Nothing tied a completion to the operation that issued it. With two metadata operations in flight:

• Completion theft — B's completion releases A's wait. ReadInto A wakes while its own read is still outstanding and copies its still-zeroed buffer, so GetIndexCheckpointMetadata reads a length prefix of 0 and rejects a valid checkpoint as truncated.
• Buffer recycled under in-flight I/O — A's finally { pbuffer.Return(); } then runs while A's read is still writing into that buffer. SectorAlignedBufferPool.Return zeroes the buffer and can hand it to another caller, so this also corrupts whichever operation was still running. This is a live use-after-return, not merely a stale read.
• Error stealing / clobbering — metadataIoErrorCode = 0 resets an error another operation has not observed yet, and the post-wait read can surface an error belonging to a different operation.

Symptom from the issue: concurrent INFO ALL intermittently logs Skipping unreadable index checkpoint / Invalid metadata length 0 and returns disk_checkpoint_entry:(empty) while the on-disk metadata is intact.

Fix

Each ReadInto/WriteInto allocates its own MetadataIoCompletion (its own SemaphoreSlim plus its own ErrorCode) and passes it as the device I/O context; IOCallback records the error on, and releases, only that instance. This mirrors the per-call semaphore Garnet.cluster.ClusterUtils already passes as its I/O context.

Because the wait can now only be satisfied by the operation's own callback, the existing finally { pbuffer.Return(); } becomes a correct buffer-lifetime guarantee instead of a race.

Metadata operations stay concurrent — INFO ALL is a normal concurrent request path, so serializing the whole metadata sequence behind one lock (the issue's other suggested option) was rejected; it would also not fix the buffer-lifetime hazard as directly.

Two details worth noting:

• The SemaphoreSlim is deliberately not disposed. It never allocates a wait handle (AvailableWaitHandle is untouched), and on a synchronous device throw — where no completion is delivered — disposing it would risk a late Release faulting a device I/O thread.
• Release/Wait supplies the happens-before edge that publishes ErrorCode to the waiting thread, which is the same guarantee the previous code already relied on.

Also in this PR:

• The lazy bufferPool creation was an unsynchronized check-then-act; it is now published with Interlocked.CompareExchange, and a candidate that loses the publish race is freed rather than dropped — its constructor reserves a process-wide pool slot that sizes every thread's shard array, and only Free returns that slot.
• GarnetClusterCheckpointManager.GetLogCheckpointMetadata sliced by the metadata length prefix without the ThrowIfInvalidMetadataSize check every base-class reader applies, so a truncated file was silently parsed as empty metadata (or threw ArgumentOutOfRangeException from Slice on a negative size). It now applies the same guard — ThrowIfInvalidMetadataSize becomes protected static — and disposes its device with using so the added throw cannot leak it. The bound is unchanged from the one the base class already applies to this identical file format.
• TestBase keys its running-test set by TestContext.Test.FullName rather than Name. Name carries a parameterized test's arguments but not its fixture, so same-named tests in different fixtures shared a key — which both blurred the unhandled-exception dump and let one test's TearDown remove an entry belonging to another.

Tests

New GatedCompletionDevice test double defers the underlying I/O, not just its callback, so a caller woken by a foreign completion observes its own buffer exactly as it left it. It releases pending operations in a chosen order, can complete one with an injected error code, and stamps a shared monotonic counter when each callback fires so "did this caller return before its own completion?" is assertable without timing.

Three tests, covering the four scenarios the issue asks for:

All three were verified to fail against the pre-fix code with the intended diagnostics (read 0 returned a buffer that its own read had not filled, the metadata write returned before its own completion fired..., a healthy read reported another operation's error), not merely to pass after it.

Detection is structural rather than timing-based: the gate holds the second caller inside the device call once its operation is captured, so it never reaches its wait. When the first completion is delivered there is exactly one waiter, and SemaphoreSlim — which specifies no wake order — has no ordering choice to make. An earlier revision relied on FIFO wake order and could pass against the broken code by scheduling luck.

ClusterCheckpointMetadataTests covers the cluster override that gained the length check: it asserts the named exception, that a failing read does not leak its device, and that valid metadata still round-trips. The internal manager is reached through the public ClusterFactory.CreateCheckpointManager rather than by widening its visibility.

Validation

• dotnet build Garnet.slnx and Tsavorite.slnx — 0 warnings, 0 errors
• dotnet format clean on Garnet.slnx and Tsavorite.slnx
• Tsavorite.test.hlog FlakyDeviceTests — 22/22 pass
• Garnet.test.cluster ClusterCheckpointMetadataTests — 3/3 pass
• Garnet.test CheckpointFailureTests + PortSlotTests — 34/34 pass
• Tsavorite recovery (CheckpointManagerTests, SimpleRecoveryTest, LogResumeTests) — 19 passed, 5 skipped (Azure)
• Standalone Garnet save/recover + checkpoint — 21 passed, 2 skipped
• Cluster replication checkpoint tests (ClusterSRPrimaryCheckpointAsync, ClusterSRPrimaryCheckpointRetrieve) fail identically on a pristine baseline with no changes applied — a pre-existing Timed out DeleteDirectory teardown flake on this Windows machine, reported as primary failure — test itself passed. Parity confirmed, not a regression from this PR.