Memory Leak Due to ThreadLocalStorage / ConcurrentBag / LifetimeScope Behavior in Owin

[ivanpfeff]

Bug Description

I would love to give a concise description of exactly what this bug is, but I don't fully understand it myself so I will instead try to explain as much of what I know about the issue and hope someone can fill in the gaps.

A web application using the Owin integration has memory leaks if it is under consistent stress which does not allow the request processing threads to exit.

LifetimeScopes are created for each request and are disposed properly when the request ends, but the LifetimeScope references are maintained and as a result garbage collection never cleans up the LifetimeScopes.

Here's a sample reference tree for one such LifetimeScope:

Rough outline of what happens to cause this issue:

• Autofac.Owin creates a LifetimeScope in the Owin pipeline and registers the OwinContext as an IOwinContext in this LifetimeScope. This OwinContext has an Environment key which tracks the LifetimeScope (something like "autofac:OwinLifetimeScope:")
• The ComponentRegistration for this OwinContext is somehow tracked in a ConcurrentBag, which apparently uses ThreadLocal as some kind of backing store
• The ThreadLocals have a strong GC handle tracking them, so they do not get cleaned up by the GC unless the thread exits. This does not happen if the web application is consistently busy.
• Since the ThreadLocals don't get GC'ed, neither does the ComponentRegistration, the ComponentRegistrations references the OwinContext, the OwinContext references the LifetimeScope, and the LifetimeScope never gets garbage collected.

I believe the ConcurrentBag which tracks the ComponentRegistrations is coming from somewhere within the Autofac 6.0 pipeline but I am not sure where.

Steps to Reproduce

I have attached a sample application (see below), this is just a simple Web API .NET Framework application to which I added the Owin pipeline and Autofac Owin integration.

To reproduce, just spam the application (using F5 in a browser works for me) and then take a memory dump. Don't wait too long between spamming and taking the dump or the threads will exit and the GC will clean up the ThreadLocal stuff and eventually clear out the LifetimeScopes. Inside of the memory dump, search for all instances of LifetimeScope and see that there are many hanging around and are all rooted in ConcurrentBags / ThreadLocals.

If you consistently ping the sample site for an extended period of time, you will see the number of LifetimeScopes in the memory dump continue to grow.

Expected Behavior

Under high load, the LifetimeScopes which are created by the Autofac.Owin integration should be cleaned up and released for garbage collection when the request ends.

Dependency Versions

Autofac 6.0
Autofac.Owin 6.0

Additional Info

I was able to work around this problem by specifically removing the OwinContext.Environment key "autofac:OwinLifetimeScope:" which removes the reference to the LifetimeScope and allows it to be garbage collected. This smells like a hack to me.
This can be found in NonAutofacMiddleware.cs inside of the sample application.

Sample Application

MemoryLeakSample.zip

If you want to see the results in memory from running requests for an extended amount of time, I wrote a small console app to just repeatedly ping the site:

static void Main(string[] args)
{
    var address = "https://localhost:44387/";
    while (true)
    {
        Console.WriteLine($"Pinging {address} at {DateTime.Now}");
        var client = new HttpClient();
        var response = client.GetAsync(address).ConfigureAwait(false).GetAwaiter().GetResult();
        response.EnsureSuccessStatusCode();

        Thread.Sleep(300);
    }
}

[ivanpfeff]

After some further thought about this, I am not sure that this is an Autofac.Owin issue or an Autofac core issue, it seems that the problematic part is really the use of ConcurrentBag inside of the main Autofac library. Removing the Environment entry for the autofac:OwinLifetimeScope solves the issue of LifetimeScopes not being garbage collected, but it doesn't solve the issue of the ComponentRegistrations not being GCed -- they are much smaller memory-wise but are still leaking.

[tillig]

I haven't been able to totally go through all this, but I see a few interesting things here that may or may not be related.

1. Use of GlobalConfiguration. OWIN uses a Startup class like ASP.NET Core and does not use GlobalConfiguration at all. We have that documented - one of the most common errors we see is folks trying to use an OWIN pipeline with GlobalConfiguration.
2. A Startup class in addition to GlobalConfiguration. I do see in the App_Start there's a Startup class (YAY!) but I also see that in Global.asax you're using GlobalConfiguration. That's going to be messy at best.
3. MVC and Web API in one app with OWIN. This is totally possible to do, but it's hard because MVC doesn't actually run inside the OWIN pipeline. Never has, never will. There is some hackery to get it all to work together, so it'll work, but I don't see any of the MVC OWIN stuff in your app.
4. No package references to the WebAPI or MVC OWIN integrations. This is a big red flag for me. You can't just add Autofac.Owin and call it good. Both WebAPI and MVC also have additional integration requirements to properly work with OWIN. MVC integration is documented as is WebAPI integration

Here's what I'd recommend:

• Catch up on the OWIN docs: Read about the MVC integration with OWIN and the WebAPI integration with OWIN. I think there are things you've missed.
• Check out the Examples repo: We have a WebAPI OWIN self-host example which isn't literally exactly what you're doing but you can see the patterns - it's not that different than an OWIN web hosted app (e.g., notice no Global.asax).
• Update your app to include all the integrations: Based on the integration docs and patterns make sure you have things correctly running in OWIN, with all the Autofac integration libraries, with the proper setup not using GlobalConfiguration and so on. No double-startup/configuration the way it is now.

If, after you've fixed the app to follow the docs and the examples, you still see a memory leak, post a new repro app with all the updates so we can check it out.

[ivanpfeff]

Okay, looking into fixing those things now

[ivanpfeff]

Okay, I'm going to go over this again using the WebAPI OWIN self-host example you linked instead of changing the initial sample application I uploaded so that we're on the same page.

To make the sample serve multiple web requests, I just changed the Program.cs:

using (WebApp.Start<Startup>(baseAddress))
{
    Console.ReadLine();
}

Following that, I did the same thing I mentioned in my initial post -- spam some web requests (against http://localhost:9123/api/test) and then take a memory dump of the application. The request spam was ~10 requests/second and I let it run for roughly 30 minutes.

Here's what I find in the memory dump after letting that run:

There are 36k LifetimeScopes sitting in memory,

They all have GC roots from the ThreadLocal stuff

[tillig]

Hmmm. Knowing that GC only runs when there's memory pressure, I'd be curious if a GC.Collect() was run to force the issue if these'd still be around or not.

I'm not honestly personally in a position at the immediate moment to diagnose this. I'm on a Mac and given MVC/WebAPI/OWIN is all Windows it'll require setting up a dev environment for that. I don't know if @alsami or @alistairjevans might have ideas off the cuff. Autofac does use ConcurrentDictionary and ConcurrentBag to help cache things for perf purposes - pipelines for building components and such - but I don't think that should be retained after disposing the child scope.

[ivanpfeff]

Okay, I added a GC.Collect into one of the middlewares after Next.Invoke but was still seeing a build up over time, though less so.

[ivanpfeff]

This is with the GC.Collect call added:

[alsami]

By taking a quick look at the given screenshots of @ivanpfeff you can see that there is always ConcurrentDictionary<Service, ServiceRegistrationInfo> and ConcurrentBag<IComponentRegistration> high in the ranks of the memory-usage.

As far as I can tell the only place where we create those types is in DefaultRegisteredServicesTracker which does inherit from Disposable. Odd (to me) seems that while each registration is being disposed, the ConcurrentDictionary<Service, ServiceRegistrationInfo> is neither cleared. Though I don't believe that this will necessary be a problem since the garbage-collector at some point should clean that up as well. @alistairjevans do you have an idea?

[alistairjevans]

Each lifetime scope created from BeginLifetimeScope that has custom registrations (in OWIN each request has custom registrations, because it registers IOwinContext per-request), will have its own instance of DefaultRegisteredServicesTracker.

The DefaultRegisteredServicesTracker will be garbage-collected when the lifetime scope is garbage-collected.

In the OWIN integration, the 'owner' of a lifetime scope is the OwinContext for the request; we can see in @ivanpfeff's most recent screenshot that there are the same number of OwinContext objects as there are LifetimeScope objects.

This makes me think the problem is firmly OWIN integration related rather than core Autofac.

@ivanpfeff, can you check you have no code that is holding on to instances of OwinContext somehow?

I suppose it's possible that OWIN is tracking old contexts somewhere, meaning they don't get GC'ed, and neither does the lifetime scope for the request.

There is something we can do to help with this though. The relevant lines in this integration are probably here, where we attach our Autofac scope to the OWIN context. You'll note that we attach the lifetime scope to the OWIN Context, but once we've disposed of the scope, we don't detach it.

I expect the lifetime scopes would get GC'ed much more rapidly if we overwrote the OWIN Context Autofac Item with null. Should be pretty straightforward to prove; @ivanpfeff, did you want to try out such a change and see if it helps?

[ivanpfeff]

Yeah I was yanking the scope out of the OwinContext and that does help with the LifetimeScope garbage collection. That was, for me at least, the majority of the memory issue we were experiencing in our enterprise application, because our LifetimeScopes were quite large due to a large number of per-request dependencies.

I think it would make sense to remove the scope from the OwinContext when your middleware has finished.

As for whether or not I'm holding on to OwinContexts on accident -- I'm using the OWIN self-hosted project from autofac\Examples with only one minor tweak in the Program.cs to make it run, so I have not intentionally added anything to hold references on the OwinContext.

[alistairjevans]

It might simply be that OWIN generally holds onto the context longer than it should do.

Did you want to raise a PR against this integration that clears out that tracking reference to the scope after the middleware exits?

I'm not sure what we can do beyond that really, once the GC cleans up the lifetime scope, everything else should go with it.

[ivanpfeff]

Yeah I will prepare that PR over the weekend, thanks for your input everyone!