Reduce memory usage during relationship cycle detection by keeping track of visited properties whenever there are FK forks (#33211)

Fixes #33176

Author: AndriySvyryd

src/EFCore/Metadata/Internal/Property.cs

@@ -40,6 +40,9 @@ public class Property : PropertyBase, IMutableProperty, IConventionProperty, IPr
     public static readonly bool UseOldBehavior32422 =
         AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32422", out var enabled32422) && enabled32422;
 
+    private static readonly bool UseOldBehavior33176 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue33176", out var enabled33176) && enabled33176;
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -911,9 +914,10 @@ public virtual (ValueConverter? ValueConverter, Type? ValueConverterType, Type?
         bool throwOnValueConverterConflict = true,
         bool throwOnProviderClrTypeConflict = true)
     {
-        Queue<(Property CurrentProperty, Property CycleBreakingPropert, int CyclePosition, int MaxCycleLength)>? queue = null;
-        (Property CurrentProperty, Property CycleBreakingPropert, int CyclePosition, int MaxCycleLength)? currentNode =
+        Queue<(Property CurrentProperty, Property CycleBreakingProperty, int CyclePosition, int MaxCycleLength)>? queue = null;
+        (Property CurrentProperty, Property CycleBreakingProperty, int CyclePosition, int MaxCycleLength)? currentNode =
             (this, this, 0, 2);
+        HashSet<Property>? visitedProperties = null;
 
         ValueConverter? valueConverter = null;
         Type? valueConverterType = null;
@@ -922,12 +926,17 @@ public virtual (ValueConverter? ValueConverter, Type? ValueConverterType, Type?
         {
             var (property, cycleBreakingProperty, cyclePosition, maxCycleLength) = currentNode ?? queue!.Dequeue();
             currentNode = null;
-            if (cyclePosition >= ForeignKey.LongestFkChainAllowedLength)
+            if (cyclePosition >= ForeignKey.LongestFkChainAllowedLength
+                || (!UseOldBehavior33176
+                    && queue is not null
+                    && queue.Count >= ForeignKey.LongestFkChainAllowedLength))
             {
                 throw new InvalidOperationException(
                     CoreStrings.RelationshipCycle(DeclaringType.DisplayName(), Name, "ValueConverter"));
             }
 
+            visitedProperties?.Add(property);
+
             foreach (var foreignKey in property.GetContainingForeignKeys())
             {
                 for (var propertyIndex = 0; propertyIndex < foreignKey.Properties.Count; propertyIndex++)
@@ -956,6 +965,13 @@ public virtual (ValueConverter? ValueConverter, Type? ValueConverterType, Type?
                         {
                             queue = new();
                             queue.Enqueue(currentNode.Value);
+                            visitedProperties = new() { property };
+                        }
+
+                        if (!UseOldBehavior33176
+                            && visitedProperties?.Contains(principalProperty) == true)
+                        {
+                            break;
                         }
 
                         if (cyclePosition == maxCycleLength - 1)

test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs

@@ -11,6 +11,20 @@ namespace Microsoft.EntityFrameworkCore.Infrastructure;
 
 public class SqlServerModelValidatorTest : RelationalModelValidatorTest
 {
+    [ConditionalFact]
+    public virtual void Passes_on_TPT_with_nested_owned_types()
+    {
+        var modelBuilder = base.CreateConventionModelBuilder();
+
+        modelBuilder.Entity<BaseEntity>().UseTptMappingStrategy();
+        modelBuilder.Entity<ChildA>();
+        modelBuilder.Entity<ChildB>();
+        modelBuilder.Entity<ChildC>();
+        modelBuilder.Entity<ChildD>();
+
+        Validate(modelBuilder);
+    }
+
     public override void Detects_duplicate_columns_in_derived_types_with_different_types()
     {
         var modelBuilder = CreateConventionModelBuilder();

test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs

@@ -118,7 +118,43 @@ protected class Generic<T> : Abstract
     {
     }
 
-    public class SampleEntity
+#nullable enable
+    protected class BaseEntity
+    {
+        public int Id { get; set; }
+    }
+
+    protected class ChildA : BaseEntity
+    {
+        public OwnedType OwnedType { get; set; } = null!;
+    }
+
+    protected class ChildB : BaseEntity
+    {
+    }
+
+    protected class ChildC : BaseEntity
+    {
+    }
+
+    protected class ChildD : BaseEntity
+    {
+    }
+
+    [Owned]
+    protected class OwnedType
+    {
+        public NestedOwnedType NestedOwnedType { get; set; } = null!;
+    }
+
+    [Owned]
+    protected class NestedOwnedType
+    {
+    }
+
+#nullable restore
+
+    protected class SampleEntity
     {
         public int Id { get; set; }
         public int Number { get; set; }
@@ -131,29 +167,29 @@ public class SampleEntity
         public ICollection<SampleEntity> OtherSamples { get; set; }
     }
 
-    public class AnotherSampleEntity
+    protected class AnotherSampleEntity
     {
         public int Id { get; set; }
         public ReferencedEntity ReferencedEntity { get; set; }
     }
 
-    public class ReferencedEntity
+    protected class ReferencedEntity
     {
         public int Id { get; set; }
         public int SampleEntityId { get; set; }
     }
 
-    public class SampleEntityMinimal
+    protected class SampleEntityMinimal
     {
         public int Id { get; set; }
         public ReferencedEntityMinimal ReferencedEntity { get; set; }
     }
 
-    public class ReferencedEntityMinimal
+    protected class ReferencedEntityMinimal
     {
     }
 
-    public class AnotherSampleEntityMinimal
+    protected class AnotherSampleEntityMinimal
     {
         public int Id { get; set; }
         public ReferencedEntityMinimal ReferencedEntity { get; set; }
@@ -373,22 +409,22 @@ protected class DependentFour
         public PrincipalFour PrincipalFour { get; set; }
     }
 
-    public class Blog
+    protected class Blog
     {
         public int BlogId { get; set; }
         public bool IsDeleted { get; set; }
         public ICollection<PicturePost> PicturePosts { get; set; }
         public List<BlogOwnedEntity> BlogOwnedEntities { get; set; }
     }
 
-    public class BlogOwnedEntity
+    protected class BlogOwnedEntity
     {
         public int BlogOwnedEntityId { get; set; }
         public int BlogId { get; set; }
         public Blog Blog { get; set; }
     }
 
-    public class Post
+    protected class Post
     {
         public int PostId { get; set; }
         public int BlogId { get; set; }
@@ -397,13 +433,13 @@ public class Post
         public Blog Blog { get; set; }
     }
 
-    public class PicturePost : Post
+    protected class PicturePost : Post
     {
         public string PictureUrl { get; set; }
         public List<Picture> Pictures { get; set; }
     }
 
-    public class Picture
+    protected class Picture
     {
         public int PictureId { get; set; }
         public bool IsDeleted { get; set; }