Fuzz core channels: harness plus cancelled-write teardown fix

[flcl42]

Stacked on #299. Deterministic fuzzing of the core \Channel/\ReaderWriter\ rendezvous state machine plus one teardown fix.

Summary

• Observe teardown in cancelled-write acknowledgement wait: a writer cancelled after its signal was consumed parked in a tokenless _read\ wait that ignores close, leaking the task and holding the write lock. The wait now observes the teardown token.
• Add deterministic core channel fuzz harness with committed seed corpus: scripted transfer tests with exact byte/result oracles over both directions plus seeded lifecycle events, a systematic parked-op race matrix (close/abort/cancel/EOF/data), zero/negative/huge-length matrix, completion and reverse-identity pins, a token-identity concurrency hammer (exact conservation when open, duplicate-free subset after close/abort), a cancellation storm, and unpinned-reader chaos with an InternalError canary. No new dependencies.

Verification

• Solution builds with warnings as errors, zero warnings.
• Core suite green, 214 tests passing, run via the test executable directly.
• Channel.cs line coverage is at 100% except defensive-only paths proven unreachable via public API (end-of-write discards with buffered data, abort-mark inside reads, Closed-flag write path, occupied-buffer write path) and one race-only cancel-reclaim path; each is analyzed in the task report.
• Note on the teardown fix: the stranded path needs a microsecond take/cancel interleave, so no deterministic test forces it (verified: the surrounding cancel/close test passes identically with and without the fix). The fix is justified by code-level proof (an uncancelled wait with no possible releaser parks past teardown) and full-suite green with it.

[Copilot]

Copilot review overview

🟡 Changes recommended

The teardown wait still uses an already-cancelled linked token, and an interrupted-hammer assertion can fail for valid executions.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Wait on teardown token instead of already-cancelled waitToken · New
• Relax sent-token assertion for intentionally interrupted writers · New
• Test does not exercise the changed acknowledgement wait · New

What changed in this PR

Fixes cancelled-write teardown in the core channel and adds extensive rendezvous fuzz coverage.

Changes:

• Makes cancelled writes observe channel teardown.
• Adds lifecycle, boundary, and concurrency fuzz tests.
• Adds seeded transfer and token-conservation checks.

File	Description
Channel.cs	Updates cancelled-write acknowledgement waiting.
ChannelModelFuzzTests.cs	Adds model and lifecycle fuzz coverage.
ChannelConcurrencyFuzzTests.cs	Adds concurrent token-conservation fuzzing.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[benaadams]

Three test-harness findings with suggested code changes. The teardown fix itself appears sound. Validation: 109 core fuzz cases and the changed Yamux test passed; the 109 cases also passed with the teardown fix temporarily removed.

[yashksaini-coder]

The fix is correct and the harness is doing real work. Worth saying first that the description undersells this PR on two counts.

The note that "no deterministic test forces it" is stale. Reverting the token — _read.WaitAsync(_closed.Token) back to _read.WaitAsync() — fails CancelledWrite_AwaitingAcknowledgement_CompletesOnClose, every time, on the five-second timeout. Something was clearly added after that paragraph was written, and the reflection trick it uses to steal the _canRead permit is a much better way to force this than a sleep would have been. Worth replacing that bullet with the test name, because as written it reads like an unverifiable change and it isn't one. Core is also 215 here rather than the 214 quoted.

On correctness, the thing I wanted to rule out was a repeat of the delivered-write-reports-failure problem from #288, and it can't happen here: the new wait sits inside catch (OperationCanceledException), so the write is already committed to reporting Cancelled or Ended on every exit, and there's no path from there back to Ok. The token only converts a park into Ended. _canWrite is still released exactly once under the canWriteTaken guard. The comment explaining why the caller token must not be reused here is genuinely useful — that's the part someone would otherwise "simplify" later.

The fuzz harness earns its size. I spot-checked it by breaking things in Channel.cs and the new tests caught each one by name rather than by hanging or by a vague invariant, and the content oracles are real — exact bytes and exact IOResult, not "didn't throw".

One thing I would like you to look at, not blocking. There are 24 Task.Delay calls in the two new files used as synchronisation rather than as a failure bound, mostly await Task.Delay(50) to let an operation park before the test perturbs it. That is the same shape as the test in #299 that failed for me one run in five — 50 ms is a guess about the scheduler, not a handshake, and these only have to lose once on a loaded runner. Fifteen consecutive runs are clean here, so there's no evidence they do fail; I'd just rather not find out on CI. You already have the better technique in this PR.

Related: the header comment says "Every await is time-bounded so a hang fails loudly instead of hanging CI", and that isn't quite true — several channel awaits carry no bound. I hit this directly while poking at the file: a deliberate break in a hot path hung the run instead of failing it, and I had to kill it. Either bound them or soften the comment, since the current wording would make someone trust a green run more than they should.

Approving.