fix(engine): keep per-test timeout token source alive through teardown (#6339)#6349
Conversation
#6339) The prior fix for #6339 restored Context.CancellationToken to the outer token before After(Test) hooks, but only repaired the context *property*. A token copy snapshotted during the test body (as EF Core, Respawn or an ASP.NET host routinely do) still pointed at the timeout-scoped CancellationTokenSource, which TimeoutHelper disposed the instant the body returned. A synchronous .WaitHandle wait in an After(Test) hook / cleanup then threw ObjectDisposedException "The CancellationTokenSource has been disposed." — surfacing randomly in ASP.NET Core integration suites even on 1.57.17. Fix the source *lifetime* rather than just the token identity: own the linked timeout CTS across the whole per-test lifecycle instead of inside TimeoutHelper's using-scope. - TimeoutHelper gains a CTS-owning overload: the caller passes an already-linked source (+ the external token for the timeout-vs-cancel check) and is responsible for disposal; the old 4-arg overload now delegates to it and keeps disposing its own source for standalone callers. - TestExecutor stores the source on the new TestContext.TimeoutCancellationSource (sibling of LinkedCancellationTokens) and no longer disposes it locally; a prior retry attempt's source is released before the next replaces it. - TestCoordinator disposes it once at the end of the per-test finally — after After(Test) hooks, instance/OnDispose, object cleanup and After(Class/Assembly) — so any token copy captured mid-body stays backed by a live source throughout teardown. The inner-finally property restore stays (it fixes token identity; this fixes source liveness — the two are complementary). The no-timeout fast path is untouched (no new allocation). Strengthens the #6339 regression test to also assert on a body-captured token copy, not just the context property — the realistic failure mode.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Summary: This PR fixes #6339 by changing the lifetime of the per-test timeout Verified:
🤖 Generated with Claude Code |
Problem
Issue #6339 —
[After(Test)] hook failed: The CancellationTokenSource has been disposed.surfacing randomly in ASP.NET Core integration suites. The reporter confirmed it still reproduces on 1.57.17, which already contains the earlier narrow fix (#6340).Root cause
When a test has a timeout (explicit
[Timeout]orDefaultTestTimeout), the body runs under a linkedCancellationTokenSourceexposed onTestContext.Execution.CancellationToken. The earlier fix restored that property to the outer token before After hooks — butTimeoutHelperstill disposed the source the instant the body returned. Any token copy captured during the body (EF Core, Respawn, an ASP.NET host all snapshot the ambient token mid-test and touch it during teardown) then pointed at a disposed source.Verified .NET behaviour: on a disposed-but-not-cancelled CTS, only
token.WaitHandlethrowsObjectDisposedException— exactly what a synchronous wait inside EF Core /SemaphoreSlim/ host shutdown hits during an[After(Test)]hook.Fix
Repair the source lifetime, not just the token identity: own the linked timeout CTS across the whole per-test lifecycle instead of inside
TimeoutHelper'susingscope.TimeoutHelpergains a CTS-owning overload — the caller passes an already-linked source (+ the external token for the timeout-vs-cancel check) and owns disposal. The original 4-arg overload delegates to it and still disposes its own source for standalone callers (e.g. the existing unit test).TestExecutorstores the source on a new internalTestContext.TimeoutCancellationSource(sibling ofLinkedCancellationTokens) and no longer disposes it locally; a prior retry attempt's source is released before the next replaces it.TestCoordinatordisposes it once at the end of the per-testfinally— after After(Test) hooks, instance/OnDispose, object cleanup and After(Class/Assembly) — so a mid-body token copy stays backed by a live source throughout teardown.The inner-finally
Context.CancellationToken = <outer>restore stays: it fixes token identity (hooks/retry back-off observe a live, non-cancelled token); this change fixes source liveness. The two are complementary.The no-timeout fast path is untouched — no new allocation, no extra state machine (one
CancellationTokenSourceper timed test, same as before, just re-owned).Tests
#6339regression test: it now asserts both on the context property and on a token copy captured during the body (the realistic failure mode). Verified it fails without the fix (After hook throwsObjectDisposedException) and passes with it.TUnitSettingsTests(exercisesTimeoutHelperdirectly) — green.TimeoutCancellationTokenTests/CancellationTokenTriggeredTestsdirect-run counts match baseline exactly (timeout still fires correctly).TUnit.Enginebuilds across all TFMs.Closes #6339.