DbContext pooling recycles a context whose ConcurrencyDetector is stuck in the critical section; every later rent fails with "A second operation was started on this context instance"
- Dominant language
- C#
- Stars
- 14.8k
- Forks
- 3.4k
- PR merge metrics
- PR metrics pending
Description
### Bug description
`ConcurrencyDetector` is not reset when a pooled `DbContext` is returned to the pool, and `EnterCriticalSection`/`ExitCriticalSection` toggle `_inCriticalSection` around an allocating `AsyncLocal` write. If that write throws (in practice: `OutOfMemoryException` under memory pressure), the flag stays set, the context is returned to the pool as usual, and every subsequent operation on that instance — for the rest of the process lifetime — throws `InvalidOperationException: A second operation was started on this context instance...`.
This turns one transient failure in one request into a permanent failure of the process. In our case a single low-traffic pod (the pool held one or two instances, so the poisoned one was rented almost every time) failed **every** database call for 45 minutes until it was redeployed: the original OOM hit one endpoint 24 times, the poisoned context produced ~1,000 errors on unrelated endpoints.
This is the mechanism behind #22802, which was closed with "OutOfMemoryException is not something that can be recovered from". Agreed that the request that hit OOM is lost; the ask here is different: **the pool should not hand a known-bad instance to the next request**, and the detector should not have a window where a failed allocation leaves it permanently locked. Both are cheap to fix and need no OOM handling.
`Interlocked.CompareExchange(ref _inCriticalSection, 1, 0)` has already run when `ThreadAcquiredLocksCount.Value++` throws, so `EnterCriticalSection` never returns a disposer and nothing ever clears the flag. `ExitCriticalSection` has the mirror-image window: `ThreadAcquiredLocksCount.Value--` (also an allocating write) runs before `_inCriticalSection = 0`.
### Your code
The pooling half is deterministic and reproduces without OOM, using the standard `AddDbContextPool` + scope-per-request setup:
```csharp
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.Extensions.DependencyInjection;
var services = new ServiceCollection();
services.AddDbContextPool(o => o.UseSqlite("Data Source=repro.db"));
await using var provider = services.BuildServiceProvider();
using (var scope = provider.CreateScope())
{
var ctx = scope.ServiceProvider.GetRequiredService();
ctx.Database.EnsureDeleted();
ctx.Database.EnsureCreated();
}
var first = await Request1();
await Request2(first);
// Separate async method: AsyncLocal changes made here do not flow back to the caller,
// so Request2 runs on a fresh flow like a real next request would.
async Task Request1()
{
using var scope = provider.CreateScope();
var ctx = scope.ServiceProvider.GetRequiredService();
await ctx.Blogs.ToListAsync();
// Simulates EnterCriticalSection succeeding but the matching Exit never running
// (what happens when the AsyncLocal write inside Enter/Exit throws).
ctx.GetService().EnterCriticalSection(); // disposer dropped
return ctx;
} // scope disposed -> context returned to pool; ResetState() does not touch ConcurrencyDetector
async Task Request2(BlogContext first)
{
using var scope = provider.CreateScope();
var ctx = scope.ServiceProvider.GetRequiredService();
Console.WriteLine($"same instance: {ReferenceEquals(first, ctx)}"); // True
await ctx.Blogs.ToListAsync(); // InvalidOperationException: A second operation was started on this context instance...
}
public class BlogContext(DbContextOptions options) : DbContext(options)
{
public DbSet Blogs => Set();
}
public class Blog { public int Id { get; set; } }
```
Note the two requests must run on different async flows (separate async methods, or `ExecutionContext.SuppressFlow()`); on the same flow the leaked `ThreadAcquiredLocksCount` makes the detector treat the second call as re-entrant and the bug is masked. That is probably why this was hard to reproduce in #22802.
### Stack traces
The other half (how the disposer gets dropped in production) is the stack trace from #22802 and from our incident:
```text
System.OutOfMemoryException
at System.Threading.AsyncLocalValueMap.MultiElementAsyncLocalValueMap.Set(IAsyncLocal key, Object value, Boolean treatNullValueAsNonexistent)
at System.Threading.ExecutionContext.SetLocalValue(IAsyncLocal local, Object newValue, Boolean needChangeNotifications)
at System.Threading.AsyncLocal`1.set_Value(T value)
at Microsoft.EntityFrameworkCore.Infrastructure.Internal.ConcurrencyDetector.EnterCriticalSection()
at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
```
### Verbose output
_No response_
### EF Core version
10.0.9 (also verified against current `main`; `ConcurrencyDetector` is unchanged since 8.x)
### Database provider
Microsoft.EntityFrameworkCore.SqlServer (repro above uses Sqlite)
### Target framework
.NET 10
### Operating system
Linux (Kubernetes); #22802 reported the same on Windows
### IDE
_No response_
### Proposed fix
Two small, allocation-free changes; no behaviour change on the happy path:
1. `ConcurrencyDetector` implements `IResettableService` and `StateManager.ResetState()` resets it (same way it already resets `_changeDetector`), so a pooled context always starts its next lease clean.
2. Reorder the bookkeeping so the flag is only kept if the AsyncLocal write succeeds, and cleared before the AsyncLocal write on exit:
```csharp
public virtual ConcurrencyDetectorCriticalSectionDisposer EnterCriticalSection()
{
if (Interlocked.CompareExchange(ref _inCriticalSection, 1, 0) == 1
&& ThreadAcquiredLocksCount.Value == 0)
{
throw new InvalidOperationException(CoreStrings.ConcurrentMethodInvocation);
}
try
{
ThreadAcquiredLocksCount.Value++;
}
catch
{
if (_currentContextRefCount == 0)
{
_inCriticalSection = 0;
}
throw;
}
_currentContextRefCount++;
return new ConcurrencyDetectorCriticalSectionDisposer(this);
}
public virtual void ExitCriticalSection()
{
if (--_currentContextRefCount == 0)
{
_inCriticalSection = 0;
}
ThreadAcquiredLocksCount.Value--;
}
```
I have this implemented with tests (pooled context reusable after a leaked section — red before, green after; `EFCore.Tests` fully green) and can open a PR if the team is open to it.
Contributor guide
Research direction
Start with ConcurrencyDetector and StateManager.ResetState, then inspect the AddDbContextPool scope-per-request repro described in the issue. Run the mentioned pooled-context test and EFCore.Tests; done means a context remains reusable after a leaked critical section and the full test suite stays green.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100