Windsor adds a dependency resolved from typed factory as a burden on an unrelated object

[jnm2]

Windsor 4.1.0 adds a dependency resolved from typed factory as a burden on an unrelated object. When that unrelated object is released, the dependency is disposed even though it's still in use by the object that actually resolved it.

This had been causing some hair-pulling all afternoon; sorry to bring another of these upon you. 😃

There's repro code at the end. Here's the clearest description I can write, besides the code itself:

➡ When a short-lived object depends on both a typed factory and a long-lived object which itself depends on the same typed factory, and the short-lived object’s constructor triggers the long-lived object to resolve a dependency of its own through the typed factory, Castle Windsor erroneously adds that dependency as a child burden to be disposed when the short-lived object is released. Even though that dependency was resolved by the long-lived service and the short-lived object does not know about it!

(In the real scenario, neither the long-lived or short-lived objects directly depend on the typed factory but rather they both depend on the same intermediate dependency which depends on the typed factory. I was able to remove the intermediate dependency and reproduce the same behavior.)

The debugger shows that the SqlConnection which the LongLivedService resolves is added as a burden to ShortLivedViewModel (breakpoint in Burden.AddChild).

---

Self-contained repro:

Program.cs

using System;
using System.Data.SqlClient;
using Castle.Facilities.TypedFactory;
using Castle.MicroKernel.Registration;
using Castle.Windsor;

public static class Program
{
    public static void Main()
    {
        using (var container = new WindsorContainer())
        {
            container.AddFacility<TypedFactoryFacility>();
            container.Register(
                Component.For(typeof(IFactory<>)).AsFactory(),
                Component.For<SqlConnection>().UsingFactoryMethod(kernel => new SqlConnection()).LifeStyle.Transient,
                Component.For<LongLivedService>().LifeStyle.Singleton,
                Component.For<ShortLivedViewModel>().LifeStyle.Transient);

            var longLivedService = container.Resolve<LongLivedService>();
            var shortLivedViewModel = container.Resolve<ShortLivedViewModel>();

            var prematureDisposalHandler = new EventHandler((s, e) =>
                throw new Exception("Long-lived service’s connection was disposed when short-lived view model was released."));

            longLivedService.SqlConnection.Disposed += prematureDisposalHandler;
            container.Release(shortLivedViewModel);
            longLivedService.SqlConnection.Disposed -= prematureDisposalHandler;

            container.Release(longLivedService);
            Console.WriteLine("All good!"); // Is never reached
        }
    }
}

public interface IFactory<T>
{
    T Resolve();
    void Release(T instance);
}

public sealed class LongLivedService
{
    public IFactory<SqlConnection> ConnectionFactory { get; }

    public SqlConnection SqlConnection { get; private set; }

    public LongLivedService(IFactory<SqlConnection> connectionFactory)
    {
        ConnectionFactory = connectionFactory;
    }

    public void StartSomething()
    {
        SqlConnection = ConnectionFactory.Resolve();
    }

    public void Dispose()
    {
        ConnectionFactory.Release(SqlConnection);
    }
}

public sealed class ShortLivedViewModel
{
    public IFactory<SqlConnection> ConnectionFactory { get; }
    public LongLivedService LongLivedService { get; }

    public ShortLivedViewModel(IFactory<SqlConnection> connectionFactory, LongLivedService longLivedService)
    {
        ConnectionFactory = connectionFactory;
        LongLivedService = longLivedService;
        longLivedService.StartSomething();
    }
}

CastleWindsorRepro.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net472</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Castle.Windsor" Version="4.1.0" />
  </ItemGroup>

</Project>

[jnm2]

➡ It seems to me that while it might be okay to dispose SqlConnections handed out via shortLivedViewModel.ConnectionFactory.Resolve() when shortLivedViewModel is released, it is not okay to dispose SqlConnections handed out by longLivedService.ConnectionFactory.Resolve() when shortLivedViewModel is released. Would you agree with that?

Since shortLivedViewModel.ConnectionFactory and longLivedService.ConnectionFactory are the same object in the repro below, Castle Windsor can't tell which of the two is resolving the SqlConnection at any point in time. The only possible fixes are:

• Hand out two different connection factory instances to the ShortLivedViewModel constructor and the LongLivedService constructor so that it's possible to tell which one resolved the SqlConnection so that the burden can be added to LongLivedService instead of ShortLivedViewModel

• Don't add the SqlConnection as a burden to either LongLivedService or ShortLivedViewModel and only release it if it is passed to the typed factory's Release method

@jonorossi @fir3pho3nixx Which way should we go? I'm hoping we can confirm the diagnosis and agree on a path forward as quickly as possible. (I appreciate your help!) I'll help implement if need be.

[jnm2]

It looks like the SqlConnection should not be added as a burden to either LongLivedService or ShortLivedViewModel. It's a total accident that the SqlConnection is being resolved during the call to ShortLivedViewModel's constructor. An async timing difference could have ended up resolving the SqlConnection at some later point; when that happens, it is not added as a burden.

[jnm2]

I'm pretty confident that this is the right change to make, so I went ahead: #439