Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Memory leak in Owin.AppBuilderExtensions

Tags:

I use OWIN + Microsoft.AspNet.Identity.Owin (v.2.0.0.0) in Web application. I register UserManager/DbContext per web request, as widely recommended:

app.CreatePerOwinContext(ApplicationDbContext.Create);
app.CreatePerOwinContext<ApplicationUserManager>(ApplicationUserManager.Create);

but neither is ever disposed. I took a glance look into reflector and it seems like a bug in extension method:

public static IAppBuilder CreatePerOwinContext<T>(this IAppBuilder app, Func<IdentityFactoryOptions<T>, IOwinContext, T> createCallback) where T: class, IDisposable
{
    if (app == null)
    {
        throw new ArgumentNullException("app");
    }
    if (createCallback == null)
    {
        throw new ArgumentNullException("createCallback");
    }
    object[] args = new object[1];
    IdentityFactoryOptions<T> options = new IdentityFactoryOptions<T> {
        DataProtectionProvider = app.GetDataProtectionProvider()
    };
    IdentityFactoryProvider<T> provider = new IdentityFactoryProvider<T> {
        OnCreate = createCallback
    };
    options.Provider = provider;
    args[0] = options;
    app.Use(typeof(IdentityFactoryMiddleware<T, IdentityFactoryOptions<T>>), args);
    return app;
}

IdentityFactoryProvider has two callbacks - create and dispose, but dispose callback is not registered here. I also confirmed my suspicion with memory profiler.

I don't see Owin on codeplex/github (actually I thought it is open source), so I don't know where to ask my question: could anyone else confirm this is memory leak? I'm not that sure because google says nothing about it, I expect it should be discussed everywhere, if this is a bug.

like image 370
Dmitry Naumov Avatar asked Jun 24 '14 05:06

Dmitry Naumov


3 Answers

I also have his issue, nothing that is registered with CreatePerOwinContext gets disposed. I'm using v2.1.

Here's a temporary fix which is working well for me as a work around until this lib is fixed. You basically have to manually register each of the types that use register with CreatePerOwnContext in the following class, and then at the end of your Startup procedure you register this custom class:

public sealed class OwinContextDisposal : IDisposable
{
    private readonly List<IDisposable> _disposables = new List<IDisposable>(); 

    public OwinContextDisposal(IOwinContext owinContext)
    {
        if (HttpContext.Current == null) return;

        //TODO: Add all owin context disposable types here
        _disposables.Add(owinContext.Get<MyObject1>());
        _disposables.Add(owinContext.Get<MyObject2>());

        HttpContext.Current.DisposeOnPipelineCompleted(this);
    }

    public void Dispose()
    {
        foreach (var disposable in _disposables)
        {
            disposable.Dispose();
        }
    }
}

At the end up your Startup process register this class:

 app.CreatePerOwinContext<OwinContextDisposal>(
      (o, c) => new OwinContextDisposal(c));

Now everything will get disposed of at the end of the request pipeline properly.

like image 168
Shazwazza Avatar answered Oct 05 '22 01:10

Shazwazza


The memory leak in AppBuilderExtensions class has been already fixed in latest version of Microsoft.AspNet.Identity.Owin library (2.2.1).

I have checked the code by using Reflector and also by putting a breakpoint into Dispose() methods of objects created by AppBuilderExtensions.CreatePerOwinContext().

like image 36
Jozef Benikovský Avatar answered Oct 05 '22 03:10

Jozef Benikovský


You can put the logic for disposing the instances you create with CreatePeOwinContext() in the same callback you use to create this intances. That is:

public class Startup
{
    public void Configuration(IAppBuilder app)
    {
        app.CreatePerOwinContext<ClassIWantOneInstancePerContext>(ClassIWantOneInstancePerContext.Create);

        //other code...
    }
}

Then you only should care about to include a call to DisposeOnPipelineCompleted() within the callback used to instantiate your class. That is:

public class ClassIWantOneInstancePerContext
{
     //other code...

    public static ClassIWantOneInstancePerContext Create()
    {
        ClassIWantOneInstancePerContext TheInstance = new ClassIWantOneInstancePerContext();
        HttpContext.Current.DisposeOnPipelineCompleted(TheInstance);

        return TheInstance;
    }
}

Also don't forget to include the Dispose() method on your class definition!

like image 36
Vi100 Avatar answered Oct 05 '22 02:10

Vi100