Skip to content

Commit

Permalink
Clear events when DbContext is disposed or returned to the pool
Browse files Browse the repository at this point in the history
Fixes #23108
  • Loading branch information
ajcvickers committed Nov 11, 2020
1 parent 53d6d02 commit 4e6c2fb
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
1 change: 1 addition & 0 deletions All.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ Licensed under the Apache License, Version 2.0. See License.txt in the project r
<s:Boolean x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=FileEF4F00E20178B341995BD2EFE53739B5/@KeyIndexDefined">True</s:Boolean>
<s:Double x:Key="/Default/Environment/InjectedLayers/InjectedLayerCustomization/=FileEF4F00E20178B341995BD2EFE53739B5/RelativePriority/@EntryValue">2</s:Double>
<s:String x:Key="/Default/Environment/PerformanceGuide/SwitchBehaviour/=VsBulb/@EntryIndexedValue">DO_NOTHING</s:String>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EFeature_002EServices_002EDaemon_002ESettings_002EMigration_002ESwaWarningsModeSettingsMigrate/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpAttributeForSingleLineMethodUpgrade/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpKeepExistingMigration/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpPlaceAttributeOnSameLineMigration/@EntryIndexedValue">True</s:Boolean>
Expand Down
20 changes: 16 additions & 4 deletions src/EFCore/DbContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -756,9 +756,7 @@ void IResettableService.ResetState()
service.ResetState();
}

SavingChanges = null;
SavedChanges = null;
SaveChangesFailed = null;
ClearEvents();

_disposed = true;
}
Expand All @@ -769,7 +767,6 @@ void IResettableService.ResetState()
/// 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>
/// <param name="cancellationToken"> A <see cref="CancellationToken" /> to observe while waiting for the task to complete. </param>
[EntityFrameworkInternal]
async Task IResettableService.ResetStateAsync(CancellationToken cancellationToken)
{
Expand All @@ -778,6 +775,8 @@ async Task IResettableService.ResetStateAsync(CancellationToken cancellationToke
await service.ResetStateAsync(cancellationToken).ConfigureAwait(false);
}

ClearEvents();

_disposed = true;
}

Expand Down Expand Up @@ -825,6 +824,9 @@ private bool DisposeSync()
if (_lease.ContextDisposed())
{
_disposed = true;

ClearEvents();

_lease = DbContextLease.InactiveLease;
}
}
Expand All @@ -842,6 +844,8 @@ private bool DisposeSync()
_changeTracker = null;
_database = null;

ClearEvents();

return true;
}

Expand All @@ -854,6 +858,14 @@ private bool DisposeSync()
public virtual ValueTask DisposeAsync()
=> DisposeSync() ? _serviceScope.DisposeAsyncIfAvailable() : default;


private void ClearEvents()
{
SavingChanges = null;
SavedChanges = null;
SaveChangesFailed = null;
}

/// <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
38 changes: 38 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/DbContextPoolingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.ChangeTracking;
Expand Down Expand Up @@ -496,9 +497,20 @@ public async Task Context_configuration_is_reset(bool useInterface, bool async)
context1.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context1.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;
context1.Database.AutoTransactionsEnabled = true;
context1.SavingChanges += (sender, args) => { };
context1.SavedChanges += (sender, args) => { };
context1.SaveChangesFailed += (sender, args) => { };

Assert.NotNull(GetContextEventField(context1, nameof(DbContext.SavingChanges)));
Assert.NotNull(GetContextEventField(context1, nameof(DbContext.SavedChanges)));
Assert.NotNull(GetContextEventField(context1, nameof(DbContext.SaveChangesFailed)));

await Dispose(serviceScope, async);

Assert.Null(GetContextEventField(context1, nameof(DbContext.SavingChanges)));
Assert.Null(GetContextEventField(context1, nameof(DbContext.SavedChanges)));
Assert.Null(GetContextEventField(context1, nameof(DbContext.SaveChangesFailed)));

serviceScope = serviceProvider.CreateScope();
scopedProvider = serviceScope.ServiceProvider;

Expand Down Expand Up @@ -531,9 +543,20 @@ public async Task Context_configuration_is_reset_with_factory(bool async)
context1.ChangeTracker.CascadeDeleteTiming = CascadeTiming.Immediate;
context1.ChangeTracker.DeleteOrphansTiming = CascadeTiming.Immediate;
context1.Database.AutoTransactionsEnabled = true;
context1.SavingChanges += (sender, args) => { };
context1.SavedChanges += (sender, args) => { };
context1.SaveChangesFailed += (sender, args) => { };

Assert.NotNull(GetContextEventField(context1, nameof(DbContext.SavingChanges)));
Assert.NotNull(GetContextEventField(context1, nameof(DbContext.SavedChanges)));
Assert.NotNull(GetContextEventField(context1, nameof(DbContext.SaveChangesFailed)));

await Dispose(context1, async);

Assert.Null(GetContextEventField(context1, nameof(DbContext.SavingChanges)));
Assert.Null(GetContextEventField(context1, nameof(DbContext.SavedChanges)));
Assert.Null(GetContextEventField(context1, nameof(DbContext.SaveChangesFailed)));

var context2 = factory.CreateDbContext();

Assert.Same(context1, context2);
Expand All @@ -553,6 +576,10 @@ public void Change_tracker_can_be_cleared_without_resetting_context_config()
new DbContextOptionsBuilder().UseSqlServer(
SqlServerNorthwindTestStoreFactory.NorthwindConnectionString).Options);

Assert.Null(GetContextEventField(context, nameof(DbContext.SavingChanges)));
Assert.Null(GetContextEventField(context, nameof(DbContext.SavedChanges)));
Assert.Null(GetContextEventField(context, nameof(DbContext.SaveChangesFailed)));

context.ChangeTracker.AutoDetectChangesEnabled = true;
context.ChangeTracker.LazyLoadingEnabled = true;
context.ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.NoTracking;
Expand All @@ -561,6 +588,9 @@ public void Change_tracker_can_be_cleared_without_resetting_context_config()
context.Database.AutoTransactionsEnabled = true;
context.ChangeTracker.Tracked += ChangeTracker_OnTracked;
context.ChangeTracker.StateChanged += ChangeTracker_OnStateChanged;
context.SavingChanges += (sender, args) => { };
context.SavedChanges += (sender, args) => { };
context.SaveChangesFailed += (sender, args) => { };

context.ChangeTracker.Clear();

Expand All @@ -579,8 +609,16 @@ public void Change_tracker_can_be_cleared_without_resetting_context_config()

Assert.True(_changeTracker_OnTracked);
Assert.True(_changeTracker_OnStateChanged);

Assert.NotNull(GetContextEventField(context, nameof(DbContext.SavingChanges)));
Assert.NotNull(GetContextEventField(context, nameof(DbContext.SavedChanges)));
Assert.NotNull(GetContextEventField(context, nameof(DbContext.SaveChangesFailed)));
}

private object GetContextEventField(DbContext context, string eventName)
=> typeof(DbContext)
.GetField(eventName, BindingFlags.GetField | BindingFlags.NonPublic | BindingFlags.Instance)
.GetValue(context);
private bool _changeTracker_OnTracked;

private void ChangeTracker_OnTracked(object sender, EntityTrackedEventArgs e)
Expand Down

0 comments on commit 4e6c2fb

Please sign in to comment.