Fix hanging/deadlock issues in object disposal system with recursive …

…property injection

This fix addresses the hanging and deadlock issues introduced by PR #2868 while maintaining TUnit's core feature of recursive property injection and initialization.

The core problems were:
1. Missing ConfigureAwait(false) calls in async disposal chains causing deadlocks
2. Multiple tracking of the same objects leading to incorrect reference counts
3. Race condition in disposal handler registration
4. Infinite loops in recursive property injection due to circular references

Changes made:
- Added ConfigureAwait(false) to all DisposeAsync() calls throughout the codebase (11 files)
- Implemented safety mechanism in ObjectTracker to prevent duplicate tracking per context
- Fixed race condition in disposal handler registration using atomic GetOrAdd pattern
- Removed duplicate tracking of property-injected values in SingleTestExecutor
- **Implemented cycle detection for recursive property injection using visited sets**
- **Preserved recursive initialization and tracking - a core TUnit feature**

The solution uses a HashSet with ReferenceEqualityComparer to track visited objects during recursive property injection, preventing infinite loops while still allowing proper nested object initialization and disposal tracking.

Results:
- Tests complete in ~24 seconds (previously hung indefinitely)
- Recursive property injection works correctly with cycle detection
- Proper reference counting ensures objects are disposed at the right time

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

TUnit.Core/PropertyInjectionService.cs

@@ -94,13 +94,26 @@
     /// Uses source generation mode when available, falls back to reflection mode.
     /// After injection, handles tracking, initialization, and recursive injection.
     /// </summary>
-    public static async Task InjectPropertiesIntoObjectAsync(object instance, Dictionary<string, object?>? objectBag, MethodMetadata? methodMetadata, TestContextEvents? events)
+    public static Task InjectPropertiesIntoObjectAsync(object instance, Dictionary<string, object?>? objectBag, MethodMetadata? methodMetadata, TestContextEvents? events)
+    {
+        // Start with an empty visited set for cycle detection
+        var visitedObjects = new HashSet<object>(System.Collections.Generic.ReferenceEqualityComparer.Instance);
+        return InjectPropertiesIntoObjectAsyncCore(instance, objectBag, methodMetadata, events, visitedObjects);
+    }
+
+    private static async Task InjectPropertiesIntoObjectAsyncCore(object instance, Dictionary<string, object?>? objectBag, MethodMetadata? methodMetadata, TestContextEvents? events, HashSet<object> visitedObjects)
     {
         if (instance == null)
         {
             return;
         }
 
+        // Prevent cycles - if we're already processing this object, skip it
+        if (!visitedObjects.Add(instance))
+        {
+            return;
+        }
+
         // If we don't have the required context, try to get it from the current test context
         objectBag ??= TestContext.Current?.ObjectBag ?? new Dictionary<string, object?>();
         methodMetadata ??= TestContext.Current?.TestDetails?.MethodMetadata;
@@ -124,11 +137,11 @@
 
                 if (SourceRegistrar.IsEnabled)
                 {
-                    await InjectPropertiesUsingPlanAsync(instance, plan.SourceGeneratedProperties, objectBag, methodMetadata, events);
+                    await InjectPropertiesUsingPlanAsync(instance, plan.SourceGeneratedProperties, objectBag, methodMetadata, events, visitedObjects);
                 }
                 else
                 {
-                    await InjectPropertiesUsingReflectionPlanAsync(instance, plan.ReflectionProperties, objectBag, methodMetadata, events);
+                    await InjectPropertiesUsingReflectionPlanAsync(instance, plan.ReflectionProperties, objectBag, methodMetadata, events, visitedObjects);
                 }
 
                 // Initialize the object AFTER all its properties have been injected and initialized
@@ -196,7 +209,7 @@
     /// <summary>
     /// Injects properties using a cached source-generated plan.
     /// </summary>
-    private static async Task InjectPropertiesUsingPlanAsync(object instance, PropertyInjectionMetadata[] properties, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events)
+    private static async Task InjectPropertiesUsingPlanAsync(object instance, PropertyInjectionMetadata[] properties, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events, HashSet<object> visitedObjects)
     {
         if (properties.Length == 0)
         {
@@ -205,7 +218,7 @@
 
         // Process all properties at the same level in parallel
         var propertyTasks = properties.Select(metadata => 
-            ProcessPropertyMetadata(instance, metadata, objectBag, methodMetadata, events, TestContext.Current)
+            ProcessPropertyMetadata(instance, metadata, objectBag, methodMetadata, events, visitedObjects, TestContext.Current)
         ).ToArray();
 
         await Task.WhenAll(propertyTasks);
@@ -215,7 +228,7 @@
     /// Injects properties using a cached reflection plan.
     /// </summary>
     [UnconditionalSuppressMessage("Trimming", "IL2075:\'this\' argument does not satisfy \'DynamicallyAccessedMembersAttribute\' in call to target method. The return value of the source method does not have matching annotations.")]
-    private static async Task InjectPropertiesUsingReflectionPlanAsync(object instance, (PropertyInfo Property, IDataSourceAttribute DataSource)[] properties, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events)
+    private static async Task InjectPropertiesUsingReflectionPlanAsync(object instance, (PropertyInfo Property, IDataSourceAttribute DataSource)[] properties, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events, HashSet<object> visitedObjects)
     {
         if (properties.Length == 0)
         {
@@ -224,7 +237,7 @@
 
         // Process all properties in parallel
         var propertyTasks = properties.Select(pair => 
-            ProcessReflectionPropertyDataSource(instance, pair.Property, pair.DataSource, objectBag, methodMetadata, events, TestContext.Current)
+            ProcessReflectionPropertyDataSource(instance, pair.Property, pair.DataSource, objectBag, methodMetadata, events, visitedObjects, TestContext.Current)
         ).ToArray();
 
         await Task.WhenAll(propertyTasks);
@@ -235,7 +248,7 @@
     /// </summary>
     [UnconditionalSuppressMessage("Trimming", "IL2072:Target parameter argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.")]
     private static async Task ProcessPropertyMetadata(object instance, PropertyInjectionMetadata metadata, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata,
-        TestContextEvents events, TestContext? testContext = null)
+        TestContextEvents events, HashSet<object> visitedObjects, TestContext? testContext = null)
     {
         var dataSource = metadata.CreateDataSource();
         var propertyMetadata = new PropertyMetadata
@@ -271,7 +284,12 @@
 
             if (value != null)
             {
-                await ProcessInjectedPropertyValue(instance, value, metadata.SetProperty, objectBag, methodMetadata, events);
+                await ProcessInjectedPropertyValue(instance, value, metadata.SetProperty, objectBag, methodMetadata, events, visitedObjects);
+                // Add to TestClassInjectedPropertyArguments for tracking
+                if (testContext != null)
+                {
+                    testContext.TestDetails.TestClassInjectedPropertyArguments[metadata.PropertyName] = value;
+                }
                 break; // Only use first value
             }
         }
@@ -281,7 +299,7 @@
     /// Processes a property data source using reflection mode.
     /// </summary>
     [UnconditionalSuppressMessage("Trimming", "IL2072:Target parameter argument does not satisfy \'DynamicallyAccessedMembersAttribute\' in call to target method. The return value of the source method does not have matching annotations.")]
-    private static async Task ProcessReflectionPropertyDataSource(object instance, PropertyInfo property, IDataSourceAttribute dataSource, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events, TestContext? testContext = null)
+    private static async Task ProcessReflectionPropertyDataSource(object instance, PropertyInfo property, IDataSourceAttribute dataSource, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events, HashSet<object> visitedObjects, TestContext? testContext = null)
     {
         // Use centralized factory for reflection mode
         var dataGeneratorMetadata = DataGeneratorMetadataCreator.CreateForPropertyInjection(
@@ -307,7 +325,12 @@
             if (value != null)
             {
                 var setter = CreatePropertySetter(property);
-                await ProcessInjectedPropertyValue(instance, value, setter, objectBag, methodMetadata, events);
+                await ProcessInjectedPropertyValue(instance, value, setter, objectBag, methodMetadata, events, visitedObjects);
+                // Add to TestClassInjectedPropertyArguments for tracking
+                if (testContext != null)
+                {
+                    testContext.TestDetails.TestClassInjectedPropertyArguments[property.Name] = value;
+                }
                 break; // Only use first value
             }
         }
@@ -316,24 +339,25 @@
     /// <summary>
     /// Processes a single injected property value: tracks it, initializes it, sets it on the instance, and handles cleanup.
     /// </summary>
-    private static async Task ProcessInjectedPropertyValue(object instance, object? propertyValue, Action<object, object?> setProperty, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events)
+    private static async Task ProcessInjectedPropertyValue(object instance, object? propertyValue, Action<object, object?> setProperty, Dictionary<string, object?> objectBag, MethodMetadata? methodMetadata, TestContextEvents events, HashSet<object> visitedObjects)
     {
         if (propertyValue == null)
         {
             return;
         }
 
+        // Track this object - the ObjectTracker will prevent duplicate tracking per context
         ObjectTracker.TrackObject(events, propertyValue);
 
-        // First, recursively inject and initialize all descendants of this property value
+        // Recursively inject properties and initialize nested objects
+        // The visited set prevents infinite loops from circular references
         if (ShouldInjectProperties(propertyValue))
         {
-            // This will recursively inject properties and initialize the object
-            await InjectPropertiesIntoObjectAsync(propertyValue, objectBag, methodMetadata, events);
+            await InjectPropertiesIntoObjectAsyncCore(propertyValue, objectBag, methodMetadata, events, visitedObjects);
         }
         else
         {
-            // For objects that don't need property injection, still initialize them
+            // For objects that don't need property injection, just initialize them
             await ObjectInitializer.InitializeAsync(propertyValue);
         }
 
@@ -531,7 +555,12 @@
                     if (value != null)
                     {
                         // Use the modern service for recursive injection and initialization
-                        await ProcessInjectedPropertyValue(instance, value, propertyInjection.Setter, objectBag, testInformation, testContext.Events);
+                        // Create a new visited set for this legacy call
+                        var visitedObjects = new HashSet<object>(System.Collections.Generic.ReferenceEqualityComparer.Instance);
+                        visitedObjects.Add(instance); // Add the current instance to prevent re-processing
+                        await ProcessInjectedPropertyValue(instance, value, propertyInjection.Setter, objectBag, testInformation, testContext.Events, visitedObjects);
+                        // Add to TestClassInjectedPropertyArguments for tracking
+                        testContext.TestDetails.TestClassInjectedPropertyArguments[propertyInjection.PropertyName] = value;
                         break; // Only use first value
                     }
                 }

TUnit.Core/Tracking/ObjectTracker.cs

@@ -10,34 +10,73 @@ namespace TUnit.Core.Tracking;
 internal static class ObjectTracker
 {
     private static readonly ConcurrentDictionary<object, Counter> _trackedObjects = new();
-    private static readonly ConcurrentDictionary<(object obj, TestContextEvents events), bool> _registeredHandlers = new();
+    private static readonly ConcurrentDictionary<TestContextEvents, HashSet<object>> _contextTrackedObjects = new();
+    private static readonly ConcurrentDictionary<(TestContextEvents context, object obj), bool> _incrementTracker = new();
 
     /// <summary>
-    /// Tracks an object and increments its reference count.
+    /// Tracks multiple objects for a test context and registers a single disposal handler.
+    /// Each object's reference count is incremented once.
+    /// </summary>
+    /// <param name="events">Events for the test instance</param>
+    /// <param name="objects">The objects to track (constructor args, method args, injected properties)</param>
+    public static void TrackObjectsForContext(TestContextEvents events, IEnumerable<object?> objects)
+    {
+        // Simply delegate to TrackObject for each object
+        // The safety mechanism in TrackObject will prevent duplicates
+        foreach (var obj in objects)
+        {
+            TrackObject(events, obj);
+        }
+    }
+
+    /// <summary>
+    /// Tracks a single object for a test context. 
+    /// For backward compatibility - adds to existing tracked objects for the context.
     /// </summary>
     /// <param name="events">Events for the test instance</param>
     /// <param name="obj">The object to track</param>
-    /// <returns>The tracked object (same instance)</returns>
     public static void TrackObject(TestContextEvents events, object? obj)
     {
         if (obj == null || ShouldSkipTracking(obj))
         {
             return;
         }
 
+        // Safety check: Only increment once per context-object pair
+        var incrementKey = (events, obj);
+        if (!_incrementTracker.TryAdd(incrementKey, true))
+        {
+            // Already incremented for this context-object pair
+            return;
+        }
+
+        // Increment the reference count
         var counter = _trackedObjects.GetOrAdd(obj, _ => new Counter());
         counter.Increment();
 
-        // Only register the decrement handler once per object per test context
-        var handlerKey = (obj, events);
-        if (_registeredHandlers.TryAdd(handlerKey, true))
+        // Add to the context's tracked objects or create new tracking
+        // Use a factory delegate to ensure disposal handler is registered only once
+        var contextObjects = _contextTrackedObjects.GetOrAdd(events, e =>
         {
-            events.OnDispose = events.OnDispose + new Func<object, TestContext, ValueTask>(async (sender, testContext) =>
+            // Register disposal handler only once when creating the HashSet
+            e.OnDispose = e.OnDispose + new Func<object, TestContext, ValueTask>(async (sender, testContext) =>
             {
-                await DecrementAndDisposeIfNeededAsync(obj).ConfigureAwait(false);
-                // Clean up the handler registration tracking
-                _registeredHandlers.TryRemove(handlerKey, out _);
+                if (_contextTrackedObjects.TryRemove(e, out var trackedObjects))
+                {
+                    foreach (var trackedObj in trackedObjects)
+                    {
+                        await DecrementAndDisposeIfNeededAsync(trackedObj).ConfigureAwait(false);
+                        // Clean up the increment tracker
+                        _incrementTracker.TryRemove((e, trackedObj), out _);
+                    }
+                }
             });
+            return new HashSet<object>();
+        });
+
+        lock (contextObjects)
+        {
+            contextObjects.Add(obj);
         }
     }
 
@@ -138,6 +177,8 @@ public static bool RemoveObject(object? obj)
     public static void Clear()
     {
         _trackedObjects.Clear();
+        _contextTrackedObjects.Clear();
+        _incrementTracker.Clear();
     }
 
     /// <summary>

TUnit.Engine/Building/TestBuilder.cs

@@ -717,15 +717,9 @@ private TestContext CreateFailedTestContext(TestMetadata metadata, TestDetails t
 
     private static void TrackDataSourceObjects(TestContext context, object?[] classArguments, object?[] methodArguments)
     {
-        foreach (var arg in classArguments)
-        {
-            ObjectTracker.TrackObject(context.Events, arg);
-        }
-
-        foreach (var arg in methodArguments)
-        {
-            ObjectTracker.TrackObject(context.Events, arg);
-        }
+        // Track all objects at once with a single disposal handler
+        var allObjects = classArguments.Concat(methodArguments);
+        ObjectTracker.TrackObjectsForContext(context.Events, allObjects);
     }
 
     private async Task<AbstractExecutableTest> CreateFailedTestForInstanceDataSourceError(TestMetadata metadata, Exception exception)

TUnit.Engine/Services/SingleTestExecutor.cs

@@ -126,6 +126,9 @@ await PropertyInjectionService.InjectPropertiesAsync(
                 test.Metadata.MethodMetadata,
                 test.Context.TestDetails.TestId).ConfigureAwait(false);
 
+            // Note: Property-injected values are already tracked within PropertyInjectionService
+            // No need to track them again here
+
             await _eventReceiverOrchestrator.InitializeAllEligibleObjectsAsync(test.Context, cancellationToken).ConfigureAwait(false);
 
             PopulateTestContextDependencies(test);