IDisposable and UsingFactoryMethod

[aaron-hammond]

Consider:

public class ServiceInstaller : IWindsorInstaller
    {
        public void Install(IWindsorContainer container, IConfigurationStore store)
        {
            container.Register(
                Component.For<ISomeConnectionService>()
                    .ImplementedBy<SomeConnectionService>()
                    .LifestyleTransient()
                    .IsDefault());
            
            container.Register(
                Component.For<FailoverDatabaseConnectionExecutor>()
                .ImplementedBy<FailoverDatabaseConnectionExecutor>()
                .LifestyleTransient());

            container.Register(Component.For<IDatabaseConnectionExecutor>()
                .UsingFactoryMethod(CreateDatabaseConnectionExecutor)
                .LifestyleTransient()
                .IsDefault());
        }

        private static IDatabaseConnectionExecutor CreateDatabaseConnectionExecutor(IKernel kernel)
        {
            return kernel.Resolve<FailoverDatabaseConnectionExecutor>();
        }
    }
    
    public interface IDatabaseConnectionExecutor
    {
    }

    public class SomeConnectionService : ISomeConnectionService, System.IDisposable
    {
        public SomeConnectionService()
        {

        }

        public void Dispose()
        {
        }
    }

    public interface ISomeConnectionService
    {
    }

    public class FailoverDatabaseConnectionExecutor : IDatabaseConnectionExecutor
    {
        private readonly ISomeConnectionService _someConnectionService;

        public FailoverDatabaseConnectionExecutor(ISomeConnectionService someConnectionService)
        {
            _someConnectionService = someConnectionService;
        }
    }

When attempting to inject ISomeConnectionService i receive the following error:

Instance FailoverDatabaseConnectionExecutor of component Late bound IDatabaseConnectionExecutor is already being tracked. The factory method providing instances of the component is reusing instances, but the lifestyle of the component is Transient which requires new instance each time. In most cases it is advised for the factory method not to be handling reuse of the instances, but to chose a lifestyle that does that appropriately. Alternatively, if you do not wish for Windsor to track the objects coming from the factory change your regustration to '.UsingFactoryMethod(yourFactory, managedExternally: true)'

If i remove IDisposable from SomeConnectionService then i don't receive the error any more. Is this by design? Is it not possible to have an object implementing IDisposable in the dependency chain in conjunction with .UsingFactoryMethod? As a side note there is also a spelling mistake "regustration" in the message.

[jonorossi]

I can't think of why this is occurring, I suspect because a dependency has a decommission concern (i.e. IDisposable) during the resolve it gets tracked, and then a second time for the factory. I assume setting managedExternally: true also resolves the problem, disabling the tracking the second time.

Can you debug Windsor to determine why it is attempting to track the component a second time (and throwing). We can go from there to determine a way forward.

P.S. fixed that spelling mistake in 61807e4, thanks.

[ghost]

@aaron-hammond - Please see 710e85c

This test is passing for me

I am going to close this issue out as I think it might be a misconfigured container. Please feel free to continue the discussion here and if we do turn up anything we can always open the issue again.

[ghost]

Sorry just double checked and I was resolving the wrong thing.

[ghost]

I changed the test case this and it is still passing:

[Test]
public void Can_resolve_service_using_factory_that_implements_disposable()
{
	var a = Container.Resolve<ISomeConnectionService>();
	Assert.That(a, Is.Not.Null);

	var b = Container.Resolve<FailoverDatabaseConnectionExecutor>();
	Assert.That(b, Is.Not.Null);
}

[ghost]

@aaron-hammond - If you post a full repro on Github using the latest version of Castle, I might be able to help you spot where you have gone wrong ...

[aaron-hammond]

@Fir3pho3nixx Sure i can do that, i think the issue isn't in doing Container.Resolve<FailoverDatabaseConnectionExecutor>(); It's when you inject and attempt to resolve the the factory. var c = container.Resolve<IDatabaseConnectionExecutor>(); should give you the error.

[aaron-hammond]

The actual error I've tracked down to Castle.MicroKernel.Releasers.LifecycledComponentsReleasePolicy initially the FailoverDatabaseConnectionExecutor is tracked and added to instance2Burden.

public virtual void Track(object instance, Burden burden)
		{
			if (burden.RequiresPolicyRelease == false)
			{
				var lifestyle = ((object)burden.Model.CustomLifestyle) ?? burden.Model.LifestyleType;
				throw new ArgumentException(
					string.Format(
						"Release policy was asked to track object '{0}', but its burden has 'RequiresPolicyRelease' set to false. If object is to be tracked the flag must be true. This is likely a bug in the lifetime manager '{1}'.",
						instance, lifestyle));
			}
			try
			{
				using (@lock.ForWriting())
				{
					instance2Burden.Add(instance, burden);
				}
			}
			catch (ArgumentNullException)
			{
				//eventually we should probably throw something more useful here too
				throw;
			}
			catch (ArgumentException)
			{
				throw HelpfulExceptionsUtil.TrackInstanceCalledMultipleTimes(instance, burden);
			}
			burden.Released += OnInstanceReleased;
			perfCounter.IncrementTrackedInstancesCount();
		}

When an attempt is then made to register the factory; the track method in Castle.MicroKernel.Lifestyle.AbstractLifestyleManager is hit and the value of RequiresPolicyRelease is set to true.

   protected virtual void Track(Burden burden, IReleasePolicy releasePolicy)
		{
			if (burden.RequiresPolicyRelease)
			{
				releasePolicy.Track(burden.Instance, burden);
			}
		}

I think this is due to the fact that it has child components that are implementing IDisposable. When the track method in the LifecycledComponentsReleasePolicy is called again the key for FailoverDatabaseConnectionExecutor has already been used in the dictionary by the initial registration and it generates a duplicate key error.

@jonorossi asked me to figure out why it was being added and tracked a second time but i ran out of time last night and haven't had a chance to update.

[ghost]

After checking this out I have managed to replicate the issue. Apologies for closing prematurely.

[aaron-hammond]

@Fir3pho3nixx no problem mate.

The issue, i think, boils down to the dictionary in Castle.MicroKernel.Releasers.LifecycledComponentsReleasePolicy :

private readonly Dictionary<object, Burden> instance2Burden =
			new Dictionary<object, Burden>(ReferenceEqualityComparer<object>.Instance);

having a key set to the resolved type FailoverDatabaseConnectionExecutor from the registration:

            container.Register(
                Component.For<FailoverDatabaseConnectionExecutor>()
                    .ImplementedBy<FailoverDatabaseConnectionExecutor>()
                    .LifestyleTransient());

and then also setting the key to FailoverDatabaseConnectionExecutor when it attempts to track the factory registered by:

           container.Register(
                Component.For<IDatabaseConnectionExecutor>()
                .UsingFactoryMethod(kernel => kernel.Resolve<IMyFactory>().Create())
                .LifestyleTransient()
                .IsDefault());

They both get to that tracked point by having child dependencies that rely on IDisposable (i think). The reasons for that i'm unsure of as this is the first time i've looked under the hood so to speak and as i say i ran out of time looking at it..

[ghost]

@aaron-hammond - you are right.

From what I can see, the TransientLifestyleManager is asking the LifecycledComponentsReleasePolicy to track the FailoverDatabaseConnectionExecutor twice. I think this is happen because of the following resolution cycle:

• Resolve<ISomeConnectionService> calls LifecycledComponentsReleasePolicy.Track<SomeConnectionService>()

• Resolve<FailoverDatabaseConnectionExecutor> calls LifecycledComponentsReleasePolicy.Track<FailoverDatabaseConnectionExecutor>()

• Resolve<IDatabaseConnectionExecutor> calls LifecycledComponentsReleasePolicy.Track<FailoverDatabaseConnectionExecutor>() <- This fails because at the very end because it cannot add the same instance for FailoverDatabaseConnectionExecutor twice in the burden tracking for LifecycledComponentsReleasePolicy.

This is a tricky one. I will keep looking.

[aaron-hammond]

@Fir3pho3nixx - agreed, i'm seeing the same. Becomes a bit of a rabbit hole for me after that but if i get a bit more time i'll have a dig at this too.

[ghost]

@aaron-hammond sorry I have simply not had time to look at this, any progress from your side?

[aaron-hammond]

@Fir3pho3nixx i spent i bit more time on this but then gave up. We just used an IHandler selector instead as a work around. See: https://stackoverflow.com/questions/46075494/castle-windsor-usingfactorymethod-with-lifestyletransient/46276146#46276146

[ghost]

I tried a bit more debugging tonight. I believe the code calling typed factory component activators are not honouring the IsDefault behaviour(or upstream handlers at least). I could also see the DefaultNamingSubSystem::GetServiceSelector is what pointed you in the direction of the handler selector work around. Problem with most of this, it is implemented using ExtendedProperties here for downstream resolve state.

@ALL - If anyone who has information on how this flows through to typed factory activators that would be handy!

[ghost]

Everything I said before was wrong.

I even added the horrible factory facility in my original example. I am so sorry. I am only starting to learn windsor internals(without help from the original authors). I know more now. Feel free to tell me off.

Turns out transient burdens for disposables are being N+1 tracked between factory registrations and non factory registrations for transients when it comes to burdens.

My smoking gun was this

This code is not quite complete because I needed to invalidate some tests.

Let me know what you think.

[jnm2]

@Fir3pho3nixx Don't feel bad. We've all been there! ❤️