diff --git a/test/EFCore.Relational.Tests/RelationalConnectionTest.cs b/test/EFCore.Relational.Tests/RelationalConnectionTest.cs index 64162f035f3..7c106aa3458 100644 --- a/test/EFCore.Relational.Tests/RelationalConnectionTest.cs +++ b/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(); + + // 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)