fix(engine): isolate per-session state under MTP server-mode concurrency (#6001) by thomhurst · Pull Request #6025 · thomhurst/TUnit (original) (raw)
…ncy (#6001)
Concurrent testing/runTests RPCs against one long-lived MTP test process were producing zero results from sessions that should produce ~100. Root causes were shared mutable state across sessions/calls rather than a single race:
- EngineCancellationToken.Initialise reassigned CancellationTokenSource and Token per RPC; the loser inherited the winner's cancellation chain. Made it idempotent — the session CTS is now established at construction and per-call cancellation flows through context.CancellationToken explicitly.
- TUnitInitializer added new Trace listeners and AppDomain/TaskScheduler closures on every ExecuteRequestAsync, capturing stale ExecuteRequestContext refs. Split into per-process / per-session work behind Interlocked guards.
- ReflectionTestDataCollector kept _scannedAssemblies and _discoveredTests as process-wide statics, so session N+1 saw assemblies as already scanned and returned []. Moved to instance fields.
- ReflectionInstanceFactory dropped its process-wide instance cache; reflection generic-type discovery is a cold path and isolation matters more than dedup.
- ReflectionHookDiscoveryService's Interlocked.Increment guard short-circuited concurrent first callers past empty Sources.* bags. Replaced with a ManualResetEventSlim gate so they block until population finishes.
- Standard{Out,Error}ConsoleInterceptor installed a per-session interceptor via Console.SetOut/SetError and removed it on dispose, breaking concurrent sibling sessions' interception. Install exactly once per process; the interceptor itself dispatches to Context.Current (AsyncLocal) so one instance serves every session.
- Sources gets a doc invariant: hook bags are append-only after init.
- ActivityCollector._current limitation under multi-session documented (deferred).
Adds TUnit.RpcTests coverage:
- RunTests_ConcurrentRpcsOnSameSession_AllReceiveFullResults: dispatches N concurrent testing/runTests RPCs on a single TestHostSession across iterations and asserts every RPC receives the full result set.
- RunTests_WithMalformedRunId_ServerAttributesUpdatesToGuidEmpty: pins the upstream microsoft/testfx behaviour where a non-Guid runId is silently coerced to Guid.Empty (finding 3 of #6001). Skipped pending upstream fix.
[![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4)
- Add 5-minute timeout to ReflectionHookDiscoveryService gate so a hung first-time discovery can't deadlock concurrent sessions.
- Correct CA1416 suppression justification on DiscoverHooks (the wait sits before the RuntimeFeature check, not after).
- Drop the unused CancellationToken parameter from EngineCancellationToken.Initialise; per-call cancellation already flows through context.CancellationToken explicitly.
- Move the Sources class XML doc above the [EditorBrowsable] attribute so it's not split by conditional compilation.
[
- ReflectionHookDiscoveryService: capture the first caller's discovery exception and rethrow from waiters so a partial-state failure can't silently fan out to concurrent sessions.
- TUnitInitializer: clarify that _sessionInitialised is per-instance and cross-session "run once per process" lives downstream.
- OptimizedConsoleInterceptor: note that ResetDefault is now a no-op for the standard interceptors so future readers don't reintroduce per-session Console.SetOut/Reset churn.
[
[
Addresses PR review finding 4 (static guard + instance dependency mismatch in TUnitInitializer). Lift process-once concerns — global exception handlers, the throw-on-trace-assert listener, and test parameter parsing — out of the per-session TUnitInitializer into a dedicated static type whose name makes the lifetime obvious.
TUnitInitializer now does only session-scoped work and delegates explicit ICommandLineOptions to the process initializer. The static flag for parameter parsing is gone; the per-instance guard left in TUnitInitializer protects only concurrent ExecuteRequestAsync calls within a single session, which is its actual job.
…zers
Three call sites had the same wait-on-first-caller pattern, and two of them (TUnitProcessInitializer.EnsureInitialised and TUnitInitializer._sessionInitialised) were still set-flag-then-work — the very race we just fixed downstream in ReflectionHookDiscoveryService. Concurrent callers could short-circuit past partial initialization and read TestContext.InternalParametersDictionary or Sources.* before the producing caller finished populating them.
- Extract OneTimeGate (TUnit.Engine/Helpers): single-producer / multi-waiter gate with bounded timeout and exception capture. Concurrent callers block until the producer finishes; the producer's exception is re-surfaced to every waiter so no caller silently proceeds with partial state.
- TUnitProcessInitializer, TUnitInitializer, and ReflectionHookDiscoveryService all route through OneTimeGate now — consistent semantics, no copy-paste.
- TUnitInitializer.Initialize drops its unused ExecuteRequestContext parameter, matching what was already done for EngineCancellationToken.Initialise.
- Drop [Retry(3)] from RunTests_ConcurrentRpcsOnSameSession_AllReceiveFullResults — a retry would mask a re-introduced race rather than surface it, and that masking is exactly what this regression test exists to prevent.
[
This was referenced
Jun 13, 2026
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
[ Show hidden characters]({{ revealButtonHref }})