diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs index 4a4ea51deba..bd42ddf3f0b 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs @@ -882,29 +882,32 @@ public bool MoveNext() { try { - if (_enumerator == null) + using (_cosmosQueryContext.ConcurrencyDetector.EnterCriticalSection()) { - var selectExpression = (SelectExpression)new InExpressionValuesExpandingExpressionVisitor( - _sqlExpressionFactory, _cosmosQueryContext.ParameterValues).Visit(_selectExpression); + if (_enumerator == null) + { + var selectExpression = (SelectExpression)new InExpressionValuesExpandingExpressionVisitor( + _sqlExpressionFactory, _cosmosQueryContext.ParameterValues).Visit(_selectExpression); - var sqlQuery = _querySqlGeneratorFactory.Create().GetSqlQuery( - selectExpression, _cosmosQueryContext.ParameterValues); + var sqlQuery = _querySqlGeneratorFactory.Create().GetSqlQuery( + selectExpression, _cosmosQueryContext.ParameterValues); - _enumerator = _cosmosQueryContext.CosmosClient - .ExecuteSqlQuery( - _selectExpression.Container, - sqlQuery) - .GetEnumerator(); - } + _enumerator = _cosmosQueryContext.CosmosClient + .ExecuteSqlQuery( + _selectExpression.Container, + sqlQuery) + .GetEnumerator(); + } - var hasNext = _enumerator.MoveNext(); + var hasNext = _enumerator.MoveNext(); - Current - = hasNext - ? _shaper(_cosmosQueryContext, _enumerator.Current) - : default; + Current + = hasNext + ? _shaper(_cosmosQueryContext, _enumerator.Current) + : default; - return hasNext; + return hasNext; + } } catch (Exception exception) { @@ -987,27 +990,30 @@ public async ValueTask MoveNextAsync() { try { - if (_enumerator == null) + using (await _cosmosQueryContext.ConcurrencyDetector.EnterCriticalSectionAsync(_cancellationToken)) { - var selectExpression = (SelectExpression)new InExpressionValuesExpandingExpressionVisitor( - _sqlExpressionFactory, _cosmosQueryContext.ParameterValues).Visit(_selectExpression); + if (_enumerator == null) + { + var selectExpression = (SelectExpression)new InExpressionValuesExpandingExpressionVisitor( + _sqlExpressionFactory, _cosmosQueryContext.ParameterValues).Visit(_selectExpression); - _enumerator = _cosmosQueryContext.CosmosClient - .ExecuteSqlQueryAsync( - _selectExpression.Container, - _querySqlGeneratorFactory.Create().GetSqlQuery(selectExpression, _cosmosQueryContext.ParameterValues)) - .GetAsyncEnumerator(_cancellationToken); + _enumerator = _cosmosQueryContext.CosmosClient + .ExecuteSqlQueryAsync( + _selectExpression.Container, + _querySqlGeneratorFactory.Create().GetSqlQuery(selectExpression, _cosmosQueryContext.ParameterValues)) + .GetAsyncEnumerator(_cancellationToken); - } + } - var hasNext = await _enumerator.MoveNextAsync(); + var hasNext = await _enumerator.MoveNextAsync(); - Current - = hasNext - ? _shaper(_cosmosQueryContext, _enumerator.Current) - : default; + Current + = hasNext + ? _shaper(_cosmosQueryContext, _enumerator.Current) + : default; - return hasNext; + return hasNext; + } } catch (Exception exception) { diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryExpressionVisitor.cs index cbfcfd82d2f..ed76a03fac7 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryExpressionVisitor.cs @@ -148,18 +148,21 @@ public bool MoveNext() { try { - if (_enumerator == null) + using (_queryContext.ConcurrencyDetector.EnterCriticalSection()) { - _enumerator = _innerEnumerable.GetEnumerator(); - } + if (_enumerator == null) + { + _enumerator = _innerEnumerable.GetEnumerator(); + } - var hasNext = _enumerator.MoveNext(); + var hasNext = _enumerator.MoveNext(); - Current = hasNext - ? _shaper(_queryContext, _enumerator.Current) - : default; + Current = hasNext + ? _shaper(_queryContext, _enumerator.Current) + : default; - return hasNext; + return hasNext; + } } catch (Exception exception) { @@ -226,20 +229,23 @@ public ValueTask MoveNextAsync() { try { - _cancellationToken.ThrowIfCancellationRequested(); - - if (_enumerator == null) + using (_queryContext.ConcurrencyDetector.EnterCriticalSection()) { - _enumerator = _innerEnumerable.GetEnumerator(); - } + _cancellationToken.ThrowIfCancellationRequested(); + + if (_enumerator == null) + { + _enumerator = _innerEnumerable.GetEnumerator(); + } - var hasNext = _enumerator.MoveNext(); + var hasNext = _enumerator.MoveNext(); - Current = hasNext - ? _shaper(_queryContext, _enumerator.Current) - : default; + Current = hasNext + ? _shaper(_queryContext, _enumerator.Current) + : default; - return new ValueTask(hasNext); + return new ValueTask(hasNext); + } } catch (Exception exception) { diff --git a/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs b/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs index 349dc7f743d..d7f933bb96a 100644 --- a/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs +++ b/src/EFCore.Relational/Query/AsyncQueryingEnumerable.cs @@ -85,81 +85,85 @@ public async ValueTask MoveNextAsync() { try { - if (_dataReader == null) + using (await _relationalQueryContext.ConcurrencyDetector.EnterCriticalSectionAsync(_cancellationToken)) { - var selectExpression = new ParameterValueBasedSelectExpressionOptimizer( - _sqlExpressionFactory, - _parameterNameGeneratorFactory) - .Optimize(_selectExpression, _relationalQueryContext.ParameterValues); - - var relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression); - - _dataReader - = await relationalCommand.ExecuteReaderAsync( - new RelationalCommandParameterObject( - _relationalQueryContext.Connection, - _relationalQueryContext.ParameterValues, - _relationalQueryContext.Context, - _relationalQueryContext.CommandLogger), - _cancellationToken); - - if (selectExpression.IsNonComposedFromSql()) + if (_dataReader == null) { - var projection = _selectExpression.Projection.ToList(); - var readerColumns = Enumerable.Range(0, _dataReader.DbDataReader.FieldCount) - .ToDictionary(i => _dataReader.DbDataReader.GetName(i), i => i, StringComparer.OrdinalIgnoreCase); - - _indexMap = new int[projection.Count]; - for (var i = 0; i < projection.Count; i++) + var selectExpression = new ParameterValueBasedSelectExpressionOptimizer( + _sqlExpressionFactory, + _parameterNameGeneratorFactory) + .Optimize(_selectExpression, _relationalQueryContext.ParameterValues); + + var relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression); + + _dataReader + = await relationalCommand.ExecuteReaderAsync( + new RelationalCommandParameterObject( + _relationalQueryContext.Connection, + _relationalQueryContext.ParameterValues, + _relationalQueryContext.Context, + _relationalQueryContext.CommandLogger), + _cancellationToken); + + if (selectExpression.IsNonComposedFromSql()) { - if (projection[i].Expression is ColumnExpression columnExpression) + var projection = _selectExpression.Projection.ToList(); + var readerColumns = Enumerable.Range(0, _dataReader.DbDataReader.FieldCount) + .ToDictionary(i => _dataReader.DbDataReader.GetName(i), i => i, StringComparer.OrdinalIgnoreCase); + + _indexMap = new int[projection.Count]; + for (var i = 0; i < projection.Count; i++) { - var columnName = columnExpression.Name; - if (columnName != null) + if (projection[i].Expression is ColumnExpression columnExpression) { - if (!readerColumns.TryGetValue(columnName, out var ordinal)) + var columnName = columnExpression.Name; + if (columnName != null) { - throw new InvalidOperationException(RelationalStrings.FromSqlMissingColumn(columnName)); - } + if (!readerColumns.TryGetValue(columnName, out var ordinal)) + { + throw new InvalidOperationException(RelationalStrings.FromSqlMissingColumn(columnName)); + } - _indexMap[i] = ordinal; + _indexMap[i] = ordinal; + } } } } - } - else - { - _indexMap = null; + else + { + _indexMap = null; + } + + _resultCoordinator = new ResultCoordinator(); } - _resultCoordinator = new ResultCoordinator(); - } - var hasNext = _resultCoordinator.HasNext ?? await _dataReader.ReadAsync(); - Current = default; + var hasNext = _resultCoordinator.HasNext ?? await _dataReader.ReadAsync(); + Current = default; - if (hasNext) - { - while (true) + if (hasNext) { - _resultCoordinator.ResultReady = true; - _resultCoordinator.HasNext = null; - Current = _shaper(_relationalQueryContext, _dataReader.DbDataReader, Current, _indexMap, _resultCoordinator); - if (_resultCoordinator.ResultReady) + while (true) { - break; - } + _resultCoordinator.ResultReady = true; + _resultCoordinator.HasNext = null; + Current = _shaper(_relationalQueryContext, _dataReader.DbDataReader, Current, _indexMap, _resultCoordinator); + if (_resultCoordinator.ResultReady) + { + break; + } - if (!await _dataReader.ReadAsync()) - { - _resultCoordinator.HasNext = false; + if (!await _dataReader.ReadAsync()) + { + _resultCoordinator.HasNext = false; - break; + break; + } } } - } - return hasNext; + return hasNext; + } } catch (Exception exception) { diff --git a/src/EFCore.Relational/Query/Internal/QueryingEnumerable.cs b/src/EFCore.Relational/Query/Internal/QueryingEnumerable.cs index e06ad0aedda..a377eac9f55 100644 --- a/src/EFCore.Relational/Query/Internal/QueryingEnumerable.cs +++ b/src/EFCore.Relational/Query/Internal/QueryingEnumerable.cs @@ -82,80 +82,83 @@ public bool MoveNext() { try { - if (_dataReader == null) + using (_relationalQueryContext.ConcurrencyDetector.EnterCriticalSection()) { - var selectExpression = new ParameterValueBasedSelectExpressionOptimizer( - _sqlExpressionFactory, - _parameterNameGeneratorFactory) - .Optimize(_selectExpression, _relationalQueryContext.ParameterValues); - - var relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression); - - _dataReader - = relationalCommand.ExecuteReader( - new RelationalCommandParameterObject( - _relationalQueryContext.Connection, - _relationalQueryContext.ParameterValues, - _relationalQueryContext.Context, - _relationalQueryContext.CommandLogger)); - - if (selectExpression.IsNonComposedFromSql()) + if (_dataReader == null) { - var projection = _selectExpression.Projection.ToList(); - var readerColumns = Enumerable.Range(0, _dataReader.DbDataReader.FieldCount) - .ToDictionary(i => _dataReader.DbDataReader.GetName(i), i => i, StringComparer.OrdinalIgnoreCase); - - _indexMap = new int[projection.Count]; - for (var i = 0; i < projection.Count; i++) + var selectExpression = new ParameterValueBasedSelectExpressionOptimizer( + _sqlExpressionFactory, + _parameterNameGeneratorFactory) + .Optimize(_selectExpression, _relationalQueryContext.ParameterValues); + + var relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression); + + _dataReader + = relationalCommand.ExecuteReader( + new RelationalCommandParameterObject( + _relationalQueryContext.Connection, + _relationalQueryContext.ParameterValues, + _relationalQueryContext.Context, + _relationalQueryContext.CommandLogger)); + + if (selectExpression.IsNonComposedFromSql()) { - if (projection[i].Expression is ColumnExpression columnExpression) + var projection = _selectExpression.Projection.ToList(); + var readerColumns = Enumerable.Range(0, _dataReader.DbDataReader.FieldCount) + .ToDictionary(i => _dataReader.DbDataReader.GetName(i), i => i, StringComparer.OrdinalIgnoreCase); + + _indexMap = new int[projection.Count]; + for (var i = 0; i < projection.Count; i++) { - var columnName = columnExpression.Name; - if (columnName != null) + if (projection[i].Expression is ColumnExpression columnExpression) { - if (!readerColumns.TryGetValue(columnName, out var ordinal)) + var columnName = columnExpression.Name; + if (columnName != null) { - throw new InvalidOperationException(RelationalStrings.FromSqlMissingColumn(columnName)); - } + if (!readerColumns.TryGetValue(columnName, out var ordinal)) + { + throw new InvalidOperationException(RelationalStrings.FromSqlMissingColumn(columnName)); + } - _indexMap[i] = ordinal; + _indexMap[i] = ordinal; + } } } } - } - else - { - _indexMap = null; - } + else + { + _indexMap = null; + } - _resultCoordinator = new ResultCoordinator(); - } + _resultCoordinator = new ResultCoordinator(); + } - var hasNext = _resultCoordinator.HasNext ?? _dataReader.Read(); - Current = default; + var hasNext = _resultCoordinator.HasNext ?? _dataReader.Read(); + Current = default; - if (hasNext) - { - while (true) + if (hasNext) { - _resultCoordinator.ResultReady = true; - _resultCoordinator.HasNext = null; - Current = _shaper(_relationalQueryContext, _dataReader.DbDataReader, Current, _indexMap, _resultCoordinator); - if (_resultCoordinator.ResultReady) + while (true) { - break; - } + _resultCoordinator.ResultReady = true; + _resultCoordinator.HasNext = null; + Current = _shaper(_relationalQueryContext, _dataReader.DbDataReader, Current, _indexMap, _resultCoordinator); + if (_resultCoordinator.ResultReady) + { + break; + } - if (!_dataReader.Read()) - { - _resultCoordinator.HasNext = false; + if (!_dataReader.Read()) + { + _resultCoordinator.HasNext = false; - break; + break; + } } } - } - return hasNext; + return hasNext; + } } catch (Exception exception) { diff --git a/test/EFCore.Cosmos.FunctionalTests/ConcurrencyDetectorCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/ConcurrencyDetectorCosmosTest.cs new file mode 100644 index 00000000000..20dd8127ea9 --- /dev/null +++ b/test/EFCore.Cosmos.FunctionalTests/ConcurrencyDetectorCosmosTest.cs @@ -0,0 +1,30 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Query; +using Microsoft.EntityFrameworkCore.TestUtilities; +using Xunit; + +namespace Microsoft.EntityFrameworkCore.Cosmos +{ + public class ConcurrencyDetectorCosmosTest : ConcurrencyDetectorTestBase> + { + public ConcurrencyDetectorCosmosTest(NorthwindQueryCosmosFixture fixture) + : base(fixture) + { + } + + [ConditionalTheory(Skip = "Issue #14935")] + public override Task Any_logs_concurrent_access_nonasync() + { + return base.Any_logs_concurrent_access_nonasync(); + } + + [ConditionalTheory(Skip = "Issue #14935")] + public override Task Any_logs_concurrent_access_async() + { + return base.Any_logs_concurrent_access_async(); + } + } +} diff --git a/test/EFCore.InMemory.FunctionalTests/ConcurrencyDetectorInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/ConcurrencyDetectorInMemoryTest.cs index 74d9bcb57c5..f7d00c7236d 100644 --- a/test/EFCore.InMemory.FunctionalTests/ConcurrencyDetectorInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/ConcurrencyDetectorInMemoryTest.cs @@ -6,8 +6,7 @@ namespace Microsoft.EntityFrameworkCore { - // TODO: See Issue#14534 - internal class ConcurrencyDetectorInMemoryTest : ConcurrencyDetectorTestBase> + public class ConcurrencyDetectorInMemoryTest : ConcurrencyDetectorTestBase> { public ConcurrencyDetectorInMemoryTest(NorthwindQueryInMemoryFixture fixture) : base(fixture) diff --git a/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs index 64f79a63f25..ef5c6ac9a46 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/SqlExecutorTestBase.cs @@ -52,7 +52,7 @@ public virtual void Executes_stored_procedure_with_generated_parameter() } } - [ConditionalFact(Skip = "Issue#14534")] + [ConditionalFact] public virtual void Throws_on_concurrent_command() { using (var context = CreateContext()) diff --git a/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs b/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs index ebde629dc22..efe1b512b05 100644 --- a/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs +++ b/test/EFCore.Specification.Tests/ConcurrencyDetectorTestBase.cs @@ -163,7 +163,7 @@ protected virtual async Task ConcurrencyDetectorTest(Func().EnterCriticalSection()) { diff --git a/test/EFCore.SqlServer.FunctionalTests/ConcurrencyDetectorSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/ConcurrencyDetectorSqlServerTest.cs index 70e185ec208..be9e761e64b 100644 --- a/test/EFCore.SqlServer.FunctionalTests/ConcurrencyDetectorSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/ConcurrencyDetectorSqlServerTest.cs @@ -10,8 +10,7 @@ namespace Microsoft.EntityFrameworkCore { - // TODO: See Issue#14534 - internal class ConcurrencyDetectorSqlServerTest : ConcurrencyDetectorRelationalTestBase< + public class ConcurrencyDetectorSqlServerTest : ConcurrencyDetectorRelationalTestBase< NorthwindQuerySqlServerFixture> { public ConcurrencyDetectorSqlServerTest(NorthwindQuerySqlServerFixture fixture) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index 66878a1d75b..5e9c1ed8696 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -6041,6 +6041,83 @@ public class TMandator15204 #endregion + #region Bug15518 + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public virtual void Nested_queries_does_not_cause_concurrency_exception_sync(bool tracking) + { + using (CreateDatabase15518()) + { + using (var context = new MyContext15518(_options)) + { + var query = context.Repos.OrderBy(r => r.Id).Where(r => r.Id > 0); + query = tracking ? query.AsTracking() : query.AsNoTracking(); + + foreach (var a in query) + { + foreach (var b in query) + { + } + } + } + } + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public virtual async Task Nested_queries_does_not_cause_concurrency_exception_async(bool tracking) + { + using (CreateDatabase15518()) + { + using (var context = new MyContext15518(_options)) + { + var query = context.Repos.OrderBy(r => r.Id).Where(r => r.Id > 0); + query = tracking ? query.AsTracking() : query.AsNoTracking(); + + await foreach (var a in query.AsAsyncEnumerable()) + { + await foreach (var b in query.AsAsyncEnumerable()) + { + } + } + } + } + } + + private SqlServerTestStore CreateDatabase15518() + => CreateTestStore( + () => new MyContext15518(_options), + context => + { + context.AddRange( + new Repo15518 { Name = "London" }, + new Repo15518 { Name = "New York" }); + + context.SaveChanges(); + + ClearLog(); + }); + + public class MyContext15518 : DbContext + { + public DbSet Repos { get; set; } + + public MyContext15518(DbContextOptions options) : base(options) + { + } + } + + public class Repo15518 + { + public int Id { get; set; } + public string Name { get; set; } + } + + #endregion + private DbContextOptions _options; private SqlServerTestStore CreateTestStore( diff --git a/test/EFCore.Sqlite.FunctionalTests/ConcurrencyDetectorSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/ConcurrencyDetectorSqliteTest.cs index bbc96435024..c974edbb597 100644 --- a/test/EFCore.Sqlite.FunctionalTests/ConcurrencyDetectorSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/ConcurrencyDetectorSqliteTest.cs @@ -6,8 +6,7 @@ namespace Microsoft.EntityFrameworkCore { - // TODO: See Issue#14534 - internal class ConcurrencyDetectorSqliteTest : ConcurrencyDetectorRelationalTestBase> + public class ConcurrencyDetectorSqliteTest : ConcurrencyDetectorRelationalTestBase> { public ConcurrencyDetectorSqliteTest(NorthwindQuerySqliteFixture fixture) : base(fixture)