OnRelease not triggered if Provided Instance service has never been resolved

[arialdomartini]

@ale7canna and I noticed a weird behavior regarding the disposal of components registered with RegisterInstance() in combination with the use of OnRelease().

We know from the documentation about Provided Instances that

If you provide an instance registration to Autofac, Autofac will assume ownership of that instance and will handle its disposal.

This is confirmed by the next test, which shows how Autofac automatically disposes of the MyClass instance during the container disposal:

internal class MyClass : IDisposable
{
    public static Boolean Disposed = false;

    public void Dispose() => Disposed = true;
}

[Fact]
void autofac_takes_care_of_the_disposal_of_instances_registered_with_RegisterInstance()
{
    var builder = new ContainerBuilder();
    builder.RegisterInstance(new MyClass());

    using (var container = builder.Build())
    {
        using (var scope = container.BeginLifetimeScope())
        {
            var service = scope.Resolve<MyClass>();
        }
    }

    MyClass.Disposed.Should().Be(true);
}

This works even if MyClass is actually never resolved, that is if the line

var service = scope.Resolve<MyClass>();

is commented out.

So far so good.
We know from OnRelease that:

OnRelease event replaces the standard cleanup behavior for a component.

So, if we register MyClass instance providing the registration with an OnRelease callback, Autofac will stop invoking Dispose on the Container disposal.
In the next test, we use OnRelease to manually invoke Dispose when OnRelease is triggered.

[Fact]
void autofac_does_does_not_invoke_OnRelease_if_service_is_never_resolved()
{
    var builder = new ContainerBuilder();

    builder
        .RegisterInstance(new MyClass())
        .OnRelease(s => s.Dispose());

    using (var container = builder.Build())
    {
        using (var scope = container.BeginLifetimeScope())
        {
            var service = scope.Resolve<MyClass>();
        }
    }

    MyClass.Disposed.Should().Be(true);
}

Strangely, and differently from the previous case, if the service is never resolved, Dispose is in fact not called. The following test fails:

[Fact]
void autofac_does_does_not_invoke_OnRelease_if_service_is_never_resolved()
{
    var builder = new ContainerBuilder();

    builder
        .RegisterInstance(new MyClass())
        .OnRelease(s => s.Dispose());

    using (var container = builder.Build())
    {
        using (var scope = container.BeginLifetimeScope())
        {
//            var service = scope.Resolve<MyClass>();
        }
    }

    MyClass.Disposed.Should().Be(true);
}

This means that the clean up of MyClass instance is never performed:

• Dispose is not called, because the automatic disposal of the provided instance has been disabled by the use of OnRelease and replaced by the provided callback;
• but OnRelease is never triggered, because no instance of MyClass service has never been requested.

Consequently, the instance is never cleaned up, potentially leading to a resource leak.
We are not sure this is a bug, but it sounds a bit confusing. Both v4 and v5 have the same behavior.

[alistairjevans]

Hmmm; looking at the OnRelease code, the way it works is:

• Attaches to the ActivatingHandlers list for the registration
• When activated, adds an object to the current scope that will be disposed of when the scope exits.
• The attached object invokes the on-release callback.

Because the service is never activated, it is never released.

It feels that perhaps we should imply AutoActivate for provided instances (or something like that).

[alistairjevans]

Ok, so it looks like if we add a new AutoActivate service in OnRelease if any Activating or Activated handlers have been attached, then the OnRelease will fire correctly when the container is disposed.

I'll look at putting a PR together.

[alistairjevans]

Oh, and thanks @arialdomartini for the detailed issue report.

[tillig]

There's a bit of history here. #383 has a similar discussion and there's been discussion (though I can't find it now... maybe the Google Group?) where there's a sort of question around how Autofac should deal with provided instances - Autofac should clearly dispose of things it creates; but if you create the thing then you own it. Should Autofac dispose out from under you?

From the docs it appears we settled on requiring ExternallyOwned to opt out of disposal. [I had a whole pros/cons of that and am trying to dredge up the memory of what we decided.]

I'm not sure I'd AutoActivate it if there's a better way; I'd love to... someday... not have AutoActivate or startable crap in the container if we could avoid it. Doing something behind the scenes that ensures it just gets added to the disposal list the same way SingleInstance does would be nicer, though off the top of my head I don't know how that is. Maybe not possible.

[alistairjevans]

I see what you mean, thanks for the reference; I suppose in a way, having an OnRelease for a provided instance is just a replacement for releasing manually when you dispose of the container...

But if you don't have access to the code at the point the container is released (e.g. in ASP.NET core) or the type does not implement dispose, and you cannot change it, what is the suggested behaviour?

Register a decorator for the service that provides a Dispose method and put OnRelease behaviour in there?

[tillig]

If the type doesn't implement Dispose then I'm not super concerned over it. If you have a type that needs cleanup for safe resource handling the pattern is to implement IDisposable. I can't say I'm terribly interested in recommending workarounds for app design that doesn't follow practices from .NET 1.0. (Not .NET Core 1.0 but actual .NET 1.0)

The only way the container gets disposed is by having access to that point in the code. Even in ASP.NET Core you can subscribe to a shutdown event to handle disposing things.

So your options would be...

• Write a decorator to handle disposal.
• Submit a pull request to get the library you're consuming updated to be standard.
• Handle it yourself at shutdown.
  • try/finally in your Main method.
  • IApplicationLifetime.ApplicationStopped in ASP.NET Core
  • etc.

That said, it's somewhat moot. It seems like the decision made way back when, for consistency, was to treat provided instances as something we need to dispose as part of the container so it's consistent with the rest of things in the container. Apparently that's not happening, though I'm not sure why it changed; possibly something to do with the immutable container updates?

[alistairjevans]

The crux of this specific issue is that:

• We do invoke Dispose on Provided Instances when the container is disposed, even if they have not been Resolved.
• We do not invoke Dispose on a service when an OnRelease callback has been registered (we assume you handle it yourself).
• We do not invoke OnRelease on Provided Instances when the container is disposed, if they have not been resolved.

This last point definitely feels like it introduces a contradiction, where one form of clean-up works in both cases (Dispose), and another form of clean-up (OnRelease), which we document on the same page as Dispose as being a equatable alternative, only works in one of them.

Apparently that's not happening, though I'm not sure why it changed; possibly something to do with the immutable container updates?

This is not a new problem, it appears to have been around for some time based on the code history. Just a side effect of using the Activating handlers.

[tillig]

Yeah, sounds like we need a fix.