From 2df5ffdfc03d5038e5dfa61327da9dfbdacf732a Mon Sep 17 00:00:00 2001 From: Paulo Mattos <142408265+inoa-pmattos@users.noreply.github.com> Date: Mon, 20 Apr 2026 17:44:03 -0300 Subject: [PATCH] Fix singleton property cycle returning wrong instance from InstanceStash When resolving a dependency cycle involving singleton property injection, GetContextInstance could return an instance from a different component that happened to be stashed in the same CreationContext chain. This caused a type mismatch error (ComponentActivatorException) instead of properly detecting the cycle. The InstanceStash uses a single shared key per context. During a cycle like A -> (ctor) B -> (stash B, property) A, the stash contains B when A's handler is looked up cyclically, causing B to be returned as if it were A. The fix adds a type check in SingletonLifestyleManager.GetContextInstance: before returning the stashed instance, it verifies the instance is assignable to at least one of the component's service types. If not, null is returned and the normal cycle detection path runs (returning null for optional deps, or throwing CircularDependencyException for required deps). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Components/PropertyCycleComponents.cs | 51 ++++++++++ .../PropertyCycleTestCase.cs | 92 +++++++++++++++++++ .../Lifestyle/SingletonLifestyleManager.cs | 22 ++++- 3 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 src/Castle.Windsor.Tests/Components/PropertyCycleComponents.cs create mode 100644 src/Castle.Windsor.Tests/PropertyCycleTestCase.cs 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