diff --git a/VContainer/Assets/Tests/ContainerTest.cs b/VContainer/Assets/Tests/ContainerTest.cs index 375167de..f94b6efc 100644 --- a/VContainer/Assets/Tests/ContainerTest.cs +++ b/VContainer/Assets/Tests/ContainerTest.cs @@ -570,6 +570,55 @@ public void CircularDependencyMsg() } } + [Test] + public void SelfCircularDependency() + { + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + + var injector = InjectorCache.GetOrBuild(typeof(HasSelfCircularDependency)); + + if (injector is ReflectionInjector) + { + Assert.Throws(() => builder.Build()); + } + } + + [Test] + public void DiamondDependencyIsNotCircular() + { + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + + Assert.DoesNotThrow(() => builder.Build()); + } + + [Test] + public void DeepAcyclicChainIsNotCircular() + { + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + + Assert.DoesNotThrow(() => builder.Build()); + } + + [Test] + public void SharedDependencyAcrossRootsIsNotCircular() + { + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + builder.Register(Lifetime.Transient); + + Assert.DoesNotThrow(() => builder.Build()); + } + [Test] public void Inject() { diff --git a/VContainer/Assets/Tests/Fixtures.cs b/VContainer/Assets/Tests/Fixtures.cs index 1366c011..bfee3506 100644 --- a/VContainer/Assets/Tests/Fixtures.cs +++ b/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; diff --git a/VContainer/Assets/VContainer/Runtime/Internal/TypeAnalyzer.cs b/VContainer/Assets/VContainer/Runtime/Internal/TypeAnalyzer.cs index c4eee2e0..4185faab 100644 --- a/VContainer/Assets/VContainer/Runtime/Internal/TypeAnalyzer.cs +++ b/VContainer/Assets/VContainer/Runtime/Internal/TypeAnalyzer.cs @@ -213,6 +213,12 @@ static class TypeAnalyzer [ThreadStatic] static Stack circularDependencyChecker; + [ThreadStatic] + static HashSet visitingTypes; + + [ThreadStatic] + static HashSet verifiedTypes; + static readonly Func AnalyzeFunc = Analyze; public static InjectTypeInfo Analyze(Type type) @@ -368,8 +374,12 @@ private static bool Contains(List fields, FieldInfo field) public static void CheckCircularDependency(IReadOnlyList registrations, Registry registry) { // ThreadStatic - if (circularDependencyChecker == null) - circularDependencyChecker = new Stack(); + circularDependencyChecker ??= new Stack(); + visitingTypes ??= new HashSet(); + verifiedTypes ??= new HashSet(); + + visitingTypes.Clear(); + verifiedTypes.Clear(); for (var i = 0; i < registrations.Count; i++) { @@ -380,27 +390,33 @@ public static void CheckCircularDependency(IReadOnlyList registrat static void CheckCircularDependencyRecursive(DependencyInfo current, Registry registry, Stack 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); } } }