[release/7.0] Prevent re-entrance into DetectChanges

src/EFCore/ChangeTracking/Internal/ChangeDetector.cs

@@ -15,8 +15,12 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 /// </summary>
 public class ChangeDetector : IChangeDetector
 {
+    private static readonly bool QuirkEnabled30135
+        = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30135", out var enabled) && enabled;
+
     private readonly IDiagnosticsLogger<DbLoggerCategory.ChangeTracking> _logger;
     private readonly ILoggingOptions _loggingOptions;
+    private bool _inCascadeDelete;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -112,37 +116,51 @@ public virtual void PropertyChanging(InternalEntityEntry entry, IPropertyBase pr
     /// </summary>
     public virtual void DetectChanges(IStateManager stateManager)
     {
-        OnDetectingAllChanges(stateManager);
-        var changesFound = false;
-
-        _logger.DetectChangesStarting(stateManager.Context);
+        if (_inCascadeDelete && !QuirkEnabled30135)
+        {
+            return;
+        }
 
-        foreach (var entry in stateManager.ToList()) // Might be too big, but usually _all_ entities are using Snapshot tracking
+        try
         {
-            switch (entry.EntityState)
-            {
-                case EntityState.Detached:
-                    break;
-                case EntityState.Deleted:
-                    if (entry.SharedIdentityEntry != null)
-                    {
-                        continue;
-                    }
+            _inCascadeDelete = true;
 
-                    goto default;
-                default:
-                    if (LocalDetectChanges(entry))
-                    {
-                        changesFound = true;
-                    }
+            OnDetectingAllChanges(stateManager);
+            var changesFound = false;
 
-                    break;
+            _logger.DetectChangesStarting(stateManager.Context);
+
+            foreach (var entry in stateManager.ToList()) // Might be too big, but usually _all_ entities are using Snapshot tracking
+            {
+                switch (entry.EntityState)
+                {
+                    case EntityState.Detached:
+                        break;
+                    case EntityState.Deleted:
+                        if (entry.SharedIdentityEntry != null)
+                        {
+                            continue;
+                        }
+
+                        goto default;
+                    default:
+                        if (LocalDetectChanges(entry))
+                        {
+                            changesFound = true;
+                        }
+
+                        break;
+                }
             }
-        }
 
-        _logger.DetectChangesCompleted(stateManager.Context);
+            _logger.DetectChangesCompleted(stateManager.Context);
 
-        OnDetectedAllChanges(stateManager, changesFound);
+            OnDetectedAllChanges(stateManager, changesFound);
+        }
+        finally
+        {
+            _inCascadeDelete = false;
+        }
     }
 
     /// <summary>
@@ -152,7 +170,22 @@ public virtual void DetectChanges(IStateManager stateManager)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual void DetectChanges(InternalEntityEntry entry)
-        => DetectChanges(entry, new HashSet<InternalEntityEntry> { entry });
+    {
+        if (_inCascadeDelete && !QuirkEnabled30135)
+        {
+            return;
+        }
+
+        try
+        {
+            _inCascadeDelete = true;
+            DetectChanges(entry, new HashSet<InternalEntityEntry> { entry });
+        }
+        finally
+        {
+            _inCascadeDelete = false;
+        }
+    }
 
     private bool DetectChanges(InternalEntityEntry entry, HashSet<InternalEntityEntry> visited)
     {

test/EFCore.InMemory.FunctionalTests/GraphUpdates/GraphUpdatesIdentityResolutionInMemoryTest.cs

@@ -0,0 +1,153 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore;
+
+public class GraphUpdatesIdentityResolutionInMemoryTest
+    : GraphUpdatesInMemoryTestBase<GraphUpdatesIdentityResolutionInMemoryTest.InMemoryIdentityResolutionFixture>
+{
+    public GraphUpdatesIdentityResolutionInMemoryTest(InMemoryIdentityResolutionFixture fixture)
+        : base(fixture)
+    {
+    }
+
+    [ConditionalFact]
+    public void Can_attach_full_required_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadRequiredGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryRequiredGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_optional_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadOptionalGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryOptionalGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_required_non_PK_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadRequiredNonPkGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryRequiredNonPkGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_required_AK_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadRequiredAkGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryRequiredAkGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_optional_AK_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadOptionalAkGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryOptionalAkGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_required_non_PK_AK_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadRequiredNonPkAkGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryRequiredNonPkAkGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_required_one_to_many_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadOptionalOneToManyGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryOptionalOneToManyGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    [ConditionalFact]
+    public void Can_attach_full_required_composite_graph_of_duplicates()
+        => ExecuteWithStrategyInTransaction(
+            context =>
+            {
+                var trackedRoot = LoadRequiredCompositeGraph(context);
+                var entries = context.ChangeTracker.Entries().ToList();
+
+                context.Attach(QueryRequiredCompositeGraph(context).AsNoTracking().Single(IsTheRoot));
+
+                AssertEntries(entries, context.ChangeTracker.Entries().ToList());
+                AssertNavigations(trackedRoot);
+
+                Assert.Equal(0, context.SaveChanges());
+            });
+
+    public class InMemoryIdentityResolutionFixture : GraphUpdatesInMemoryFixtureBase
+    {
+        protected override string StoreName
+            => "GraphUpdatesIdentityResolutionTest";
+
+        public override bool HasIdentityResolution
+            => true;
+
+        public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
+            => base.AddOptions(builder).AddInterceptors(new UpdatingIdentityResolutionInterceptor());
+    }
+}