Fix test hanging issue: remove manual OnDispose event triggering

claude[bot] · thomhurst · claude[bot] · commit 816da19fa432 · 2025-08-16T08:05:04.000Z
The original fix introduced a deadlock/race condition by manually invoking
OnDispose events after tracking objects with ObjectTracker. This caused
tests to hang because:

1. ObjectTracker.TrackObject() adds disposal callbacks to events.OnDispose
2. Manual invocation of events.OnDispose triggered these callbacks immediately
3. This led to premature disposal and potential deadlocks in the disposal system

Fixed by removing manual OnDispose invocations and letting ObjectTracker
handle disposal automatically through the normal test lifecycle.

Co-authored-by: Tom Longhurst &lt;thomhurst@users.noreply.github.com&gt;
diff --git a/TUnit.Engine/Services/SingleTestExecutor.cs b/TUnit.Engine/Services/SingleTestExecutor.cs
@@ -268,36 +268,12 @@ private async Task<TestResult> HandleSkippedTestInternalAsync(AbstractExecutable
             instance is not SkippedTestInstance && 
             instance is not PlaceholderInstance)
         {
-            try
-            {
-                // For skipped tests, also ensure the test instance is tracked for disposal
-                if (instance is IAsyncDisposable or IDisposable)
-                {
-                    ObjectTracker.TrackObject(test.Context.Events, instance);
-                }
-                
-                // Trigger disposal of all tracked objects (including test instance and injected properties)
-                if (test.Context.Events.OnDispose != null)
-                {
-                    foreach (var invocation in test.Context.Events.OnDispose.InvocationList.OrderBy(x => x.Order))
-                    {
-                        try
-                        {
-                            await invocation.InvokeAsync(test.Context, test.Context).ConfigureAwait(false);
-                        }
-                        catch (Exception ex)
-                        {
-                            // Log but don't throw - we still need to dispose other objects
-                            await _logger.LogErrorAsync($"Error during OnDispose event for skipped test: {ex.Message}").ConfigureAwait(false);
-                        }
-                    }
-                }
-            }
-            catch (Exception ex)
+            // For skipped tests, also ensure the test instance is tracked for disposal
+            if (instance is IAsyncDisposable or IDisposable)
             {
-                // Log disposal errors but don't fail the skipped test
-                await _logger.LogErrorAsync($"Error disposing skipped test instance: {ex.Message}").ConfigureAwait(false);
+                ObjectTracker.TrackObject(test.Context.Events, instance);
             }
+            // Note: ObjectTracker will handle disposal automatically when the test context ends
         }
 
         return test.Result;
@@ -344,23 +320,8 @@ private async Task ExecuteTestWithHooksAsync(AbstractExecutableTest test, object
         }
         finally
         {
-            // Trigger disposal of all tracked objects (including test instance and injected properties)
-            // The ObjectTracker system ensures proper disposal order and reference counting
-            if (test.Context.Events.OnDispose != null)
-            {
-                foreach (var invocation in test.Context.Events.OnDispose.InvocationList.OrderBy(x => x.Order))
-                {
-                    try
-                    {
-                        await invocation.InvokeAsync(test.Context, test.Context).ConfigureAwait(false);
-                    }
-                    catch (Exception ex)
-                    {
-                        // Log but don't throw - we still need to dispose other objects
-                        await _logger.LogErrorAsync($"Error during OnDispose event: {ex.Message}").ConfigureAwait(false);
-                    }
-                }
-            }
+            // Note: ObjectTracker will handle disposal automatically when the test context ends
+            // No need to manually trigger disposal events here as it could cause deadlocks
         }
 
         if (testException != null)
