IDisposable Instance is not disposed unless it is activated/resolved

[jeremyhewett]

The issue is clearly illustrated by this failing XUnit test:

public class TestAutofacDisposal
{
    [Fact]
    public void ShouldDisposeIDisposableInstance()
    {
        // Arrange
        ContainerBuilder builder = new ContainerBuilder();
        IContainer container = builder.Build();
        ShouldDispose dep = new DisposeMe();

        // Act
        using (var scope = container.BeginLifetimeScope())
        {
            scope.Update(b => b.RegisterInstance(dep));
            //scope.Resolve<ShouldDispose>(); //UNCOMMENTING THIS LINE CAUSES THE TEST TO PASS
        }

        // Assert
        Assert.Equal(dep.IsDisposed, true);
    }
}

public interface ShouldDispose : IDisposable
{
    bool IsDisposed { get; }
}

public class DisposeMe : ShouldDispose
{
    public bool IsDisposed { get; private set; }
    public void Dispose()
    {
        IsDisposed = true;
    }
}

Autofac version is 3.5.2.

[tillig]

I was talking to @alexmg about this and I think you found a bug... but in the inverse of what you're mentioning.

Which is to say: Autofac should be responsible for disposing things that it creates; but if you create the instance, you should also be responsible for disposing it... otherwise it could well get "disposed out from under you."

We'll have to look at that.

Minor note, somewhat unrelated: In your test, you should register the instance when you begin the lifetime scope rather than using Update. Try to avoid Update wherever possible.

using (var scope = container.BeginLifetimeScope(c => c.RegisterInstance(dep)))
{
  // scope.Resolve<ShouldDispose>();
}

[jeremyhewett]

@tillig Thanks for the info and sorry for the very slow reply on my part! I would be ok with having to dispose instances that I create, as long as it's consistent whether they are referenced or not.

But an important note regarding the use of Update, some frameworks (at least Nancy for sure) rely on this method in order to integrate with Autofac for DI. The framework creates the lifetime scope and then passes it to the application code to be updated with request-specific dependencies (see usage). I feel if the Update method is provided by Autofac then it should work as expected, if it doesn't then it shouldn't be there at all.

[alexmg]

There is an interesting comment related to this in #383 which discusses the disposal of provided instances.

But my understanding is I'm handing ownership over to the container at registration time, unless I specify 'ExternallyOwned()'. Surely the instance shouldn't have to be resolved to be deterministically disposed when the container is disposed?

I can understand that with an explicit opt-out option in ExternallyOwned being available why someone would assume that by default instances will be tracked for disposal even when externally provided.

We actually have tests around disposing provided instances.

This one ensures that a provided instance is disposed along with the container even when not resolved.

[Fact]
public void UnresolvedProvidedInstances_DisposedWithLifetimeScope()
{
    var builder = new ContainerBuilder();
    var disposable = new DisposeTracker();
    builder.RegisterInstance(disposable);
    var container = builder.Build();
    container.Dispose();
    Assert.True(disposable.IsDisposed);
}

And this one ensures it is only disposed once in the case it is resolved.

[Fact]
public void ResolvedProvidedInstances_OnlyDisposedOnce()
{
    // Issue 383: Disposing a container should only dispose a provided instance one time.
    var builder = new ContainerBuilder();
    var count = 0;
    var disposable = new DisposeTracker();
    disposable.Disposing += (sender, e) => count++;
    builder.RegisterInstance(disposable);
    var container = builder.Build();
    container.Resolve<DisposeTracker>();
    container.Dispose();
    Assert.Equal(1, count);
}

I added another test similar to the first but using an action provided to BeginLifetimeScope to register the instance.

[Fact]
public void UnresolvedProvidedInstances_DisposedWithNestedLifetimeScope()
{
    var builder = new ContainerBuilder();
    var disposable = new DisposeTracker();
    var container = builder.Build();
    var scope = container.BeginLifetimeScope(b => b.RegisterInstance(disposable));
    scope.Dispose();
    container.Dispose();
    Assert.True(disposable.IsDisposed);
}

This test does not pass even when both the lifetime scope and container are disposed.

The decision now is do we try to make the behaviour consistent or just make all provided instances externally owned regardless of whether or not that has been requested with ExternallyOwned. We already make the registration implicitly SingleInstance and throw an exception is the user tries to change the registration to anything else. The same could be done with ExternallyOwned.

[tillig]

I'm starting to think that if you create it, you're responsible for disposing of it.

We've seen that pattern in lifetime scopes, for example: If Autofac integration libraries create, say, a per-request scope, Autofac integration libraries are also responsible for cleaning it up; if you create a lifetime scope, you're responsible for cleaning it up. That's why we didn't resolve #397 by tracking nested lifetime scopes - you're responsible for cleaning up things you create.

Likewise, I can see from issues like autofac/Autofac.Extensions.DependencyInjection#15 that it can be confusing if you dispose a container and have it dispose instances out from under you. The container didn't create that thing, why is it disposing it?

I'd be up for making instance handling 100% externally owned across the board.

[bcmedeiros]

What do you think about a opt-in for having Autofac disposing instances I create? It may be hard to know how/when to dispose some objects I had to create, being able to ask Autofac to do it for me seems a good feature.

[alexmg]

This fix is included in 4.6.1 which has been pushed to NuGet.

[sake402]

Hi
I am using Autofac 4.8.1 for a project and I realized autofac is disposing an instance give to it when the container is disposed. Is this by design or I am doing something wrong.
The following Test failed

`
[TestClass]
public class TestAutofacDisposal
{
[TestMethod]
public void ShouldDisposeIDisposableInstance()
{
ContainerBuilder builder = new ContainerBuilder();
ShouldDispose dep = new DisposeMe();
builder.RegisterInstance(dep);
IContainer container = builder.Build();
using (var scope = container.BeginLifetimeScope())
{
}
container.Dispose();
Assert.AreEqual(dep.IsDisposed, false);
}
}

public interface ShouldDispose : IDisposable
{
    bool IsDisposed { get; }
}

public class DisposeMe : ShouldDispose
{
    public bool IsDisposed { get; private set; }
    public void Dispose()
    {
        IsDisposed = true;
    }
}

`

[tillig]

The net result of this issue: Once you give Autofac the instance, it will consider itself the owner of the instance lifetime and will dispose the instance when the owning lifetime scope is disposed unless you specifically register it as ExternallyOwned().

[sake402]

Ok. Thanks a bunch.

[tillig]

For future readers this is also explained in the documentation.