Skip to content

Commit a475145

Browse files
fix(selftest): eliminate timer-vs-cancel race in DepsThrashing fixture (#200)
* fix(selftest): eliminate timer-vs-cancel race in DepsThrashing fixture DepsThrashing_AtMostOneCompleted hit completed=2 on CI Stress run #25518410339 (shards 5 and 10). The previous fix (ThrowIfCancellationRequested after Task.Delay(200)) left a TOCTOU window: if the pool thread's CT check ran before the UI thread's Cts.Cancel() completed, the counter incremented for a fetch the production hook correctly discards. Root cause: when CI render ticks approach 200ms, the delay timer and dep-change cancellation fire nearly simultaneously on different threads, making the race likely rather than theoretical. Fix: replace Task.Delay(200, ct) with Task.Delay(Timeout.Infinite, ct). An infinite-duration delay can only exit via OperationCanceledException — there is no timer to race against. completed stays 0; the assertion guards against regressions. Remove the now-unreachable ThrowIfCancellationRequested() and the Harness.Render(400) wait that existed solely to let the last fetch resolve. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fixup: address CR feedback on DepsThrashing fixture Two issues raised in review of #200: 1. host.Dispose() before GC drain: the last dep's Task.Delay(Infinite, ct) ran forever with no explicit teardown. Disposing the host triggers the UseEffect cleanup which cancels the CTS, ensuring the in-flight task is torn down before the unobserved-exception probe runs. 2. Tighten completed==0: the comment said "completed stays 0" but the assertion was completed<=1. Changed to completed==0 so the fixture actually catches a regression if a completion slips through. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent c363bdf commit a475145

1 file changed

Lines changed: 12 additions & 15 deletions

File tree

tests/Reactor.AppTests.Host/SelfTest/Fixtures/AsyncResourceFramerateFixtures.cs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,12 @@ public override async Task RunAsync()
7171
try
7272
{
7373
ct.Register(() => Interlocked.Increment(ref cancelled));
74-
await Task.Delay(200, ct);
75-
// Close the race where Task.Delay's 200ms timer
76-
// fires almost simultaneously with cancellation:
77-
// Task.Delay can complete normally (timer wins
78-
// the atomic result-set) even though Cancel()
79-
// was called, leaving the await to resume and
80-
// increment `completed` for a fetch the hook
81-
// will correctly discard. Re-checking the CT
82-
// after the await observes the cancellation.
83-
ct.ThrowIfCancellationRequested();
74+
// Infinite delay: the only exit is OperationCanceledException
75+
// when the CT fires. This eliminates the timer-vs-cancel race
76+
// that made completed reach 2 when CI render ticks approached
77+
// the old 200ms fixed delay. `completed` stays 0; the
78+
// assertion below guards against regressions.
79+
await Task.Delay(Timeout.Infinite, ct);
8480
Interlocked.Increment(ref completed);
8581
return $"dep={dep}";
8682
}
@@ -101,9 +97,6 @@ public override async Task RunAsync()
10197
await Harness.Render();
10298
}
10399

104-
// Only the final deps value's fetch should survive. Let it resolve.
105-
await Harness.Render(400);
106-
107100
// Invariant 1: fetches started roughly once per deps-change (some coalesce).
108101
H.Check($"DepsThrashing_Started (started={started}, frames={Frames})",
109102
started >= Frames / 3 && started <= Frames + 2);
@@ -117,11 +110,15 @@ public override async Task RunAsync()
117110
H.Check($"DepsThrashing_StaleCancelled (cancelled={cancelled}, started={started})",
118111
cancelled >= started - 1);
119112

120-
// Invariant 4: at most one fetch ran to completion — the last deps' one.
113+
// Invariant 4: no fetch body completes — all use Timeout.Infinite delays and
114+
// exit only via cancellation. completed must be exactly 0.
121115
H.Check($"DepsThrashing_AtMostOneCompleted (completed={completed})",
122-
completed <= 1);
116+
completed == 0);
123117

124118
// Invariant 5: no unobserved task exceptions escaped under this load.
119+
// Dispose the host first so the last in-flight Task.Delay(Infinite, ct) is
120+
// cancelled via UseEffect teardown before we drain for unobserved exceptions.
121+
host.Dispose();
125122
await Harness.Render();
126123
GC.Collect();
127124
GC.WaitForPendingFinalizers();

0 commit comments

Comments
 (0)