From ee126013c85ef67b1a84ad485aff1a4be0b7dbee Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Wed, 7 Aug 2019 12:14:05 -0700 Subject: [PATCH] Query: Remove serialization of async query code This was causing deadlock issue in product because after concurrency exception the context is unusuable and in pooling scenario it caused forever wait in async queries Resolves #10914 Resolves #12138 --- ...osShapedQueryCompilingExpressionVisitor.cs | 2 +- .../RelationalDatabaseFacadeExtensions.cs | 4 +- .../Query/AsyncQueryingEnumerable.cs | 2 +- src/EFCore/Internal/ConcurrencyDetector.cs | 54 +------------------ src/EFCore/Internal/IConcurrencyDetector.cs | 10 ---- .../Query/SqlExecutorTestBase.cs | 2 +- .../ConcurrencyDetectorTestBase.cs | 14 ++--- .../Query/AsyncSimpleQueryTestBase.cs | 4 +- 8 files changed, 15 insertions(+), 77 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs index bd42ddf3f0b..a114b52bd7e 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs @@ -990,7 +990,7 @@ public async ValueTask MoveNextAsync() { try { - using (await _cosmosQueryContext.ConcurrencyDetector.EnterCriticalSectionAsync(_cancellationToken)) + using (_cosmosQueryContext.ConcurrencyDetector.EnterCriticalSection()) { if (_enumerator == null) { diff --git a/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs index 90698e79562..9470ea9f4e0 100644 --- a/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs @@ -377,7 +377,7 @@ public static async Task ExecuteSqlCommandAsync( var concurrencyDetector = facadeDependencies.ConcurrencyDetector; var logger = facadeDependencies.CommandLogger; - using (await concurrencyDetector.EnterCriticalSectionAsync(cancellationToken)) + using (concurrencyDetector.EnterCriticalSection()) { var rawSqlCommand = GetFacadeDependencies(databaseFacade).RawSqlCommandBuilder .Build(sql.Format, parameters); @@ -645,7 +645,7 @@ public static async Task ExecuteSqlRawAsync( var concurrencyDetector = facadeDependencies.ConcurrencyDetector; var logger = facadeDependencies.CommandLogger; - using (await concurrencyDetector.EnterCriticalSectionAsync(cancellationToken)) + using (concurrencyDetector.EnterCriticalSection()) { var rawSqlCommand = GetFacadeDependencies(databaseFacade).RawSqlCommandBuilder .Build(sql, parameters); diff --git a/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs b/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs index d7f933bb96a..ac7f07ec00f 100644 --- a/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs +++ b/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs @@ -85,7 +85,7 @@ public async ValueTask MoveNextAsync() { try { - using (await _relationalQueryContext.ConcurrencyDetector.EnterCriticalSectionAsync(_cancellationToken)) + using (_relationalQueryContext.ConcurrencyDetector.EnterCriticalSection()) { if (_dataReader == null) { diff --git a/src/EFCore/Internal/ConcurrencyDetector.cs b/src/EFCore/Internal/ConcurrencyDetector.cs index 65d1a9f4a50..701168db061 100644 --- a/src/EFCore/Internal/ConcurrencyDetector.cs +++ b/src/EFCore/Internal/ConcurrencyDetector.cs @@ -25,12 +25,9 @@ namespace Microsoft.EntityFrameworkCore.Internal /// The implementation does not need to be thread-safe. /// /// - public class ConcurrencyDetector : IConcurrencyDetector, IDisposable + public class ConcurrencyDetector : IConcurrencyDetector { private readonly IDisposable _disposer; - - private SemaphoreSlim _semaphore = new SemaphoreSlim(1); - private int _inCriticalSection; /// @@ -64,43 +61,6 @@ private void ExitCriticalSection() _inCriticalSection = 0; } - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual async Task EnterCriticalSectionAsync(CancellationToken cancellationToken) - { - await _semaphore.WaitAsync(cancellationToken); - - return new AsyncDisposer(EnterCriticalSection(), this); - } - - private readonly struct AsyncDisposer : IDisposable - { - private readonly IDisposable _disposable; - private readonly ConcurrencyDetector _concurrencyDetector; - - public AsyncDisposer(IDisposable disposable, ConcurrencyDetector concurrencyDetector) - { - _disposable = disposable; - _concurrencyDetector = concurrencyDetector; - } - - public void Dispose() - { - _disposable.Dispose(); - - if (_concurrencyDetector._semaphore == null) - { - throw new ObjectDisposedException(GetType().ShortDisplayName(), CoreStrings.ContextDisposed); - } - - _concurrencyDetector._semaphore.Release(); - } - } - private readonly struct Disposer : IDisposable { private readonly ConcurrencyDetector _concurrencyDetector; @@ -110,17 +70,5 @@ public Disposer(ConcurrencyDetector concurrencyDetector) public void Dispose() => _concurrencyDetector.ExitCriticalSection(); } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual void Dispose() - { - _semaphore?.Dispose(); - _semaphore = null; - } } } diff --git a/src/EFCore/Internal/IConcurrencyDetector.cs b/src/EFCore/Internal/IConcurrencyDetector.cs index a644554540e..66b1c2bc242 100644 --- a/src/EFCore/Internal/IConcurrencyDetector.cs +++ b/src/EFCore/Internal/IConcurrencyDetector.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Threading; -using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.EntityFrameworkCore.Internal @@ -31,13 +29,5 @@ public interface IConcurrencyDetector /// doing so can result in application failures when updating to a new Entity Framework Core release. /// IDisposable EnterCriticalSection(); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - Task EnterCriticalSectionAsync(CancellationToken cancellationToken); } } diff --git a/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs index ef5c6ac9a46..0b4ce93a381 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs @@ -221,7 +221,7 @@ public virtual async Task Executes_stored_procedure_with_generated_parameter_asy } } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual async Task Throws_on_concurrent_command_async() { using (var context = CreateContext()) diff --git a/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs b/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs index efe1b512b05..0d6b5d1bd1f 100644 --- a/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs +++ b/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs @@ -51,7 +51,7 @@ public virtual Task Find_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task Find_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.FindAsync(1).AsTask()); @@ -68,7 +68,7 @@ public virtual Task Count_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task Count_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.CountAsync()); @@ -85,7 +85,7 @@ public virtual Task First_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task First_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.FirstAsync()); @@ -102,7 +102,7 @@ public virtual Task Last_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task Last_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.LastAsync()); @@ -119,7 +119,7 @@ public virtual Task Single_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task Single_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.SingleAsync(p => p.ProductID == 1)); @@ -136,7 +136,7 @@ public virtual Task Any_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task Any_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.AnyAsync(p => p.ProductID < 10)); @@ -153,7 +153,7 @@ public virtual Task ToList_logs_concurrent_access_nonasync() }); } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual Task ToList_logs_concurrent_access_async() { return ConcurrencyDetectorTest(c => c.Products.ToListAsync()); diff --git a/test/EFCore.Specification.Tests/Query/AsyncSimpleQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/AsyncSimpleQueryTestBase.cs index dcad0e3b733..3a9176b5047 100644 --- a/test/EFCore.Specification.Tests/Query/AsyncSimpleQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/AsyncSimpleQueryTestBase.cs @@ -263,7 +263,7 @@ public virtual async Task Mixed_sync_async_in_query_cache() } } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual async Task Throws_on_concurrent_query_list() { using (var context = CreateContext()) @@ -298,7 +298,7 @@ public virtual async Task Throws_on_concurrent_query_list() } } - [ConditionalFact(Skip = "#12138")] + [ConditionalFact] public virtual async Task Throws_on_concurrent_query_first() { using (var context = CreateContext())