Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions TUnit.Core/EngineCancellationToken.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using TUnit.Core.Settings;
using TUnit.Core.Settings;

namespace TUnit.Core;

Expand All @@ -10,25 +10,34 @@ public class EngineCancellationToken : IDisposable
/// <summary>
/// Gets the internal cancellation token source.
/// </summary>
internal CancellationTokenSource CancellationTokenSource { get; private set; } = new();
internal CancellationTokenSource CancellationTokenSource { get; } = new();

/// <summary>
/// Gets the cancellation token.
/// </summary>
public CancellationToken Token { get; private set; }
public CancellationToken Token { get; }

private int _initialised;
private volatile bool _forcefulExitStarted;

/// <summary>
/// Initializes the cancellation token with a linked token source.
/// </summary>
/// <param name="cancellationToken">The cancellation token to link with.</param>
internal void Initialise(CancellationToken cancellationToken)
public EngineCancellationToken()
{
CancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
Token = CancellationTokenSource.Token;
}

Token.Register(_ => Cancel(), this);
/// <summary>
/// Hooks up process-wide cancellation signals (Ctrl+C / ProcessExit) the first time it's
/// called for this instance. Idempotent — subsequent calls are no-ops so that concurrent
/// MTP server-mode RPCs against one session don't clobber each other's cancellation chain.
/// Per-call cancellation flows through the explicit <c>CancellationToken</c> threaded into
/// discovery/execution, not through this session-scoped token.
/// </summary>
internal void Initialise()
{
if (Interlocked.CompareExchange(ref _initialised, 1, 0) != 0)
{
return;
}

// Console.CancelKeyPress is not supported on browser platforms
#if NET5_0_OR_GREATER
Expand Down
8 changes: 8 additions & 0 deletions TUnit.Core/Sources.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@

namespace TUnit.Core;

/// <remarks>
/// INVARIANT: these collections are populated once at module initialization (by source-gen
/// in source-gen mode, by ReflectionHookDiscoveryService once-per-process in reflection mode)
/// and must be treated as append-only for the remainder of the process lifetime. Engine code
/// that calls <c>.Clear()</c> on any field here will starve concurrent MTP sessions of their
/// hooks (#6001). The reflection-mode discovery's one-time <c>ClearSourceGeneratedHooks</c>
/// runs before any session executes — adding new clear calls during a session is a bug.
/// </remarks>
#if !DEBUG
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
#endif
Expand Down
56 changes: 39 additions & 17 deletions TUnit.Engine/Discovery/ReflectionHookDiscoveryService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ internal sealed class ReflectionHookDiscoveryService
// Cache attribute lookups to avoid repeated reflection calls in hot paths
private static readonly ConcurrentDictionary<MethodInfo, (BeforeAttribute?, AfterAttribute?, BeforeEveryAttribute?, AfterEveryAttribute?)> _attributeCache = new();
private static int _registrationIndex = 0;
private static int _discoveryRunCount = 0;

// Hook discovery is process-wide (populates the shared Sources.* bags). Use a
// ManualResetEventSlim gate so concurrent first-time callers from multiple MTP sessions
// block until the single producer has finished populating, rather than racing past empty
// bags after a single Interlocked.Increment short-circuit.
private static int _discoveryStarted;
private static readonly ManualResetEventSlim _discoveryCompleted = new(initialState: false);

private static string GetMethodKey(MethodInfo method)
{
Expand Down Expand Up @@ -164,37 +170,53 @@ public static void DiscoverInstanceHooksForType(Type closedGenericType)

#if NET8_0_OR_GREATER
[RequiresUnreferencedCode("Hook discovery scans assemblies and types using reflection")]
[UnconditionalSuppressMessage("Platform", "CA1416:Validate platform compatibility", Justification = "Reflection-mode hook discovery is not reachable on browser targets — those scenarios use AOT/source-gen mode which never invokes this method.")]
#endif
public static void DiscoverHooks()
{
// Prevent running hook discovery multiple times in the same process
// This can happen when both discovery and execution run in the same process
if (Interlocked.Increment(ref _discoveryRunCount) > 1)
if (Interlocked.CompareExchange(ref _discoveryStarted, 1, 0) != 0)
{
// Another caller is doing (or has done) the work. Wait for it to finish so we
// don't proceed past empty Sources.* bags on a concurrent first call. Bounded
// so a stuck producer (e.g. a hanging ModuleInitializer in a scanned assembly)
// doesn't deadlock every concurrent session indefinitely.
if (!_discoveryCompleted.Wait(TimeSpan.FromMinutes(5)))
{
throw new TimeoutException(
"Reflection-mode hook discovery timed out waiting for the first caller to complete. " +
"This usually indicates a hang in an assembly being scanned for hooks.");
}
return;
}

// Clear source-generated hooks since we're discovering via reflection
// In reflection mode, source generation may have already populated Sources
// We need to clear them to avoid duplicates
ClearSourceGeneratedHooks();
try
{
// Clear source-generated hooks since we're discovering via reflection
// In reflection mode, source generation may have already populated Sources
// We need to clear them to avoid duplicates
ClearSourceGeneratedHooks();

#if NET
if (!RuntimeFeature.IsDynamicCodeSupported)
{
throw new Exception("Using TUnit Reflection mechanisms isn't supported in AOT mode");
}
if (!RuntimeFeature.IsDynamicCodeSupported)
{
throw new Exception("Using TUnit Reflection mechanisms isn't supported in AOT mode");
}
#endif

var assemblies = AppDomain.CurrentDomain.GetAssemblies();
var assemblies = AppDomain.CurrentDomain.GetAssemblies();

foreach (var assembly in assemblies)
{
if (ShouldScanAssembly(assembly))
foreach (var assembly in assemblies)
{
DiscoverHooksInAssembly(assembly);
if (ShouldScanAssembly(assembly))
{
DiscoverHooksInAssembly(assembly);
}
}
}
finally
{
_discoveryCompleted.Set();
}
}

#if NET8_0_OR_GREATER
Expand Down
25 changes: 6 additions & 19 deletions TUnit.Engine/Discovery/ReflectionInstanceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ namespace TUnit.Engine.Discovery;
/// This is a simplified version that handles basic property injection scenarios
/// where instance data sources depend on property-injected values.
/// </summary>
/// <remarks>
/// No process-wide instance cache: under MTP server mode multiple sessions in one host can
/// invoke generic-type discovery concurrently, and a shared cache would leak instances and
/// their per-session state across sessions (#6001). Reflection-mode generic discovery is
/// already a cold path, so we accept the duplication cost in exchange for isolation.
/// </remarks>
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Reflection mode isn't used in AOT scenarios")]
[UnconditionalSuppressMessage("Trimming", "IL2067", Justification = "Reflection mode isn't used in AOT scenarios")]
[UnconditionalSuppressMessage("Trimming", "IL2070", Justification = "Reflection mode isn't used in AOT scenarios")]
Expand All @@ -19,19 +25,11 @@ namespace TUnit.Engine.Discovery;
[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "Reflection mode isn't used in AOT scenarios")]
internal static class ReflectionInstanceFactory
{
private static readonly ConcurrentDictionary<Type, object?> _instanceCache = new();

/// <summary>
/// Creates an instance of the specified type with property injection performed.
/// </summary>
public static async Task<object?> CreateInstanceWithPropertyInjectionAsync(Type type)
{
// Check cache first to avoid creating multiple instances for the same type
if (_instanceCache.TryGetValue(type, out var cachedInstance))
{
return cachedInstance;
}

try
{
// Create the instance
Expand All @@ -44,9 +42,6 @@ internal static class ReflectionInstanceFactory
// Perform basic property injection
await InjectPropertiesAsync(instance, type);

// Cache the instance
_instanceCache.TryAdd(type, instance);

return instance;
}
catch
Expand Down Expand Up @@ -187,12 +182,4 @@ private static bool IsFunc(Type type)
var invokeMethod = func.GetType().GetMethod("Invoke");
return invokeMethod?.Invoke(func, null);
}

/// <summary>
/// Clears the instance cache. Should be called at the end of test discovery.
/// </summary>
public static void ClearCache()
{
_instanceCache.Clear();
}
}
10 changes: 8 additions & 2 deletions TUnit.Engine/Discovery/ReflectionTestDataCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ namespace TUnit.Engine.Discovery;
[UnconditionalSuppressMessage("Trimming", "IL2060:Call to \'System.Reflection.MethodInfo.MakeGenericMethod\' can not be statically analyzed. It\'s not possible to guarantee the availability of requirements of the generic method.")]
internal sealed class ReflectionTestDataCollector : ITestDataCollector
{
private static readonly ConcurrentDictionary<Assembly, bool> _scannedAssemblies = new();
private static ImmutableList<TestMetadata> _discoveredTests = ImmutableList<TestMetadata>.Empty;
// Per-session state: each TUnitServiceProvider instantiates its own collector, and these
// fields must not leak between sessions. Under MTP server mode the test host can serve
// many sessions sequentially or concurrently; static caches here would cause session N+1
// to see session N's assemblies as "already scanned" and return zero tests (issue #6001).
private readonly ConcurrentDictionary<Assembly, bool> _scannedAssemblies = new();
private ImmutableList<TestMetadata> _discoveredTests = ImmutableList<TestMetadata>.Empty;

// Process-wide caches: pure reflection metadata that doesn't change across sessions.
private static readonly ConcurrentDictionary<Assembly, Type[]> _assemblyTypesCache = new();
private static readonly ConcurrentDictionary<Type, MethodInfo[]> _typeMethodsCache = new();

Expand Down
2 changes: 1 addition & 1 deletion TUnit.Engine/Framework/TUnitTestFramework.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public async Task ExecuteRequestAsync(ExecuteRequestContext context)

await serviceProvider.HookDelegateBuilder.InitializeAsync();

serviceProvider.CancellationToken.Initialise(context.CancellationToken);
serviceProvider.CancellationToken.Initialise();

await _requestHandler.HandleRequestAsync((TestExecutionRequest) context.Request, serviceProvider, context, GetFilter(context));
}
Expand Down
22 changes: 13 additions & 9 deletions TUnit.Engine/Logging/StandardErrorConsoleInterceptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ namespace TUnit.Engine.Logging;

internal class StandardErrorConsoleInterceptor : OptimizedConsoleInterceptor
{
public static StandardErrorConsoleInterceptor Instance { get; private set; } = null!;

public static TextWriter DefaultError { get; }

// See StandardOutConsoleInterceptor for the rationale — Console.Error is also process-wide
// and must be installed exactly once so concurrent sessions in one host (#6001) don't
// clobber each other's interception.
private static int s_installed;

protected override LogLevel SinkLogLevel => LogLevel.Error;

protected override ConsoleLineBuffer GetLineBuffer() => Context.Current.ConsoleStdErrLineBuffer;
Expand All @@ -21,17 +24,18 @@ static StandardErrorConsoleInterceptor()
};
}

public StandardErrorConsoleInterceptor()
{
Instance = this;
}

public void Initialize()
{
Console.SetError(this);
if (Interlocked.CompareExchange(ref s_installed, 1, 0) == 0)
{
Console.SetError(this);
}
}

private protected override TextWriter GetOriginalOut() => DefaultError;

private protected override void ResetDefault() => Console.SetError(DefaultError);
private protected override void ResetDefault()
{
// No-op: we install exactly once per process. See StandardOutConsoleInterceptor.
}
}
26 changes: 17 additions & 9 deletions TUnit.Engine/Logging/StandardOutConsoleInterceptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ namespace TUnit.Engine.Logging;

internal class StandardOutConsoleInterceptor : OptimizedConsoleInterceptor
{
public static StandardOutConsoleInterceptor Instance { get; private set; } = null!;

public static TextWriter DefaultOut { get; }

// Console.Out is process-wide. Per-session interceptors must NOT clobber each other or
// a sibling session's writes would stop being intercepted (#6001). We install exactly
// one interceptor for the process and route through it; the interceptor itself is
// stateless and dispatches to Context.Current (an AsyncLocal), so a single instance
// serves every session safely.
private static int s_installed;

protected override LogLevel SinkLogLevel => LogLevel.Information;

protected override ConsoleLineBuffer GetLineBuffer() => Context.Current.ConsoleStdOutLineBuffer;
Expand All @@ -21,17 +26,20 @@ static StandardOutConsoleInterceptor()
};
}

public StandardOutConsoleInterceptor()
{
Instance = this;
}

public void Initialize()
{
Console.SetOut(this);
if (Interlocked.CompareExchange(ref s_installed, 1, 0) == 0)
{
Console.SetOut(this);
}
}

private protected override TextWriter GetOriginalOut() => DefaultOut;

private protected override void ResetDefault() => Console.SetOut(DefaultOut);
private protected override void ResetDefault()
{
// No-op: we install exactly once per process and keep that interceptor live for the
// lifetime of the process. Resetting Console.Out on a per-session dispose would tear
// down interception for any concurrent sessions sharing the test host.
}
}
7 changes: 6 additions & 1 deletion TUnit.Engine/Reporters/Html/ActivityCollector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ private static void WarnCapHitOnce()

// Process-wide pointer to the currently-running collector, used by the OTLP receiver
// (in TUnit.OpenTelemetry) to feed external spans without an explicit wiring step.
// Only one HtmlReporter runs per session, so a static slot is sufficient.
// KNOWN LIMITATION (#6001): under MTP server mode with multiple concurrent sessions in
// one host, only the first-started session claims this slot — subsequent sessions'
// HtmlReporters end up sharing collectors with the first, and once the first session
// stops the slot is cleared while later sessions are still running. Acceptable for the
// common case (single session per host) and orthogonal to the discovery/execution race
// fixes in #6001; a session-keyed registry is the proper long-term fix.
private static ActivityCollector? _current;

public static ActivityCollector? Current => _current;
Expand Down
Loading
Loading