Skip to content

Commit

Permalink
Fix caching of resettable services for pooled uninitialized context (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
roji authored Jun 13, 2021
1 parent 7105f84 commit 2c7670f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 10 deletions.
28 changes: 18 additions & 10 deletions src/EFCore/DbContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Diagnostics;
using System.Linq;
using System.Linq.Expressions;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.ChangeTracking;
Expand Down Expand Up @@ -788,7 +789,7 @@ void IDbContextPoolable.SnapshotConfiguration()
[EntityFrameworkInternal]
void IResettableService.ResetState()
{
foreach (var service in _cachedResettableServices ??= GetResettableServices())
foreach (var service in GetResettableServices())
{
service.ResetState();
}
Expand All @@ -807,7 +808,7 @@ void IResettableService.ResetState()
[EntityFrameworkInternal]
async Task IResettableService.ResetStateAsync(CancellationToken cancellationToken)
{
foreach (var service in _cachedResettableServices ??= GetResettableServices())
foreach (var service in GetResettableServices())
{
await service.ResetStateAsync(cancellationToken).ConfigureAwait(false);
}
Expand All @@ -817,22 +818,29 @@ async Task IResettableService.ResetStateAsync(CancellationToken cancellationToke
_disposed = true;
}

private List<IResettableService> GetResettableServices()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private IEnumerable<IResettableService> GetResettableServices()
{
var resettableServices = new List<IResettableService>();
if (_cachedResettableServices is not null)
{
return _cachedResettableServices;
}

var services
= _contextServices?.InternalServiceProvider?
.GetService<IEnumerable<IResettableService>>();
var resettableServices = new List<IResettableService>();

if (services != null)
var services = _contextServices?.InternalServiceProvider.GetService<IEnumerable<IResettableService>>();
if (services is not null)
{
resettableServices.AddRange(services);

// Note that if the context hasn't been initialized yet, we don't cache the resettable services
// (since some services haven't been added yet).
_cachedResettableServices = resettableServices;
}

if (_sets != null)
if (_sets is not null)
{
resettableServices.AddRange((_sets.Values.OfType<IResettableService>()));
resettableServices.AddRange(_sets.Values.OfType<IResettableService>());
}

return resettableServices;
Expand Down
37 changes: 37 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,17 @@ private interface ISecondContext

private class SecondContext : DbContext, ISecondContext
{
public DbSet<Blog> Blogs { get; set; }

public SecondContext(DbContextOptions options)
: base(options)
{
}

public class Blog
{
public int Id { get; set; }
}
}

[ConditionalFact]
Expand Down Expand Up @@ -717,6 +724,36 @@ public async Task Context_configuration_is_reset(bool useInterface, bool async)
Assert.Null(context1.Database.GetCommandTimeout());
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task Uninitialized_context_configuration_is_reset_properly(bool async)
{
var serviceProvider = BuildServiceProvider<SecondContext>();

DbContext ctx;
using (var scope = serviceProvider.CreateScope())
using (var ctx1 = scope.ServiceProvider.GetRequiredService<SecondContext>())
{
await ctx1.DisposeAsync();

This comment has been minimized.

Copy link
@Tommigun1980

Tommigun1980 Jun 15, 2021

Not await Dispose(ctx1, async);?

This comment has been minimized.

Copy link
@roji

roji Jun 15, 2021

Author Member

Thanks, you're right... Submitted #25102 to fix-up.

ctx = ctx1;
}

using (var scope = serviceProvider.CreateScope())
using (var ctx2 = scope.ServiceProvider.GetRequiredService<SecondContext>())
{
Assert.Same(ctx, ctx2);
ctx2.Blogs.Add(new SecondContext.Blog());
}

using (var scope = serviceProvider.CreateScope())
using (var ctx3 = scope.ServiceProvider.GetRequiredService<SecondContext>())
{
Assert.Same(ctx, ctx3);
Assert.Empty(ctx3.ChangeTracker.Entries());
}
}

[ConditionalTheory]
[InlineData(false, false)]
[InlineData(true, false)]
Expand Down

0 comments on commit 2c7670f

Please sign in to comment.