Skip to content

IoPoolTest holds two TaskCompletionSource-as-a-signal gates (one without RunContinuationsAsynchronously) #2809

Closed

Description

@rbuergi

Found by the Copilot review on #2806 (the sweep that took test/'s hand-woven gates from 79 to 0). The finding is correct and was deliberately left out of that PR: these are not ManualResetEventSlim and so were never in the ratcheted inventory, and the change set was scoped to the 79 measured sites plus the guard's move to zero-tolerance. Filing it so it is not lost in a review thread.

The sites

test/MeshWeaver.Hosting.Test/IoPoolTest.cs holds two TaskCompletionSource-as-a-signal gates — the shape AGENTS.md lists in the same breath as ManualResetEventSlim ("a TaskCompletionSource as a gate"):

line site note
~317 Invoke_caps_in_flight_at_the_pool_bound — var release = new TaskCompletionSource();, awaited as await release.Task inside cap pooled leaves 🚨 no RunContinuationsAsynchronously, so release.SetResult() resumes every waiting leaf INLINE on the releasing thread — the exact inline-resumption defect ObservableToTaskBridgeGuard exists for
~467 Drain_joinsABlockingLeaf_evenWhileAnAsyncLeafIsRunning — var asyncGate = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);, returned as the async leaf's Task has the flag, so it is the milder of the two

Neither has a finally: on Invoke_caps_in_flight_at_the_pool_bound, an assertion that throws before release.SetResult() leaves 20 pooled leaves waiting on a signal that never arrives.

The fix

AsyncSubject<T> + ObservableAwait.Await() — the one sanctioned bridge, which sets RunContinuationsAsynchronously itself:

var release = new AsyncSubject<Unit>();
…
    await release.Await();      // in the pooled leaf, replacing `await release.Task`
…
try     { … release.OnNext(Unit.Default); release.OnCompleted(); … }
finally { release.OnNext(Unit.Default); release.OnCompleted(); }   // completing twice is a no-op

Keep Await() without a cancellation token at the asyncGate site: the test's premise is that the async leaf is still in flight when the blocking join runs, so a ct-aware wait that unwinds on the drain's cancel would weaken what it measures.

Why it is not caught today

HandWovenGateRatchetGuard's regex matches the primitives that park a thread by name (SemaphoreSlim|ManualResetEventSlim|…|Monitor.Wait); a TaskCompletionSource used as a gate is structurally the same thing but has a hundred legitimate uses, so a name match would cry wolf. ObservableToTaskBridgeGuard already detects hand-rolled bridges structurally (a TCS settled from inside a subscription) — worth considering whether it can be extended to "a TCS awaited by a body the test later completes", or whether these two sites are simply fixed and the shape left to review.

Acceptance

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions