Fix flaky test by creating proper race condition between HandleTransactionCompleted and ClearTransactions (#37101)

Fixes #37086

Co-authored-by: cincuranet <4540597+cincuranet@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>

Author: 3 people

test/EFCore.Relational.Tests/RelationalConnectionTest.cs

@@ -1085,76 +1085,61 @@ public async Task Reports_command_diagnostic_on_cancellation()
     }
 
     [ConditionalFact]
-    public void HandleTransactionCompleted_with_concurrent_ClearTransactions_is_thread_safe()
+    public async Task HandleTransactionCompleted_with_concurrent_ClearTransactions_is_thread_safe()
     {
         // This test verifies the fix for the race condition where HandleTransactionCompleted
         // could be called on a different thread while ClearTransactions is executing.
-        var exceptions = new List<Exception>();
+
+        // Test with scope.Complete() before ResetState()
         for (var i = 0; i < Environment.ProcessorCount; i++)
         {
             var connection = new FakeRelationalConnection(
                 CreateOptions(new FakeRelationalOptionsExtension().WithConnectionString("Database=ConcurrencyTest")));
 
-            using var scope = new TransactionScope();
+            var scope = new TransactionScope();
             connection.Open();
+            scope.Complete();
 
-            var random = new Random();
-            var resetFirst = random.Next(0, 1) == 0;
-            var tasks = new Task[2];
-            tasks[0] = Task.Run(async () =>
+            // Start the reset task first, which will yield and then try to reset
+            var resetTask = Task.Run(() =>
             {
-                try
-                {
-                    // Small delay to increase chance of race condition
-                    await Task.Yield();
-
-                    if (resetFirst)
-                    {
-                        ((IResettableService)connection).ResetState();
-                    }
-                    else
-                    {
-                        scope.Complete();
-                    }
-                }
-                catch (Exception ex)
-                {
-                    lock (exceptions)
-                    {
-                        exceptions.Add(ex);
-                    }
-                }
+                // ResetState calls ClearTransactions which might race with HandleTransactionCompleted
+                ((IResettableService)connection).ResetState();
             });
 
-            tasks[1] = Task.Run(async () =>
+            // Dispose the scope on the main thread, which will trigger the TransactionCompleted event
+            // The event handler (HandleTransactionCompleted) may execute on a different thread and race
+            // with the ClearTransactions call in resetTask
+            scope.Dispose();
+
+            await resetTask;
+        }
+
+        // Test with ResetState() before scope.Complete()
+        for (var i = 0; i < Environment.ProcessorCount; i++)
+        {
+            var connection = new FakeRelationalConnection(
+                CreateOptions(new FakeRelationalOptionsExtension().WithConnectionString("Database=ConcurrencyTest")));
+
+            var scope = new TransactionScope();
+            connection.Open();
+
+            // Start the reset task first
+            var resetTask = Task.Run(async () =>
             {
-                try
-                {
-                    // Small delay to increase chance of race condition
-                    await Task.Yield();
-
-                    if (resetFirst)
-                    {
-                        scope.Complete();
-                    }
-                    else
-                    {
-                        ((IResettableService)connection).ResetState();
-                    }
-                }
-                catch (Exception ex)
-                {
-                    lock (exceptions)
-                    {
-                        exceptions.Add(ex);
-                    }
-                }
+                // Small delay to increase chance of race condition
+                await Task.Yield();
+
+                // ResetState calls ClearTransactions which might race with HandleTransactionCompleted
+                ((IResettableService)connection).ResetState();
             });
 
-            Task.WaitAll(tasks, TimeSpan.FromSeconds(10));
-        }
+            // Complete and dispose the scope, which will trigger the TransactionCompleted event
+            scope.Complete();
+            scope.Dispose();
 
-        Assert.Empty(exceptions);
+            await resetTask;
+        }
     }
 
     private static IDbContextOptions CreateOptions(params RelationalOptionsExtension[] optionsExtensions)