diff --git a/docs/docs/writing-tests/lifecycle.md b/docs/docs/writing-tests/lifecycle.md index 4176ab843cc..3f924c3983b 100644 --- a/docs/docs/writing-tests/lifecycle.md +++ b/docs/docs/writing-tests/lifecycle.md @@ -247,6 +247,10 @@ flowchart LR | `PerTestSession` | After test session ends | | `Keyed` | When all tests using that key complete | +:::note Finished test instances are released +Once a test has finished and its result has been reported, its `TestContext` drops its reference to the class instance and its per-test injected objects so they can be garbage collected. Reading `TestContext.Metadata.TestDetails.ClassInstance` for a finished test (for example from `ClassHookContext.Tests` in an `[After(Class)]` or `[After(TestSession)]` hook) no longer returns the test object. Keep your own reference if you need to inspect instances after their tests complete. +::: + ## Property Injection Lifecycle ```mermaid diff --git a/src/TUnit.Core/PlaceholderInstance.cs b/src/TUnit.Core/PlaceholderInstance.cs index 528dec26462..85e8c25fb75 100644 --- a/src/TUnit.Core/PlaceholderInstance.cs +++ b/src/TUnit.Core/PlaceholderInstance.cs @@ -12,4 +12,9 @@ private PlaceholderInstance() { // Private constructor to ensure singleton pattern } + + // Surfaces in debuggers and logs when user code reads ClassInstance before it is created or after + // a finished test has released it (#7260). + public override string ToString() => + "Test class instance not available: it has not been created yet, or it was released after the test finished."; } \ No newline at end of file diff --git a/src/TUnit.Core/TestContext.cs b/src/TUnit.Core/TestContext.cs index 259b507e395..ae07a94a91f 100644 --- a/src/TUnit.Core/TestContext.cs +++ b/src/TUnit.Core/TestContext.cs @@ -226,6 +226,27 @@ internal void RemoveFromRegistry() Volatile.Write(ref CachedReportingProperties, null); } + /// + /// Drops this context's references to the disposed class instance and untracked objects once the + /// test has finished, so per-test state can be collected while the session keeps the context alive (#7260). + /// + internal void ReleaseFinishedTestState() + { + var details = Metadata.TestDetails; + if (details.ClassInstance is not (SkippedTestInstance or PlaceholderInstance)) + { + details.ClassInstance = PlaceholderInstance.Instance; + } + + // The cached receiver lists can hold the instance when it implements event receiver interfaces. + InvalidateEventReceiverCaches(); + + // Tracked objects were untracked (and disposed when no longer shared) during cleanup. + Volatile.Write(ref _trackedObjects, null); + DisposalOnlyTrackedObjects = null; + details.ReleaseInjectedPropertyArguments(); + } + internal static void ClearReportingCaches() { // Discovery-only sessions and failures before execution can leave contexts diff --git a/src/TUnit.Core/TestDetails.cs b/src/TUnit.Core/TestDetails.cs index eabf9965c29..165c5dd3ee0 100644 --- a/src/TUnit.Core/TestDetails.cs +++ b/src/TUnit.Core/TestDetails.cs @@ -90,6 +90,10 @@ public partial class TestDetails : ITestIdentity, ITestClass, ITestMethod, ITest return Interlocked.CompareExchange(ref _testClassInjectedPropertyArguments, created, null) ?? created; } + // The cache is only read while the test registers and executes; dropping it afterwards lets + // per-test injected objects be collected while the session keeps the context alive (#7260). + internal void ReleaseInjectedPropertyArguments() => _testClassInjectedPropertyArguments = null; + private static class EmptyInjectedPropertyArguments { public static readonly IDictionary Instance = diff --git a/src/TUnit.Engine/Services/ObjectLifecycleService.cs b/src/TUnit.Engine/Services/ObjectLifecycleService.cs index fb39191b157..2d0ff7ce151 100644 --- a/src/TUnit.Engine/Services/ObjectLifecycleService.cs +++ b/src/TUnit.Engine/Services/ObjectLifecycleService.cs @@ -1,5 +1,6 @@ using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using TUnit.Core; using TUnit.Core.Discovery; using TUnit.Core.Helpers; @@ -8,7 +9,6 @@ using TUnit.Core.Tracking; #if NET using System.Diagnostics; -using System.Runtime.CompilerServices; using System.Threading; #endif @@ -33,10 +33,10 @@ internal sealed class ObjectLifecycleService : IObjectRegistry, IInitializationC private readonly ObjectGraphDiscoveryService _objectGraphDiscoveryService; private readonly ObjectTracker _objectTracker; - // Track initialization state per object - // Use ReferenceEqualityComparer to prevent objects with custom Equals from sharing initialization state - private readonly ConcurrentDictionary> _initializationTasks = - new(Core.Helpers.ReferenceEqualityComparer.Instance); + // Track initialization state per object. ConditionalWeakTable compares keys by reference, so objects + // with custom Equals never share initialization state, and it does not keep per-test objects alive + // for the rest of the session once their test has finished (#7260). + private readonly ConditionalWeakTable> _initializationTasks = new(); #if NET // Gates span creation so only the first caller for a given object creates a trace span. @@ -481,7 +481,15 @@ public async ValueTask EnsureInitializedAsync( // Slow path: need to initialize var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - existingTcs = _initializationTasks.GetOrAdd(obj, tcs); + // Lock instead of GetValue(obj, _ => tcs) so the slow path does not allocate a closure. + lock (_initializationTasks) + { + if (!_initializationTasks.TryGetValue(obj, out existingTcs)) + { + _initializationTasks.Add(obj, tcs); + existingTcs = tcs; + } + } if (existingTcs == tcs) { @@ -622,15 +630,4 @@ private async Task InitializeNestedObjectsCoreAsync( } #endregion - - /// - /// Clears the initialization cache. Called at end of test session. - /// - public void ClearCache() - { - _initializationTasks.Clear(); -#if NET - _spannedObjects.Clear(); -#endif - } } diff --git a/src/TUnit.Engine/Services/TestExecution/TestCoordinator.cs b/src/TUnit.Engine/Services/TestExecution/TestCoordinator.cs index 991fc98cea0..d8ddb2c14c3 100644 --- a/src/TUnit.Engine/Services/TestExecution/TestCoordinator.cs +++ b/src/TUnit.Engine/Services/TestExecution/TestCoordinator.cs @@ -349,6 +349,7 @@ await _eventReceiverOrchestrator.InvokeLastTestInSessionEventReceiversAsync( test.Context.TimeoutCancellationSource = null; test.Context.RemoveFromRegistry(); + test.Context.ReleaseFinishedTestState(); } } } diff --git a/tests/TUnit.Engine.Tests/Issue7260Tests.cs b/tests/TUnit.Engine.Tests/Issue7260Tests.cs new file mode 100644 index 00000000000..dfc24100681 --- /dev/null +++ b/tests/TUnit.Engine.Tests/Issue7260Tests.cs @@ -0,0 +1,23 @@ +using Shouldly; +using TUnit.Engine.Tests.Enums; + +namespace TUnit.Engine.Tests; + +/// +/// A finished test's class instance is collectable before the session ends (#7260). +/// +public class Issue7260Tests(TestMode testMode) : InvokableTestBase(testMode) +{ + [Test] + public async Task Finished_Test_Instance_Is_Collectable_Before_Session_Ends() + { + await RunTestsWithFilter( + "/*/TUnit.TestProject.Bugs._7260/*/*", + [ + result => result.ResultSummary.Outcome.ShouldBe("Completed"), + result => result.ResultSummary.Counters.Total.ShouldBe(4), + result => result.ResultSummary.Counters.Passed.ShouldBe(4), + result => result.ResultSummary.Counters.Failed.ShouldBe(0) + ]); + } +} diff --git a/tests/TUnit.TestProject/Bugs/2867/DisposalNotCalledTests.cs b/tests/TUnit.TestProject/Bugs/2867/DisposalNotCalledTests.cs index 8bb82577307..4413d0360b8 100644 --- a/tests/TUnit.TestProject/Bugs/2867/DisposalNotCalledTests.cs +++ b/tests/TUnit.TestProject/Bugs/2867/DisposalNotCalledTests.cs @@ -106,7 +106,14 @@ public class PerClassSharedDisposalTest : IAsyncDisposable private static readonly List _webAppIds = new(); private static readonly List _serviceIds = new(); + // Finished tests release their class instance (#7260), so keep our own references to inspect. + private static readonly System.Collections.Concurrent.ConcurrentBag _instances = []; private bool _isDisposed; + + public PerClassSharedDisposalTest() + { + _instances.Add(this); + } [Before(Class)] public static void BeforeClass() @@ -115,6 +122,7 @@ public static void BeforeClass() DisposalNotCalledTests.DisposableService.Reset(); _webAppIds.Clear(); _serviceIds.Clear(); + _instances.Clear(); } public ValueTask DisposeAsync() @@ -215,8 +223,8 @@ public static async Task VerifyDisposalAfterClass(ClassHookContext context) await Assert.That(_serviceIds.Distinct().Count()).IsEqualTo(1); // Verify all test instances themselves are disposed - var testInstances = context.Tests.Select(t => t.Metadata.TestDetails.ClassInstance).OfType().ToList(); - foreach (var instance in testInstances) + await Assert.That(_instances).IsNotEmpty(); + foreach (var instance in _instances) { await Assert.That(instance._isDisposed).IsTrue(); } diff --git a/tests/TUnit.TestProject/Bugs/3219/ClassDataSourceRetryTests.cs b/tests/TUnit.TestProject/Bugs/3219/ClassDataSourceRetryTests.cs index 878e1b6a81b..1ccad9d1182 100644 --- a/tests/TUnit.TestProject/Bugs/3219/ClassDataSourceRetryTests.cs +++ b/tests/TUnit.TestProject/Bugs/3219/ClassDataSourceRetryTests.cs @@ -37,6 +37,8 @@ public class ClassDataSourceRetryTests { private static bool _wasExecuted; private static int _attemptCount; + // Finished tests release their class instance (#7260), so keep our own reference to inspect. + private static DataClass? _dataClass; [ClassDataSource(Shared = SharedType.PerTestSession)] public required DataClass DataClass { get; init; } @@ -53,6 +55,7 @@ public async Task TestThatFailsAndRetries() { _wasExecuted = true; _attemptCount++; + _dataClass = DataClass; await Assert.That(DataClass.Value).IsEqualTo(42); @@ -75,9 +78,7 @@ public async Task VerifyNotDisposedDuringTests() [After(TestSession)] public static async Task VerifyDisposalAfterTestSession(TestSessionContext context) { - var classInstance = context.TestClasses.FirstOrDefault(x => x.ClassType == typeof(ClassDataSourceRetryTests))?.Tests.FirstOrDefault()?.Metadata.TestDetails.ClassInstance as ClassDataSourceRetryTests; - - var dataClass = classInstance?.DataClass; + var dataClass = _dataClass; if (dataClass != null) { diff --git a/tests/TUnit.TestProject/Bugs/_7260/FinishedTestInjectedObjectReleaseTests.cs b/tests/TUnit.TestProject/Bugs/_7260/FinishedTestInjectedObjectReleaseTests.cs new file mode 100644 index 00000000000..9dddd9f291f --- /dev/null +++ b/tests/TUnit.TestProject/Bugs/_7260/FinishedTestInjectedObjectReleaseTests.cs @@ -0,0 +1,40 @@ +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._7260; + +/// +/// A per-test injected object is tracked by the test's context; once the test has finished and the +/// object has been untracked, the context must not keep it reachable either (#7260). +/// +[EngineTest(ExpectedResult.Pass)] +public class FinishedTestInjectedObjectReleaseTests +{ + private static WeakReference? _finishedInjectedObject; + + [ClassDataSource(Shared = SharedType.None)] + public required PerTestPayload Payload { get; init; } + + [Test] + public void Holds_Injected_Object() + { + _finishedInjectedObject = new WeakReference(Payload); + } + + [Test] + [DependsOn(nameof(Holds_Injected_Object))] + public async Task Finished_Injected_Object_Is_Collectable() + { + await Assert.That(_finishedInjectedObject).IsNotNull(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + await Assert.That(_finishedInjectedObject!.IsAlive).IsFalse(); + } + + public class PerTestPayload + { + public byte[] Data { get; } = new byte[1024 * 1024]; + } +} diff --git a/tests/TUnit.TestProject/Bugs/_7260/FinishedTestInstanceReleaseTests.cs b/tests/TUnit.TestProject/Bugs/_7260/FinishedTestInstanceReleaseTests.cs new file mode 100644 index 00000000000..690afa90fe2 --- /dev/null +++ b/tests/TUnit.TestProject/Bugs/_7260/FinishedTestInstanceReleaseTests.cs @@ -0,0 +1,35 @@ +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._7260; + +/// +/// A finished test's class instance must not stay reachable through its TestContext, which the +/// session keeps until it ends, so state held in instance fields can be collected (#7260). +/// +[EngineTest(ExpectedResult.Pass)] +public class FinishedTestInstanceReleaseTests +{ + private static WeakReference? _finishedInstance; + + private byte[]? _payload; + + [Test] + public void Holds_State_In_Instance_Field() + { + _payload = new byte[1024 * 1024]; + _finishedInstance = new WeakReference(this); + } + + [Test] + [DependsOn(nameof(Holds_State_In_Instance_Field))] + public async Task Finished_Instance_Is_Collectable() + { + await Assert.That(_finishedInstance).IsNotNull(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + await Assert.That(_finishedInstance!.IsAlive).IsFalse(); + } +} diff --git a/tests/TUnit.TestProject/DisposalRegressionTests.cs b/tests/TUnit.TestProject/DisposalRegressionTests.cs index bf4c3cd9337..8570a4b3cf2 100644 --- a/tests/TUnit.TestProject/DisposalRegressionTests.cs +++ b/tests/TUnit.TestProject/DisposalRegressionTests.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using TUnit.TestProject.Attributes; namespace TUnit.TestProject; @@ -9,6 +10,14 @@ namespace TUnit.TestProject; [EngineTest(ExpectedResult.Pass)] public class DisposalRegressionTests : IAsyncDisposable { + // Finished tests release their class instance (#7260), so keep our own references to inspect. + private static readonly ConcurrentBag Instances = []; + + public DisposalRegressionTests() + { + Instances.Add(this); + } + [ClassDataSource(Shared = SharedType.PerTestSession)] public required DisposableTestData InjectedData { get; init; } @@ -32,7 +41,9 @@ public async Task TestExecutes() public static async Task VerifyDisposal(ClassHookContext context) { // After the test class is done, both the test instance and injected property should be disposed - foreach (var testInstance in context.Tests.Select(x => x.Metadata.TestDetails.ClassInstance).OfType()) + await Assert.That(Instances).IsNotEmpty(); + + foreach (var testInstance in Instances) { await Assert.That(testInstance.IsDisposed).IsTrue(); await Assert.That(testInstance.InjectedData.IsDisposed).IsTrue(); diff --git a/tests/TUnit.TestProject/IDisposableTests.cs b/tests/TUnit.TestProject/IDisposableTests.cs index a1dc7a1be33..231e5ca5a67 100644 --- a/tests/TUnit.TestProject/IDisposableTests.cs +++ b/tests/TUnit.TestProject/IDisposableTests.cs @@ -1,10 +1,19 @@ -using TUnit.TestProject.Attributes; +using System.Collections.Concurrent; +using TUnit.TestProject.Attributes; namespace TUnit.TestProject; [EngineTest(ExpectedResult.Pass)] public class DisposableTests : IDisposable { + // Finished tests release their class instance (#7260), so keep our own references to inspect. + private static readonly ConcurrentBag Instances = []; + + public DisposableTests() + { + Instances.Add(this); + } + [Test] public void One() { @@ -26,9 +35,11 @@ public async Task Three() [After(Class)] public static async Task AssertDisposed(ClassHookContext context) { - foreach (var disposableTestse in context.Tests.Select(x => x.Metadata.TestDetails.ClassInstance).OfType()) + await Assert.That(Instances).IsNotEmpty(); + + foreach (var disposableTests in Instances) { - await Assert.That(disposableTestse.IsDisposed).IsTrue(); + await Assert.That(disposableTests.IsDisposed).IsTrue(); } } diff --git a/tests/TUnit.TestProject/SharedDisposalTest.cs b/tests/TUnit.TestProject/SharedDisposalTest.cs index 3e348864e2d..275687d70dd 100644 --- a/tests/TUnit.TestProject/SharedDisposalTest.cs +++ b/tests/TUnit.TestProject/SharedDisposalTest.cs @@ -41,11 +41,15 @@ public static void Reset() [ClassDataSource(Shared = SharedType.PerClass)] public class SharedDisposalTest { + // Finished tests release their class instance (#7260), so keep our own references to inspect. + private static readonly System.Collections.Concurrent.ConcurrentBag Instances = []; + private readonly DisposableSharedInstance sharedInstance; public SharedDisposalTest(DisposableSharedInstance sharedInstance) { this.sharedInstance = sharedInstance; + Instances.Add(this); } [Test] public async Task Test1_ShouldUseSharedInstance() @@ -102,7 +106,7 @@ public static async Task VerifySharedInstanceDisposal(ClassHookContext context) } // Print the test instances to see if they were using the same shared instance - var testInstances = context.Tests.Select(t => t.Metadata.TestDetails.ClassInstance).OfType().ToList(); + var testInstances = Instances.ToList(); Console.WriteLine($"[AfterClass] Test instances: {testInstances.Count}"); for (var i = 0; i < testInstances.Count; i++) {