diff --git a/src/Castle.Windsor.Tests/Components/PropertyCycleComponents.cs b/src/Castle.Windsor.Tests/Components/PropertyCycleComponents.cs new file mode 100644 index 0000000000..ac95c1b766 --- /dev/null +++ b/src/Castle.Windsor.Tests/Components/PropertyCycleComponents.cs @@ -0,0 +1,51 @@ +// Copyright 2004-2024 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace CastleTests.Components +{ + public interface IHasState + { + IState State { get; set; } + } + + public interface IState + { + } + + /// + /// A component that has a property dependency on . + /// When registered as singleton, resolving this component will trigger a + /// property injection cycle if the implementation + /// depends back on . + /// + public class PublisherWithState : IHasState + { + public IState State { get; set; } + } + + /// + /// A component that takes as a constructor + /// dependency, forming the other half of a cycle with + /// . + /// + public class StateComponent : IState + { + public StateComponent(IHasState publisher) + { + Publisher = publisher; + } + + public IHasState Publisher { get; } + } +} diff --git a/src/Castle.Windsor.Tests/PropertyCycleTestCase.cs b/src/Castle.Windsor.Tests/PropertyCycleTestCase.cs new file mode 100644 index 0000000000..b7cdb4285d --- /dev/null +++ b/src/Castle.Windsor.Tests/PropertyCycleTestCase.cs @@ -0,0 +1,92 @@ +// Copyright 2004-2024 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace CastleTests +{ + using Castle.MicroKernel; + using Castle.MicroKernel.Registration; + + using CastleTests.Components; + + using NUnit.Framework; + + [TestFixture] + public class PropertyCycleTestCase : AbstractContainerTestCase + { + [Test] + public void Singleton_property_cycle_should_not_return_wrong_component_from_instance_stash() + { + // This test reproduces a bug where TryGetHandlerFromKernel returns a cycling handler + // when no non-cycling alternative exists, instead of setting handler = null. + // When the property is required (non-optional), CanResolve is not called, so the + // code path goes directly through TryGetHandlerFromKernel → ResolveCore → + // GetContextInstance, which reads the wrong instance from the InstanceStash. + // + // The chain is: + // Resolve IState (StateComponent) + // → StateComponent ctor needs IHasState (PublisherWithState) + // → PublisherWithState is created and stashed in the CreationContext + // → PublisherWithState.State property (IState) is resolved + // → Handler for IState is StateComponent, which IS cycling + // → TryGetHandlerFromKernel finds no non-cycling alternative + // → BUG: returns true with the cycling handler + // → ResolveCore reads InstanceStash → gets PublisherWithState (wrong type!) + // → SetValue fails with type mismatch → ComponentActivatorException + Container.Register( + Component.For>().ImplementedBy>() + .LifestyleSingleton() + .PropertiesRequire(p => p.Name == "State"), + Component.For>().ImplementedBy>().LifestyleSingleton()); + + // Before the fix: ComponentActivatorException wrapping InvalidCastException + // ("Object of type 'PublisherWithState`1[String]' cannot be converted to type 'IState`1[String]'") + // After the fix: CircularDependencyException with a proper cycle message + Assert.Throws(() => Container.Resolve>()); + } + + [Test] + public void Singleton_optional_property_cycle_should_leave_property_null() + { + // When properties are optional (the default), CanResolve detects the cycle + // and skips the property (left null). + Container.Register( + Component.For>().ImplementedBy>().LifestyleSingleton(), + Component.For>().ImplementedBy>().LifestyleSingleton()); + + var state = (StateComponent)Container.Resolve>(); + + Assert.IsNotNull(state); + Assert.IsNotNull(state.Publisher); + Assert.IsNull(((PublisherWithState)state.Publisher).State, + "Optional cycling property should be left null."); + } + + [Test] + public void Singleton_property_cycle_with_concrete_types_should_leave_cycling_property_null() + { + // ACycleProp.Prop → BCycleProp, BCycleProp.Prop → ACycleProp (property-only cycle). + // Both properties are optional, so the cycling property should be left null. + Container.Register( + Component.For().LifestyleSingleton(), + Component.For().LifestyleSingleton()); + + var a = Container.Resolve(); + + Assert.IsNotNull(a); + Assert.IsNotNull(a.Prop, "Non-cycling direction should resolve normally."); + Assert.IsNull(a.Prop.Prop, + "Cycling property should be left null, not set to the wrong component instance."); + } + } +} diff --git a/src/Castle.Windsor/MicroKernel/Lifestyle/SingletonLifestyleManager.cs b/src/Castle.Windsor/MicroKernel/Lifestyle/SingletonLifestyleManager.cs index 5367ddee22..4e7451d75f 100644 --- a/src/Castle.Windsor/MicroKernel/Lifestyle/SingletonLifestyleManager.cs +++ b/src/Castle.Windsor/MicroKernel/Lifestyle/SingletonLifestyleManager.cs @@ -70,7 +70,27 @@ public override object Resolve(CreationContext context, IReleasePolicy releasePo public object GetContextInstance(CreationContext context) { - return context.GetContextualProperty(DefaultComponentActivator.InstanceStash); + var instance = context.GetContextualProperty(DefaultComponentActivator.InstanceStash); + if (instance == null) + { + return null; + } + + // The InstanceStash is a single shared key per context, so during a dependency + // cycle it may contain an instance from a different component that was being + // activated in the same context chain. Only return it if the instance actually + // matches this component's service types; otherwise the caller will follow the + // normal cycle detection path (returning null for optional dependencies or + // throwing CircularDependencyException for required ones). + foreach (var s in Model.Services) + { + if (s.IsInstanceOfType(instance)) + { + return instance; + } + } + + return null; } } } \ No newline at end of file