Optimizing the circular dependency detection

VContainer/Assets/Tests/ContainerTest.cs

@@ -570,6 +570,55 @@ public void CircularDependencyMsg()
             }
         }
 
+        [Test]
+        public void SelfCircularDependency()
+        {
+            var builder = new ContainerBuilder();
+            builder.Register<HasSelfCircularDependency>(Lifetime.Transient);
+
+            var injector = InjectorCache.GetOrBuild(typeof(HasSelfCircularDependency));
+
+            if (injector is ReflectionInjector)
+            {
+                Assert.Throws<VContainerException>(() => builder.Build());
+            }
+        }
+
+        [Test]
+        public void DiamondDependencyIsNotCircular()
+        {
+            var builder = new ContainerBuilder();
+            builder.Register<DiamondRoot>(Lifetime.Transient);
+            builder.Register<DiamondLeft>(Lifetime.Transient);
+            builder.Register<DiamondRight>(Lifetime.Transient);
+            builder.Register<DiamondLeaf>(Lifetime.Transient);
+
+            Assert.DoesNotThrow(() => builder.Build());
+        }
+
+        [Test]
+        public void DeepAcyclicChainIsNotCircular()
+        {
+            var builder = new ContainerBuilder();
+            builder.Register<AcyclicChainA>(Lifetime.Transient);
+            builder.Register<AcyclicChainB>(Lifetime.Transient);
+            builder.Register<AcyclicChainC>(Lifetime.Transient);
+            builder.Register<AcyclicChainD>(Lifetime.Transient);
+
+            Assert.DoesNotThrow(() => builder.Build());
+        }
+
+        [Test]
+        public void SharedDependencyAcrossRootsIsNotCircular()
+        {
+            var builder = new ContainerBuilder();
+            builder.Register<DiamondLeft>(Lifetime.Transient);
+            builder.Register<DiamondRight>(Lifetime.Transient);
+            builder.Register<DiamondLeaf>(Lifetime.Transient);
+
+            Assert.DoesNotThrow(() => builder.Build());
+        }
+
         [Test]
         public void Inject()
         {

VContainer/Assets/Tests/Fixtures.cs

@@ -294,6 +294,91 @@ class HasCircularDependencyMsg4
         [Inject] public HasCircularDependencyMsg1 Prop { get; set; }
     }
 
+    class HasSelfCircularDependency
+    {
+        public HasSelfCircularDependency(HasSelfCircularDependency self)
+        {
+            if (self == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class DiamondLeaf
+    {
+    }
+
+    class DiamondLeft
+    {
+        public DiamondLeft(DiamondLeaf leaf)
+        {
+            if (leaf == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class DiamondRight
+    {
+        public DiamondRight(DiamondLeaf leaf)
+        {
+            if (leaf == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class DiamondRoot
+    {
+        public DiamondRoot(DiamondLeft left, DiamondRight right)
+        {
+            if (left == null || right == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class AcyclicChainA
+    {
+        public AcyclicChainA(AcyclicChainB b)
+        {
+            if (b == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class AcyclicChainB
+    {
+        public AcyclicChainB(AcyclicChainC c)
+        {
+            if (c == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class AcyclicChainC
+    {
+        public AcyclicChainC(AcyclicChainD d)
+        {
+            if (d == null)
+            {
+                throw new ArgumentException();
+            }
+        }
+    }
+
+    class AcyclicChainD
+    {
+    }
+
     class HasMethodInjection : I1
     {
         public I2 Service2;

VContainer/Assets/VContainer/Runtime/Internal/TypeAnalyzer.cs

@@ -213,6 +213,12 @@ static class TypeAnalyzer
         [ThreadStatic]
         static Stack<DependencyInfo> circularDependencyChecker;
 
+        [ThreadStatic]
+        static HashSet<Type> visitingTypes;
+
+        [ThreadStatic]
+        static HashSet<Type> verifiedTypes;
+
         static readonly Func<Type, InjectTypeInfo> AnalyzeFunc = Analyze;
 
         public static InjectTypeInfo Analyze(Type type)
@@ -368,8 +374,12 @@ private static bool Contains(List<InjectFieldInfo> fields, FieldInfo field)
         public static void CheckCircularDependency(IReadOnlyList<Registration> registrations, Registry registry)
         {
             // ThreadStatic
-            if (circularDependencyChecker == null)
-                circularDependencyChecker = new Stack<DependencyInfo>();
+            circularDependencyChecker ??= new Stack<DependencyInfo>();
+            visitingTypes ??= new HashSet<Type>();
+            verifiedTypes ??= new HashSet<Type>();
+
+            visitingTypes.Clear();
+            verifiedTypes.Clear();
 
             for (var i = 0; i < registrations.Count; i++)
             {
@@ -380,27 +390,33 @@ public static void CheckCircularDependency(IReadOnlyList<Registration> registrat
 
         static void CheckCircularDependencyRecursive(DependencyInfo current, Registry registry, Stack<DependencyInfo> stack)
         {
-            var i = 0;
-            foreach (var dependency in stack)
+            var currentType = current.ImplementationType;
+
+            if (verifiedTypes.Contains(currentType))
+                return;
+
+            if (!visitingTypes.Add(currentType))
             {
-                if (current.ImplementationType == dependency.ImplementationType)
-                {
-                    // When instantiated by Func, the abstract type cycle is user-avoidable.
-                    if (current.Dependency.Provider is FuncInstanceProvider)
-                    {
-                        return;
-                    }
+                // When instantiated by Func, the abstract type cycle is user-avoidable.
+                if (current.Dependency.Provider is FuncInstanceProvider)
+                    return;
 
-                    stack.Push(current);
+                stack.Push(current);
 
-                    var path = string.Join("\n",
-                        stack.Take(i + 1)
-                            .Reverse()
-                            .Select((item, itemIndex) => $"    [{itemIndex + 1}] {item} --> {item.ImplementationType.FullName}"));
-                    throw new VContainerException(current.Dependency.ImplementationType,
-                        $"Circular dependency detected!\n{path}");
+                var i = 0;
+                foreach (var dependency in stack)
+                {
+                    if (i > 0 && current.ImplementationType == dependency.ImplementationType)
+                        break;
+                    i++;
                 }
-                i++;
+
+                var path = string.Join("\n",
+                    stack.Take(i)
+                        .Reverse()
+                        .Select((item, itemIndex) => $"    [{itemIndex + 1}] {item} --> {item.ImplementationType.FullName}"));
+                throw new VContainerException(current.Dependency.ImplementationType,
+                    $"Circular dependency detected!\n{path}");
             }
 
             stack.Push(current);
@@ -464,6 +480,8 @@ static void CheckCircularDependencyRecursive(DependencyInfo current, Registry re
             }
 
             stack.Pop();
+            visitingTypes.Remove(currentType);
+            verifiedTypes.Add(currentType);
         }
     }
 }