8

We had a major outage today in production where memory was disappearing very quickly from our webservers. This was traced back to a caching mechanism in Ninject (I think it was Activation Cache or something - not totally sure). After investigating the issue we came to the conclusion that we had a circular reference in our scope callback.

class View
{
    Presenter presenter;

    View()
    {
        //service locators are smelly, but webforms forces this uglyness
        this.presenter = ServiceLocator.Get<Func<View, Presenter>>()(this);

        this.presenter.InitUI();
    }
}

class Presenter
{
    CookieContainer cookieContainer;
    View view;

    Presenter(View view, CookieContainer cookieContainer)
    {
        this.view = view;
        this.cookieContainer = cookieContainer;
    }
}

class CookieContainer
{
    HttpRequest request;
    HttpResponse response; 

    CookieContainer()
    {
        this.request = HttpRequest.Current.Request;
        this.response = HttpRequest.Current.Response;
    }
}

Bind<Func<View, Presenter>>().ToMethod(ctx => view => 
        ctx.Kernel.Get<Presenter>(new ConstructorArgument("view", view)));

Bind<Presenter>().ToSelf().InTransientScope();
Bind<CookieContainer>().ToSelf().InRequestScope();

This is a representation of our code that was causing the issue. Seemingly what happened was the scope callback for the CookieContainer was HttpContext.Current, and HttpContext.Current was also being referenced by CookieContainer. So Ninject could never prune CookieContainer instances from its cache, as the CookieContainer instances where keeping their scope callback objects alive. When we change CookieContainer's scope to transient, all works fine, as we expected. But I'm still not totally sure why this happened as it seems like this is a fairly conventional thing to do right? Maybe not perhaps...

I am also confused as I thought that if the callback object stayed alive as it did, then shouldn't Ninject just hand back the same instance from the cache, seeing as though the callback was still alive, so the instance should appear to be in scope? Why would ninject keep getting new instances of CookieContainer and caching them? I guess there would be other issues related to the incorrect object coming back, but that would at least just be a bug, not a memory leak.

My question is a) have we diagnosed this error correctly? b) is there a recommended approach to take for this not to happen again? c) Can I put a fix in to the code to check for this type of circular dependancy (assuming we have diagnosed this correctly)?

AaronHS
  • 1,334
  • 12
  • 29

1 Answers1

8

Simply described, the cache is a dictionary of the weak referenced scope object to the instance. As long as the scope is alive the referenced objects are kept alive too. So yes if your CookieContainer referneces HttpContext.Current and is in request scope this standard mechanism will never apply to release them.

But in the special case of InRequestScope there is another releasing machanism implemented by the OnePerRequestModule which will release all InRequestScoped object immediately after the request is complete. If you are using an up to date version of either Ninject.Web or Ninject.Web.MVC3 then it is preconfigured. Otherwise you have to add it explicitly by configuring this HTTPModule in the web.config.

The other point you didn't understand is that Ninject will not return the same object as long as it lives. E.g. in request scope it will return the same object for a request. If multiple requests run at the same time they all get a different instance.

Remo Gloor
  • 32,665
  • 4
  • 68
  • 98