fix(engine): reset test cancellation token before After hooks on timeout path (#6339)

[thomhurst]

Problem

Fixes #6339. Users running ASP.NET Core integration tests hit random failures:

[Test Failure] After(Test) hook failed: The CancellationTokenSource has been disposed.
  at TestExecutor.ExecuteAsync -> TestCoordinator.ExecuteTestAsync

Root cause

When a test is subject to a timeout (an explicit [Timeout] or a configured
DefaultTestTimeout), TimeoutHelper.ExecuteWithTimeoutAsync runs the body under a linked
CancellationTokenSource and TestExecutor.ExecuteTestAsync stores that token on
TestContext.Execution.CancellationToken (TestExecutor.cs:428). The linked CTS is disposed via
using the instant the body returns — but the context was left pointing at it.

The finally in ExecuteAsync then runs the test-end event receivers and
After(Test)/AfterEvery(Test) hooks. Those hooks are invoked with CancellationToken.None, but
any code that reads context.Execution.CancellationToken and touches the underlying source —
.WaitHandle (synchronous waits in EF Core / Respawn / SemaphoreSlim.Wait), .Token, .Cancel,
etc. — throws ObjectDisposedException: The CancellationTokenSource has been disposed. The same
disposed token also feeds RetryHelper.ApplyBackoffDelay (RetryHelper.cs:118).

It looks "random" because only timed tests whose After-hooks/retries actually touch the token in a
source-accessing way fail, and the no-timeout fast path is unaffected (it stores the still-valid
outer session token).

Fix

Restore the still-valid outer token at the top of the ExecuteAsync finally, before the test-end
phase runs and before the exception unwinds into any retry back-off:

if (testTimeout.HasValue)
{
    executableTest.Context.CancellationToken = cancellationToken;
}

• Gated on testTimeout.HasValue so the no-timeout fast path (and any user-added linked token via
  TestContext.AddLinkedCancellationToken) is untouched.
• Runs on every exit (pass / fail / skip / exception) and before the re-throw, so it also covers the
  retry back-off manifestation.
• Engine-only; identical in source-gen and reflection modes. No public API or snapshot changes.

Test

TUnit.TestProject/Bugs/_6339/TimeoutAfterHookTokenTests.cs — a passing [Timeout] test whose
[After(Test)] hook reads the context token's WaitHandle. Verified it fails with
"The CancellationTokenSource has been disposed." before the fix and passes after.

Note for the reporter

The companion BeforeTest hook failed: ... transient failure ... EnableRetryOnFailure message is a
plain EF Core SqlServer transient fault in your own seeding hook, not a TUnit defect — add
options.UseSqlServer(cs, o => o.EnableRetryOnFailure()) to handle it.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric	Results
Complexity	0

View in Codacy

_{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.}

[claude]

Reviewed the fix, root-cause analysis, and regression test. This is a solid, well-scoped fix.

Verification performed (not just reading the diff):

• Traced the token flow: TimeoutHelper.ExecuteWithTimeoutAsync creates a linked timeoutCts from the outer cancellationToken and disposes it via using once the body returns; TestExecutor.ExecuteTestAsync stores that linked token on Context.CancellationToken. The finally fix restores Context.CancellationToken = cancellationToken (the still-valid outer token) before the test-end event receivers and After hooks run — exactly closing the gap described.
• Confirmed RetryHelper.ApplyBackoffDelay (Task.Delay(delayMs, testContext.CancellationToken)) reads the same context token, so the fix also covers the retry back-off crash mentioned in the PR description, not just the After-hook path.
• Confirmed each retry attempt re-invokes TestExecutor.ExecuteAsync fresh (RetryHelper.ExecuteWithRetry → TestCoordinator.ExecuteTestLifecycleAsync → ExecuteAsync), so the per-attempt finally reset is correctly scoped — no cross-attempt state leakage.
• Checked interaction with TestContext.AddLinkedCancellationToken: the composite token it produces is never fed into TimeoutHelper in the first place (that call always uses the plain outer cancellationToken parameter, not Context.CancellationToken), so this fix doesn't regress that feature for timeout+linked-token combinations — that gap (if it matters) predates this PR.
• Built and ran the new regression test (TimeoutAfterHookTokenTests) against the branch on net8.0/net9.0/net10.0 — passes with the fix. Confirmed via git diff against main that only TestExecutor.cs and the new test file changed — no source-gen/reflection divergence risk, no public API or snapshot changes, consistent with the PR description.
• No unresolved reviewer feedback on the PR to reconcile (only an automated Codacy comment reporting 0 issues).

Nothing to change. The testTimeout.HasValue gate is the right scope (untouched fast path for non-timeout tests), the placement at the top of the finally (before event receivers/After hooks/retry backoff) is correct, and the regression test reproduces the exact failure mode from the issue.

[claude]

Code review

Reviewed the new commit since my previous approval (10bede65 — "refactor(engine): colocate #6339 token restore with the timeout call").

What changed: the Context.CancellationToken restore was moved from the outer finally (after test-end receivers/After hooks run) into the inner finally that wraps the TimeoutHelper/fast-path call — i.e. right next to the code that mutates the token in the first place. The regression test was also simplified to an expression-bodied method and its doc comment was corrected to match the actual repro (WaitHandle access rather than CreateLinkedTokenSource).

Verification:

• Traced execution: the inner try/catch/finally always completes — including the finally — before the exception (if any) propagates to the outer catch (SkipTestException) / catch (Exception) blocks, so moving the reset into the inner finally is behaviorally identical to the previous outer-finally placement for every exit path (pass/fail/skip/exception). No regression introduced.
• Rebuilt TUnit.Engine — succeeds, no warnings.
• Re-ran the TimeoutAfterHookTokenTests regression test — passes.
• No unresolved reviewer feedback to reconcile (only the automated Codacy comment reporting 0 issues).

On the design change itself: this is a genuine improvement, not just churn. The original version put the mutation (TimeoutHelper swaps in a linked, later-disposed token) and its compensating cleanup (Context.CancellationToken = cancellationToken) in two different finally blocks separated by ~25 lines — exactly the kind of distance that let the original bug hide for as long as it did. Colocating the fix with the code that creates the problem reduces the chance a future edit reintroduces the same disposed-token leak, since the invariant ("if you scope a token for the timeout call, restore it right here before falling through") is now visible in one place instead of implied across two finally blocks.

No issues found. Approving.