Skip to content

Commit

Permalink
Fix context pooling concurrency issue (#26226)
Browse files Browse the repository at this point in the history 
Fixes #26202
  • Loading branch information
@roji
roji authored Oct 4, 2021
1 parent c634711 commit 93ef75f
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 51 deletions.
68 changes: 36 additions & 32 deletions src/EFCore/DbContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -956,20 +956,51 @@ private IEnumerable<IResettableService> GetResettableServices()
/// </remarks>
public virtual void Dispose()
{
var leaseActive = _lease.IsActive;
var contextDisposed = leaseActive && _lease.ContextDisposed();
var lease = _lease;
var contextShouldBeDisposed = lease.IsActive && _lease.IsStandalone;

if (DisposeSync(leaseActive, contextDisposed))
if (DisposeSync(lease.IsActive, contextShouldBeDisposed))
{
_serviceScope?.Dispose();
}

lease.ContextDisposed();
}

/// <summary>
/// <para>
/// Releases the allocated resources for this context.
/// </para>
/// <para>
/// Entity Framework Core does not support multiple parallel operations being run on the same DbContext instance. This
/// includes both parallel execution of async queries and any explicit concurrent use from multiple threads.
/// Therefore, always await async calls immediately, or use separate DbContext instances for operations that execute
/// in parallel. See <see href="https://aka.ms/efcore-docs-threading">Avoiding DbContext threading issues</see>
/// for more information.
/// </para>
/// </summary>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-dbcontext">DbContext lifetime, configuration, and initialization</see>
/// for more information.
/// </remarks>
public virtual async ValueTask DisposeAsync()
{
var lease = _lease;
var contextShouldBeDisposed = lease.IsActive && _lease.IsStandalone;

if (DisposeSync(lease.IsActive, contextShouldBeDisposed))
{
await _serviceScope.DisposeAsyncIfAvailable();
}

await lease.ContextDisposedAsync();
}

private bool DisposeSync(bool leaseActive, bool contextDisposed)
private bool DisposeSync(bool leaseActive, bool contextShouldBeDisposed)
{
if (leaseActive)
{
if (contextDisposed)
if (contextShouldBeDisposed)
{
_disposed = true;
_lease = DbContextLease.InactiveLease;
Expand Down Expand Up @@ -1000,33 +1031,6 @@ private bool DisposeSync(bool leaseActive, bool contextDisposed)
return false;
}

/// <summary>
/// <para>
/// Releases the allocated resources for this context.
/// </para>
/// <para>
/// Entity Framework Core does not support multiple parallel operations being run on the same DbContext instance. This
/// includes both parallel execution of async queries and any explicit concurrent use from multiple threads.
/// Therefore, always await async calls immediately, or use separate DbContext instances for operations that execute
/// in parallel. See <see href="https://aka.ms/efcore-docs-threading">Avoiding DbContext threading issues</see>
/// for more information.
/// </para>
/// </summary>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-dbcontext">DbContext lifetime, configuration, and initialization</see>
/// for more information.
/// </remarks>
public virtual async ValueTask DisposeAsync()
{
var leaseActive = _lease.IsActive;
var contextDisposed = leaseActive && await _lease.ContextDisposedAsync();

if (DisposeSync(leaseActive, contextDisposed))
{
await _serviceScope.DisposeAsyncIfAvailable();
}
}

/// <summary>
/// Gets an <see cref="EntityEntry{TEntity}" /> for the given entity. The entry provides
/// access to change tracking information and operations for the entity.
Expand Down
32 changes: 13 additions & 19 deletions src/EFCore/Internal/DbContextLease.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ namespace Microsoft.EntityFrameworkCore.Internal
public struct DbContextLease
{
private IDbContextPool? _contextPool;
private readonly bool _standalone;

/// <summary>
/// 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.
/// </summary>
public bool IsStandalone { get; }

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -34,7 +41,7 @@ public struct DbContextLease
public DbContextLease(IDbContextPool contextPool, bool standalone)
{
_contextPool = contextPool;
_standalone = standalone;
IsStandalone = standalone;

var context = _contextPool.Rent();
Context = context;
Expand Down Expand Up @@ -63,16 +70,12 @@ public bool IsActive
/// 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.
/// </summary>
public bool ContextDisposed()
public void ContextDisposed()
{
if (_standalone)
if (IsStandalone)
{
Release();

return true;
}

return false;
}

/// <summary>
Expand All @@ -81,17 +84,8 @@ public bool ContextDisposed()
/// 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.
/// </summary>
public async ValueTask<bool> ContextDisposedAsync()
{
if (_standalone)
{
await ReleaseAsync();

return true;
}

return false;
}
public ValueTask ContextDisposedAsync()
=> IsStandalone ? ReleaseAsync() : default;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
26 changes: 26 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,32 @@ private async Task WriteResults()
}
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task Concurrency_test2(bool async)
{
var factory = BuildFactory<PooledContext>(withDependencyInjection: false);

await Task.WhenAll(
Enumerable.Range(0, 10).Select(_ => Task.Run(async () =>
{
for (var j = 0; j < 1_000_000; j++)
{
var ctx = factory.CreateDbContext();
if (async)
{
await ctx.DisposeAsync();
}
else
{
ctx.Dispose();
}
}
})));
}
private async Task Dispose(IDisposable disposable, bool async)
{
if (async)
Expand Down

0 comments on commit 93ef75f

Please sign in to comment.