Order of component decommission upon container disposal when using typed factories

[tigers0307]

First off, I am not sure if this qualifies as an "issue", but there is some questionable behavior here. I also believe that the underlying problem is related to the issues discussed in https://github.com/castleproject/Windsor/issues/134 and https://github.com/castleproject/Windsor/issues/100

My scenario is as follows:

• Use a singleton typed factory to create transient components at runtime (that require additional parameters known at runtime).
• While the factory-generated components are registered transient, they are effectively singleton (cached based on the runtime arguments by my code) and I do not Release them via the typed factory.
• Upon container disposal (at application shutdown), the transient components created by the factory get disposed before the factory (and before components that depend on the factory).
• This is suboptimal because this creates a temporary situation during shutdown where components that depend on the factory-built components are NOT disposed, but those dependencies have been disposed.
• Ideally (for my use case), the factory-built components would "wait" to get disposed until the factory itself is disposed.
• To add some color, the "real world" scenario here is that the factory is building TCP sockets, keyed by address. Annoying errors are generated on shutdown if disposed sockets are being accessed by components that have yet to be disposed.
• Note that I'm in the context of desktop and service applications, not web apps.
• I'm willing to create a custom lifestyle for the factory and/or the factory-built components to support this use case, but I haven't been able to figure out how to do so. In fact, I currently use a custom lifestyle to implement the "caching" behavior discussed above.

Here's a small unit test that encapsulates the problem:

[TestFixture]
public class TypedFactoryDecomissionOrder
{
    private readonly List<string> _destroyed = new List<string>();

    private IWindsorContainer GetContainer()
    {
        _destroyed.Clear();
        return new WindsorContainer()
            .AddFacility<TypedFactoryFacility>()
            .Register(
                Component.For<Wrapper>().OnDestroy(f => _destroyed.Add("wrapper")),
                Component.For<IFooFactory>().AsFactory().OnDestroy(f => _destroyed.Add("factory")),
                Component.For<IFoo>().ImplementedBy<Foo>().LifestyleTransient().OnDestroy(f => _destroyed.Add("foo")));
    }

    [Test]
    public void Test()
    {
        // in this test, we resolve the typed factory and call the creational method explicitly
        using (var container = GetContainer()) container.Resolve<IFooFactory>().Create();
        // we allow the container disposal to decomission all components
        // I would expect this order to be reversed, the factory should be disposed before
        // the items that it creates
        Assert.That(_destroyed, Is.EqualTo(new[] {"foo", "factory"}));
    }

    [Test]
    public void TestWithWrapper()
    {
        // in this test, we resolve out a singleton that calls the typed factory creational
        // method in its constructor
        using (var container = GetContainer()) container.Resolve<Wrapper>();
        // when using this approach, "foo" actually gets destroyed multiple times (which probably shouldn't be allowed)
        // ideally, the order of decomission here would be "wrapper", "factory", "foo"
        Assert.That(_destroyed, Is.EqualTo(new[] {"foo", "wrapper", "foo", "factory"}));
    }

    public class Wrapper
    {
        public Wrapper(IFooFactory factory)
        {
            factory.Create();
        }
    }

    public interface IFooFactory
    {
        IFoo Create();
    }

    public interface IFoo
    {
    }

    public class Foo : IFoo
    {
    }
}

[hammett]

Components are destroyed in the reverse order of construction. If we stick by this rule of thumb, the current behaviour is correct.

You can work around this for your scenario, by implementing a decommission protocol managed by your own factory, for that you should not use IDisposable interface, but your own interface instead.

[tigers0307]

Thanks for the quick response. I'm guessing the double-dispose issue i note in the second test is due to the issue being discussed in #100 -- because the typed-factory resolution is happening "within the context of" the resolution of the Wrapper class.

I would love a way to have the container not track the factory-resolved components (perhaps using the "externally managed" feature), but to retain the ability to have the factory track it (via the sub-release-policy mechanism). I experimented with this in a custom lifestyle implementation but couldn't make it work -- it appears you can either have "both" (tracked via factory AND via container) or "neither" (the component is never automatically disposed). I will continue to play around with this and see if I can come up with something.

[ghost]

@hammett - I think there are more dragons here.

Components are destroyed in the reverse order of construction. If we stick by this rule of thumb, the current behaviour is correct.

I agree.

You can work around this for your scenario, by implementing a decommission protocol managed by your own factory, for that you should not use IDisposable interface, but your own interface instead.

Do you think it is possible to fix the original problem?

https://github.com/castleproject/Windsor/pull/349 https://github.com/castleproject/Windsor/issues/134

Your help would be much appreciated.

[hammett]

Apologies but I dont even have .net installed anymore. I can help with high level discussion and reviewing PRs, but wont go much beyond that.

[ghost]

Thanks @hammett I understand completely. Most of this was @kkozmic's work. He is no longer with us so your input is valuable.

This is a completely "off the cuff" aside, do you think a rewrite here is in order? (in the absence of the original committer's mea culpa)

You can work around this for your scenario, by implementing a decommission protocol managed by your own factory, for that you should not use IDisposable interface, but your own interface instead.

To help @tigers0307 out could you elaborate on the high level abstractions you were thinking of? The problem is I simply don't know and would have to research the problem which would take a lot longer. I also appreciate you might not have the answer.

[kkozmic]

I'm happy to admit mea culpa on this. Not sure there's an easy solution to this with the current implementation and (from memory, I haven't looked at the code in a few years) there are some hacks there for certain scenarios that should be properly addressed.

Happy to answer further questions although I'm in a similar boat to @hammett in that I no longer even have .net installed on my box at the moment and it's been a while since I wrote C#

[ghost]

@kkozmic - thanks for the offer of discussion. I appreciate your input.

From memory when I spent time debugging this, I came to the conclusion that most of the work needed to happen in TypedFactoryInterceptor to make it aware of lifestyles. This solves one problem but introduces another which I will get to below.

Looking at it again now I can see that IOnBehalfAware provides the ComponentModel to the TypedFactoryInterceptor, which I thought was significant because it can give out information as to whether the instance is a singleton or not.

If a reference to this was held somehow then the Dispose method on the TypedFactoryInterceptor could be made smarter and choose not to dispose singeltons via the ReleasePolicy. This does fix the issue where singletons are unexpectedly disposed. Considering this, can you see any fatal flaws with this approach?

I did find that when doing this in my horrible hacky way in PR #349 that it introduced the problem of singletons resolved via the typed factory were not being disposed when the container got disposed. Not something I have an elegant answer for. Do you have an idea's as to how this could possibly be done?

[tigers0307]

Thanks for the engagement on this, @fir3pho3nixx @hammett and @kkozmic. I'm not very familiar with the internals of Windsor, but I've been using it extensively in somewhat advanced ways for a long time now (I've been lurking on the forum and have posted the occasional issue) -- so take my suggestions with a grain of salt.

That being said, in my experimentation with this so far, I've been thinking that the typed factory facility's utilization of the "sub release policy" to track components it creates could be the key to making this work (at least for my use case). The basic idea would be to have the factory-built components follow a custom lifestyle that is similar to singleton, but would allow tracking by the factory's sub-release-policy. Unfortunately, the default release policy (and therefore the sub release policy) doesn't allow tracking unless the Burden is NOT "tracked externally" (and in that mode, the container disposes the components out of order). I'm considering trying to find a way around that restriction. Do you think that's worth pursuing? I realize it won't fix the larger issue, but it would provide a hook to make the typed factory behave better under certain (and I expect somewhat common) use cases.

My basic goal is to make the components the factory builds the responsibility of the factory -- they shouldn't get tracked/disposed by the parent container (only indirectly when the parent container disposes the factory).

[hammett]

My basic goal is to make the components the factory builds the responsibility of the factory -- they shouldn't get tracked/disposed by the parent container (only indirectly when the parent container disposes the factory)

I believe a way to indicate that one wants this custom behavior is a resonable approach. Maybe mark something on the ComponentModel?

You'll have to figure out if there are implications down the resolution tree once you tell the container "from this point on, I'm tracking these components instances".

[ghost]

First of all nobody on this thread(@hammett + @kkozmic) has answered my questions. I also have evidence that you claims of not having dotnet installed are bullshit.

Please see this.

Stop messing around and help me out.

[hammett]

Gavin, let me as clear as one can be: this style of communication is not acceptable. If your intention is to nudge people there are better ways to do it. I suggest checking Rust’s code of conduct as a guideline.

Finally no one is forcing you to spend your time sorting out this “mess”. You’re doing of your own volition. In other words: no one owes you anything, and in no way your time investment gives you a free pass to be an obnoxious person.

I hope you reflect and adjust. Contributions are valuable but not at any cost.

On Jan 12, 2018, at 6:43 PM, Gavin van der Merwe notifications@github.com wrote:

I am a willing contributor who is prepared to spend time sorting through your "abandoned" mess. You guys need to buck up ... FFS

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[ghost]

@hammett - I also think a little guidance from you guys would help.

Gavin have you read this? Gavin have you looked at this code over here? Gavin this was the thinking here but it changed to this.

I am frustrated by no input at all.

[ghost]

@tigers0307 - this is an issue. I also think @jonorossi has proposed some answers. We just need to sync up on how we deal with this. @jonorossi will be out for a while so please be patient. I am collating all of this in the background to create a "god" typed factory issue. So we can address this in a single release. Not the first time we have seen nuances around this.

[ghost]

You are blending named vanilla(microkernel default) component registrations with typed factory component registrations.

This N+1 tracks late bound transient components that implement IDisposable when it comes to burdens && PR(https://github.com/castleproject/Windsor/pull/373).

If we start collating the rules we think are good(there might be existing rules we need to drop though), I can turn them into failing tests and start comitting them to a branch.

As we solve one issue, we might know whether it solved another. We also have coded evidence of what is going on here.

[mario-d-s]

@fir3pho3nixx just wanted to let you know I've read through the issue and everything related to it. I'm going to dive a bit more into the Windsor internals.

When you get round to "collating the rules" please ping me!

[ghost]

Good to have you on board, I am up to my eyeballs on aspnet core dependency injection shit. Should I create a new branch where we start collating failing tests from our issue tracker? I can think of 2 already and will add them in when I get time. Our spec for this should be failing tests. Please be aware, some of this stuff mental ;)

[mario-d-s]

If you want I can look into that. By "issue tracker" you just mean GH issues right? If so, I guess I know which failing tests you are talking about.

If you are thinking about rewriting the typed factory stuff, what is the scope? Are you planning on overhauling stuff in MicroKernel too?

I'm concerned how it will affect the public API, if at all. Do you just want to go chopping at the code or do you have ideas for a more structured approach?