Default behavior with disposables makes it easy to create memory leaks

[alexmg]

From mike.ade...@gmail.com on January 04, 2014 08:56:11

What steps will reproduce the problem? 1. register some IDisposable service as InstancePerDependency()
2. resolve instances of that service from a static (root) container
This happened to us when using Autofac's WebApi integration. We had the following setup:

// when configuring autofac
containerBuilder.Register().InstancePerDependency();
containerBuilder.Register().SingleInstance();

// in the authorization attribute
// because we're in the attribute, this resolution happens in the root
// scope rather than the request scope
actionContext.ControllerContext.Configuration.DependencyResolver
.GetService(typeof(AuthHelper));

// in AuthHelper
public AuthHelper(Func factory) { ... }

public void Help() {
using (var service = this.factory()) { /* do stuff with service */ }
}

Basically, because the attribute resolves from the root, all of the MyDisposableService instances created by the AuthHelper would be tracked by the root scope's disposer forever, thus creating the leak.

When we finally diagnosed the problem, the fix was to have AuthHelper depend on a Func<Owned> instead. What is the expected output? What do you see instead? In general, Autofac's treatment of IDisposables is extremely helpful, however, I think some validation could be added to help people avoid this.

For example, a ContainerBuilderOption could be added called "StaticContainer" to make autofac aware that this scope will be long-lived. Thus, you'd have:

var container = builder.Build(ContainerBuilderOptions.StaticContainer);
// because of the option, this would throw an exception with the following
// message:
// "autofac does not automatically disposing IDisposable services when
// they are resolved from a static root container. Register the service as
// SingleInstance() or Resolve an Owned instance to manage
// disposal manually
container.Resolve();

I suppose another approach would be to have the Disposer hold weak references to the IDisposables (therefore assuming that they have destructors as necessary). Periodically (e. g. when adding a new disposable), dead references could be cleaned up What version of Autofac are you using? On what version of .NET/Silverlight? 3.0.2, .NET 4.5 Please provide any additional information below.

Original issue: http://code.google.com/p/autofac/issues/detail?id=481

[alexmg]

From travis.illig on January 03, 2014 16:06:17

The issue you describe is why we have documented on the best practices wiki page that you should always resolve dependencies from nested lifetime scopes: https://code.google.com/p/autofac/wiki/BestPractices#Always_Resolve_Dependencies_from_Nested_Lifetimes One way you can fix this for your components that must be IDisposable but resolved from the root container is to mark them ExternallyOwned. That's on the wiki here: https://code.google.com/p/autofac/wiki/DeterministicDisposal Alternatively, as you found, you can use the Owned relationship to automatically disable the tracking.

Using DI and properly configuring it is a somewhat advanced thing that is really hard to help people "stop shooting themselves in the foot." We can't anticipate whether you'll be resolving an IDisposable component from the root of the container. We don't know if you're building a temporary container and it's OK to allow you to resolve references to singleton IDisposables right out of the container. There are numerous other situations where you can also get into a sticky place if you're not careful. (Create a nested lifetime scope and forget to clean it up?)

Unfortunately what that all amounts to is that there's no great way to add "sanity checking" like this. We have to trust that the application developer is doing proper memory management, lifetime scope management, and so forth.

I'm glad you caught the issue in your app, but I don't think we'll be implementing safeguards around this.

Status: WontFix
Owner: travis.illig
Cc: alex.meyergleaves

[alexmg]

From mike.ade...@gmail.com on January 06, 2014 05:03:16

Did you consider the solution of using WeakReference in the implementation of the Disposer? It seems like would resolve the issue without any change to the API.

[alexmg]

From travis.illig on January 06, 2014 07:39:38

Yes, I did consider that. The concepts in this issue fairly strongly overlap with Issue #397 , "Nested lifetime scopes aren't disposed when the parent is disposed": https://code.google.com/p/autofac/issues/detail?id=397 Just switching to WeakReference doesn't fix it; it changes fundamentally the way you need to track activated instances and clean up after them. It adds a lot of complexity overall and from a cost/benefit standpoint isn't necessarily worth it. Check out the chain in issue #397 for some insight there.

We are looking at a lot of different ways to update the internals of Autofac for future releases - from performance to functionality - and may fix this as a byproduct of those improvements, but for now we're not looking to switch this up.