Repository navigation
fix(#2809): the two IoPoolTest gates use the sanctioned bridge, and release in a finally - #2822
Merged
Merged
Conversation
…elease in a finally test/MeshWeaver.Hosting.Test/IoPoolTest.cs held two TaskCompletionSource-as-a-gate sites — the shape AGENTS.md names in the same breath as ManualResetEventSlim. Found by the Copilot review on #2806 and deliberately left out of that PR, whose scope was the 79 measured ManualResetEventSlim sites plus the guard's move to zero-tolerance. ## Why the ratchet cannot catch them HandWovenGateRatchetGuard matches the primitives that park a thread BY NAME (SemaphoreSlim | ManualResetEventSlim | ManualResetEvent | AutoResetEvent | CountdownEvent | Monitor.Wait). A TaskCompletionSource used as a gate is structurally the same thing, but the type has a hundred legitimate uses, so a name match would cry wolf. That is why these two needed a human finding rather than a ratchet, and why the fix is worth landing rather than waiting for the guard to grow a rule it cannot express. ## The two sites * `Invoke_caps_in_flight_at_the_pool_bound` — 20 pooled leaves parked on one signal, created WITHOUT RunContinuationsAsynchronously. A single SetResult() resumed every waiting leaf INLINE on the test thread: the exact inline- resumption defect ObservableToTaskBridgeGuard exists for. * `Drain_joinsABlockingLeaf_evenWhileAnAsyncLeafIsRunning` — the async leaf's gate. Carried the flag, so the milder of the two. Neither released in a `finally`: an assertion that threw before the release left the parked leaves waiting on a signal that never arrived, so the host HUNG and an ordinary assertion failure was reported as a timeout. ## The fix AsyncSubject<T> + ObservableAwait.Await() — the one sanctioned bridge, which sets RunContinuationsAsynchronously itself — with the release in a `finally` at both sites. Completing twice is a no-op, so the happy path and the finally can both release without a flag. No cancellation token on the asyncGate Await, deliberately: that test's premise is that the async leaf is STILL in flight when the blocking join runs, so a ct-aware wait unwinding on the drain's cancel would weaken exactly what it measures. ## Mutation-checked, on the property that matters Made an assertion fail BEFORE the release (`Be(cap)` → `Be(cap + 99)`): MeshWeaver.Reactive.Assertions.AssertionException : Expected value to be 102, but found 3. Failed: 1, Duration: 22 ms 22 ms and the real message — not a hang. Without the finally that same mutation strands 20 pooled leaves and the assertion is reported as a timeout. dotnet build test/MeshWeaver.Hosting.Test -c Release -warnaserror 0 Error(s) dotnet test test/MeshWeaver.Hosting.Test (whole project) 269 passed, 0 failed Fixes #2809 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FhP7Jg65q8cAATnFfeaTK
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two remaining “TaskCompletionSource-as-a-gate” patterns in IoPoolTest by switching to the sanctioned AsyncSubject<T> + ObservableAwait.Await() bridge and ensuring the release happens from a finally, so assertion failures don’t strand pooled workers and masquerade as timeouts.
Changes:
- Replace
TaskCompletionSourcegate usage withAsyncSubject<T>plusObservableAwait.Await()in twoIoPoolTestcases. - Move gate release to
finallyblocks to prevent hangs when assertions fail before the release path. - Add a What’s New entry documenting the user-noticeable impact (tests reporting the real failure instead of hanging).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Test/IoPoolTest.cs | Replaces the two TCS gate sites with AsyncSubject + Await() and guarantees release in finally. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-two-test-gates-stop-resuming-inline.md | Adds a What’s New entry describing the fix and its effect on test failure reporting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two
TaskCompletionSource-as-a-gate sites intest/MeshWeaver.Hosting.Test/IoPoolTest.cs— the shape AGENTS.md names in the same breath asManualResetEventSlim. Found by the Copilot review on #2806 and deliberately left out of it, whose scope was the 79 measuredManualResetEventSlimsites plus the guard's move to zero-tolerance.Why the ratchet cannot catch these
HandWovenGateRatchetGuardmatches the primitives that park a thread by name (SemaphoreSlim | ManualResetEventSlim | ManualResetEvent | AutoResetEvent | CountdownEvent | Monitor.Wait). ATaskCompletionSourceused as a gate is structurally the same thing, but the type has a hundred legitimate uses — a name match would cry wolf on all of them. So these needed a human finding, and the fix is worth landing rather than waiting for a guard to grow a rule it cannot express.The two sites
Invoke_caps_in_flight_at_the_pool_boundRunContinuationsAsynchronously— a singleSetResult()resumed every waiting leaf inline on the test thread, the exact inline-resumption defectObservableToTaskBridgeGuardexists forDrain_joinsABlockingLeaf_evenWhileAnAsyncLeafIsRunningNeither released in a
finally. An assertion that threw before the release left the parked leaves waiting on a signal that never arrived — so the host hung, and an ordinary assertion failure with a perfectly good message was reported as a timeout.The fix
AsyncSubject<T>+ObservableAwait.Await()— the one sanctioned bridge, which setsRunContinuationsAsynchronouslyitself — with the release in afinallyat both sites. Completing twice is a no-op, so the happy path and thefinallycan both release without a flag.No cancellation token on the
asyncGateAwait(), deliberately: that test's premise is that the async leaf is still in flight when the blocking join runs, so act-aware wait unwinding on the drain's cancel would weaken exactly what it measures.Mutation-checked on the property that matters
Made an assertion fail before the release (
Be(cap)→Be(cap + 99)):22 ms and the real message — not a hang. Without the
finally, that same mutation strands 20 pooled leaves and the assertion is reported as a timeout.Fixes #2809
🤖 Generated with Claude Code
https://claude.ai/code/session_014FhP7Jg65q8cAATnFfeaTK