Avoid stack overflow when saving multiple modified entities with the same key

src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs

@@ -352,9 +352,28 @@ private void SetEntityState(EntityState oldState, EntityState newState, bool acc
 
             FireStateChanged(oldState);
 
-            if (newState == EntityState.Unchanged)
+            if (SharedIdentityEntry != null)
             {
-                SharedIdentityEntry?.SetEntityState(EntityState.Detached);
+                if (newState == EntityState.Unchanged)
+                {
+                    SharedIdentityEntry.SetEntityState(EntityState.Detached);
+                }
+                else if (newState == EntityState.Modified
+                    && SharedIdentityEntry.EntityState == EntityState.Modified)
+                {
+                    if (StateManager.SensitiveLoggingEnabled)
+                    {
+                        throw new InvalidOperationException(
+                            CoreStrings.IdentityConflictSensitive(
+                                EntityType.DisplayName(),
+                                this.BuildCurrentValuesString(EntityType.FindPrimaryKey()!.Properties)));
+                    }
+
+                    throw new InvalidOperationException(
+                        CoreStrings.IdentityConflict(
+                            EntityType.DisplayName(),
+                            EntityType.FindPrimaryKey()!.Properties.Format()));
+                }
             }
 
             if ((newState == EntityState.Deleted

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs

@@ -477,6 +477,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         b.Property<long>("EntityZId");
                         b.HasOne(e => e.EntityZ).WithMany().HasForeignKey("EntityZId").IsRequired();
                     });
+
+                modelBuilder.Entity<City>();
             }
 
             protected virtual object CreateFullGraph()
@@ -3409,7 +3411,43 @@ protected class EntityZ : NotifyingEntity
             public long Id
             {
                 get => _id;
-                set => _id = value;
+                set => SetWithNotify(value, ref _id);
+            }
+        }
+
+        protected class City : NotifyingEntity
+        {
+            private int _id;
+            private ICollection<College> _colleges = new ObservableHashSet<College>();
+
+            public int Id
+            {
+                get => _id;
+                set => SetWithNotify(value, ref _id);
+            }
+
+            public ICollection<College> Colleges
+            {
+                get => _colleges;
+                set => SetWithNotify(value, ref _colleges);
+            }
+        }
+
+        protected class College : NotifyingEntity
+        {
+            private int _id;
+            private int _cityId;
+
+            public int Id
+            {
+                get => _id;
+                set => SetWithNotify(value, ref _id);
+            }
+
+            public int CityId
+            {
+                get => _cityId;
+                set => SetWithNotify(value, ref _cityId);
             }
         }
 

test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs

@@ -7,7 +7,6 @@
 using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
 using Microsoft.EntityFrameworkCore.Diagnostics;
-using Microsoft.EntityFrameworkCore.Utilities;
 using Xunit;
 
 // ReSharper disable AccessToDisposedClosure
@@ -19,6 +18,60 @@ namespace Microsoft.EntityFrameworkCore
     public abstract partial class GraphUpdatesTestBase<TFixture>
         where TFixture : GraphUpdatesTestBase<TFixture>.GraphUpdatesFixtureBase, new()
     {
+        [ConditionalTheory] // Issue #23043
+        [InlineData(false)]
+        [InlineData(true)]
+        public virtual async Task Saving_multiple_modified_entities_with_the_same_key_does_not_overflow(bool async)
+        {
+            await ExecuteWithStrategyInTransactionAsync(
+                async context =>
+                {
+                    var city = new City
+                    {
+                        Colleges =
+                        {
+                            new()
+                        }
+                    };
+
+                    if (async)
+                    {
+                        await context.AddAsync(city);
+                        await context.SaveChangesAsync();
+                    }
+                    else
+                    {
+                        context.Add(city);
+                        context.SaveChanges();
+                    }
+                },
+                context =>
+                {
+                    var city = context.Set<City>().Include(x => x.Colleges).Single();
+                    var college = city.Colleges.Single();
+
+                    city.Colleges.Clear();
+                    city.Colleges.Add(new() { Id = college.Id });
+
+                    if (Fixture.ForceClientNoAction)
+                    {
+                        Assert.Equal(
+                            CoreStrings.RelationshipConceptualNullSensitive(nameof(City), nameof(College), $"{{CityId: {city.Id}}}"),
+                            Assert.Throws<InvalidOperationException>(
+                                () => context.Entry(college).State = EntityState.Modified).Message);
+                    }
+                    else
+                    {
+                        Assert.Equal(
+                            CoreStrings.IdentityConflictSensitive(nameof(College), $"{{Id: {college.Id}}}"),
+                            Assert.Throws<InvalidOperationException>(
+                                () => context.Entry(college).State = EntityState.Modified).Message);
+                    }
+
+                    return Task.CompletedTask;
+                });
+        }
+
         [ConditionalTheory] // Issue #22465
         [InlineData(false)]
         [InlineData(true)]

test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientCascadeTest.cs

@@ -0,0 +1,41 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Linq;
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
+
+namespace Microsoft.EntityFrameworkCore
+{
+    public class GraphUpdatesSqlServerClientCascadeTest : GraphUpdatesSqlServerTestBase<GraphUpdatesSqlServerClientCascadeTest.SqlServerFixture>
+    {
+        public GraphUpdatesSqlServerClientCascadeTest(SqlServerFixture fixture)
+            : base(fixture)
+        {
+        }
+
+        protected override void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
+            => facade.UseTransaction(transaction.GetDbTransaction());
+
+        public class SqlServerFixture : GraphUpdatesSqlServerFixtureBase
+        {
+            public override bool NoStoreCascades
+                => true;
+
+            protected override string StoreName { get; } = "GraphClientCascadeUpdatesTest";
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+            {
+                base.OnModelCreating(modelBuilder, context);
+
+                foreach (var foreignKey in modelBuilder.Model
+                    .GetEntityTypes()
+                    .SelectMany(e => e.GetDeclaredForeignKeys())
+                    .Where(e => e.DeleteBehavior == DeleteBehavior.Cascade))
+                {
+                    foreignKey.DeleteBehavior = DeleteBehavior.ClientCascade;
+                }
+            }
+        }
+    }
+}

test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientNoActionTest.cs

@@ -0,0 +1,40 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Linq;
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
+
+namespace Microsoft.EntityFrameworkCore
+{
+    public class GraphUpdatesSqlServerClientNoActionTest : GraphUpdatesSqlServerTestBase<GraphUpdatesSqlServerClientNoActionTest.SqlServerFixture>
+    {
+        public GraphUpdatesSqlServerClientNoActionTest(SqlServerFixture fixture)
+            : base(fixture)
+        {
+        }
+
+        protected override void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
+            => facade.UseTransaction(transaction.GetDbTransaction());
+
+        public class SqlServerFixture : GraphUpdatesSqlServerFixtureBase
+        {
+            public override bool ForceClientNoAction
+                => true;
+
+            protected override string StoreName { get; } = "GraphClientNoActionUpdatesTest";
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+            {
+                base.OnModelCreating(modelBuilder, context);
+
+                foreach (var foreignKey in modelBuilder.Model
+                    .GetEntityTypes()
+                    .SelectMany(e => e.GetDeclaredForeignKeys()))
+                {
+                    foreignKey.DeleteBehavior = DeleteBehavior.ClientNoAction;
+                }
+            }
+        }
+    }
+}

test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerHiLoTest.cs

@@ -0,0 +1,31 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
+
+namespace Microsoft.EntityFrameworkCore
+{
+    public class GraphUpdatesSqlServerHiLoTest : GraphUpdatesSqlServerTestBase<GraphUpdatesSqlServerHiLoTest.SqlServerFixture>
+    {
+        public GraphUpdatesSqlServerHiLoTest(SqlServerFixture fixture)
+            : base(fixture)
+        {
+        }
+
+        protected override void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
+            => facade.UseTransaction(transaction.GetDbTransaction());
+
+        public class SqlServerFixture : GraphUpdatesSqlServerFixtureBase
+        {
+            protected override string StoreName { get; } = "GraphHiLoUpdatesTest";
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+            {
+                modelBuilder.UseHiLo();
+
+                base.OnModelCreating(modelBuilder, context);
+            }
+        }
+    }
+}

test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerIdentityTest.cs

@@ -0,0 +1,31 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Storage;
+
+namespace Microsoft.EntityFrameworkCore
+{
+    public class GraphUpdatesSqlServerIdentityTest : GraphUpdatesSqlServerTestBase<GraphUpdatesSqlServerIdentityTest.SqlServerFixture>
+    {
+        public GraphUpdatesSqlServerIdentityTest(SqlServerFixture fixture)
+            : base(fixture)
+        {
+        }
+
+        protected override void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
+            => facade.UseTransaction(transaction.GetDbTransaction());
+
+        public class SqlServerFixture : GraphUpdatesSqlServerFixtureBase
+        {
+            protected override string StoreName { get; } = "GraphIdentityUpdatesTest";
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
+            {
+                modelBuilder.UseIdentityColumns();
+
+                base.OnModelCreating(modelBuilder, context);
+            }
+        }
+    }
+}