Ninject Memory Leak Under Heavy Load?

[Brian Chavez]

Hi,

Our latest project using Ninject has finally made it to release and is
currently in production.

The application is a ASP.NET web-forms application running under IIS7-
Integrated Mode.

After monitoring the server, I've noticed the application consuming
gobs of memory (recycling almost 5 times in a single day). I could
barely keep the system online on launch day because the app got
hammered so much. w3wp.exe would consume about 800MB before the app
would grind to a halt and response times increase dramatically.

To help identify the problem, I created a crash dump soon as w3wp.exe
hit 800MB. Then loaded the process dump file into WinDbg, loaded SOS
extensions, and started examining the heap and memory space.

After examining the heap, I found System.String was the most used
object. But checking !gcroot on many of those objects quickly lead me
to paths through Ninject. For example:

----------------------------------------------
0:000> !gcroot 28c73324

DOMAIN(017F5080):HANDLE(Pinned):11f12ec:Root:099cdf68(System.Object[])-
>
059fc220(ProjectBase.Service.Svc)->
059fc310(Ninject.Core.StandardKernel)->
059fc544(Ninject.Core.Infrastructure.StandardComponentContainer)->
059fc55c(System.Collections.Generic.Dictionary`2[[System.Type,
mscorlib],[Ninject.Core.Infrastructure.IKernelComponent,
Ninject.Core]])->
059fd004(System.Collections.Generic.Dictionary`2+Entry[[System.Type,
mscorlib],[Ninject.Core.Infrastructure.IKernelComponent, Ninject.Core]]
[])->
059fca34(Ninject.Core.Tracking.StandardTracker)->
059fca54(System.Collections.Generic.Dictionary`2[[System.Object,
mscorlib],[Ninject.Core.Tracking.IScope, Ninject.Core]])->
059fd544(System.Collections.Generic.Dictionary`2+Entry[[System.Object,
mscorlib],[Ninject.Core.Tracking.IScope, Ninject.Core]][])->
059fd4b4(Ninject.Core.Tracking.StandardScope)->
059fd4d0(System.Collections.Generic.Dictionary`2[[System.Object,
mscorlib],[Ninject.Core.Activation.IContext, Ninject.Core]])->
0ba707d0(System.Collections.Generic.Dictionary`2+Entry[[System.Object,
mscorlib],[Ninject.Core.Activation.IContext, Ninject.Core]][])->
28c6d9f4(ASP.default_aspx)->
28c708d4(ASP.site_master)->
28c72e00(System.Web.UI.WebControls.Panel)->
28c72f3c(System.Web.UI.Control+OccasionalFields)->
28c72f68(System.Web.UI.ControlCollection)->
28c72f8c(System.Object[])->
28c732ec(System.Web.UI.WebControls.PlaceHolder)->
28c73360(System.Web.UI.Control+OccasionalFields)->
28c7338c(System.Web.UI.ControlCollection)->
28c733b0(System.Object[])->
28c73324(System.Web.UI.LiteralControl)
-------------------------------------------------

In the code behind on many pages on the website, I have something
similar to:

public class SomePage : SomeBasePage{
protected void OnInit(){
Kernel.Inject(this)
}
}

So, my question is the following: Is Ninject trying to perform "object
tracking" on the ASP.NET page itself when the page is Inject()ed? The
ASP.NET page in compiled form is composed of many LiteralControl
(string) objects, and if Ninject is holding a reference to the ASPX
page itself, then Ninject can become a major issue in ASP.NET
applications.

Any thoughts? Am I missing something? Perhaps, I should call
Kernel.Release() at the end of the page life cycle and see it fixes
the problem? How can I prevent Ninject from tracking ASP.NET pages?
Or is this not even a problem with Ninject?

Thanks,
Brian Chavez
Bit Armory, Inc.

[Miguel Madero]

Can you check/post the code where you configure the Kernel? Check
which Behavior you're using by default. I'm not familiar with Ninject
in an ASP.NET Context, but on SL we had several issues with objec
lifetime management. This is what we did:

We defined different "Modules" per use case and created a different
Scope for each one. At the end of the use case we simply dispose the
Scope. Used Transient Behavior (default) for objects we don't want to
reuse, but simply want to create a new instance.

I heard (I'm not sure if I remember correctly), that there's a
SessionBehavior in ASP.NET, however, you don't want to use this for
your pages, you need a TransientBehavior. It would be interesting to
have a RequestBehavior or even better to create a Request level Scope,
so everything you create on that Scope's Kernel will get properly
dispoed/unreference at the end of the request.

However, for most escenarios you simply need a TransientBehavior.

--
Miguel A. Madero Reyes
www.miguelmadero.com (blog)
m...@miguelmadero.com

[David Kirkland]

Miguel, the SessionBehavior you refer to is OnePerRequestBehavior I
think.

I have also seen memory issues in apps we have using Ninject. It's
difficult to diagnose these kind of error though, and cannot place the
blame directly with Ninject. I do have my suspicions though...

In fact Nate added the following comment to the latest subversion
check-in:
Added OnePerRequestModule to fix long-standing problem with
OnePerRequestBehavior.

Anyway, as Miguel said, to begin investigating this it would be
necessary to know which behaviors you are using, and how your modules
are set up.

[Miguel Madero]

David,

That's the one OnePerRequestBehavior, I wasn't close, not even in the
intention, but you still got it right.

Brian,

The best way we found to diagnose them is to use windbg. I guess you
already have a suspect, so you want to know why is that object
remaining in memory. After attaching to windbg and loading SOS
(.loadby sos mscorwks) and the symbosl do the following:

!dumpheap -type YourNamespace.YourClass


This will list you all the instances of the type you're looking for.
Then using the memory address in that list run the following command:

!gcroot <address>

The information can be overwhelming, but that will tell you who is
causing your object to remain in memory.

[Brian Chavez]

Miguel,

If you check my first post on this thread, you'll see I've already
been using WinDbg to troubleshoot this problem. If you see the root
paths that I've posted, you'll see that the Kernel is holding
references by tracking the default_aspx page. My suspicion of Ninject
(or my incorrect use of Ninject) being the root of these problems is
getting stronger.

IMHO, I'm starting to feel that using Ninject in an ASP.NET
application is a VERY BAD idea because of how ASP.NET works under the
hood. If I can't get this issue resolved soon, I'm going to spend
some time ripping out Ninject and replacing it with something a bit
more easier to manage (and less automatic) like LinFu.IoC.

Anyway, here's what I've found:

1. Using ImplictSelfBinding is dangerous in ASP.NET. Why? From my
findings, ASPX/ASCX pages, when deployed, are compiled down into
strongly derived typed classes which inherit from the code behind base
page like so:

public class Default : System.Web.UI.Page{
[Inject]
IFoo SomeProperty{get;set;}
//my code behind stuff....

override OnInit(..){
Kernel.Inject(this);
}
}

// ___Compiler Generated___
public class default_aspx : Default (code behind) {
//...... BuildControlTree has a bunch of LiteralControls() etc...
}

The problem is, you as an ASP.NET developer, do not have access to
default_aspx type during design-time nor can you use it from your code
behind page. Calling Bind<default_aspx>.ToSelf() is impossible. So
what I've done is set KernelOptions.ImplicitSelfBinding=true to ensure
that calls to Kernel.Inject(page) succeed in resolving IFoo in the
code behind page.

Since the Kernel is a singleton object in our application, it is
acting as a magnet for ASP.NET page/control objects that are created
by the ASP.NET compiler/runtime. Imagine if you had N aspx pages and
M ascx user controls, you would now have N+M additional bindings in
Kernel.Bindings.

Not exactly a big deal, but this behavior is NOT something I want
Ninject to do especially by a singleton object in application memory.

Furthermore, the problem is exacerbated by the fact that Ninject has
additional features such as deterministic disposal which track every
ASP.NET default_aspx object instance that gets created on every HTTP
request. *ouch* So, over time, you build up a very big collection of
asp.net pages that never get disposed of correctly.

So, what I need to do is the following:

1. Tell Ninject to STOP tracking ASP.NET compiler generated objects
for disposal. I want to let ASP.NET handle the disposal of the pages
& user controls on it's own terms. ASP.NET does not need Ninject's
help. Ninject needs to keep it's hands off compiler generated types.

2. Tell Ninject to STOP performing self-bindings on every request to
Kernel.Inject(). I did this by setting ImplicitSelfBindings=false.
Now I'm stuck in trying to figure out how to get Kernel.Inject() calls
to work again from the code behind page without having to add self
bindings to default_aspx (and the rest of the asp.net website)
compiler generated types...

-Brian

PS. Sorry if I sound a bit pissed, I've spent almost 3 weeks on this
issue. :-/

[Nate Kohari]

Brian:

Sorry for not getting back to you sooner on this issue, and sorry for the frustration that I'm sure this has caused.

When you call Kernel.Inject(), Ninject will create an automatic self-binding because it needs to be able to generate the "activation plan" for the type of object that you pass in.

The problem that you're seeing is that Ninject 1.x tracks instances that were passed to Inject(), instead of allowing their lifecycle to be managed outside of the container. In order to keep the instance alive, Ninject will hold a reference to it in memory. This protects it from being collected by the garbage collector.

This was simply a bad design decision on my part, and this behavior will not be present in Ninject 2.0.

That being said, the problem should be correctable. Here's a couple things to try:

1. Unless you're using the most-current version in the trunk, there is a KernelOption that tells Ninject what to track. If you're not passing anything to Kernel.Release() already, you should safely be able to set the tracker to track nothing. However, if you tell Ninject not to track anything, you won't be able to pass anything to Kernel.Release() for deactivation.

2. Your Pages and Controls can override Dispose() and pass themselves to Kernel.Release(). This should cause Ninject to release the reference to the objects, allowing them to be garbage collected. When I wrote the ASP.NET integration for Ninject, I was tempted to do this, but I wasn't sure that it was a good idea to make that decision for users.

Please let me know if you continue to run into trouble. I'll do my best to be more responsive next time!

Thanks,

Nate

[Miguel Madero]

Hehehe, sorry Brian, I lost the context of the Thread.

[Brian Chavez]

Ok, here we go. Memory leaks are caused by the following:

* If an object is IDisposable and the object is Transient the Kernel
will track the object in kernel scope. Why?

DisposableStrategy.AfterInitialize overrides all
context.ShouldTrackInstance values and forcibly sets
ShouldTrackInstance = true if context.Instance is IDisposable.

(This would effectively invalidate your
InstancesOfTransientServicesAreNotTracked unit test, if
ObjectWithTransientBehavor had an IDisposable interface)

If you use Ninject.Get<SomeTransientObjectWithDisposableInterface>() N
times, you will end up with N references to N transient objects
with disposable interfaces in Kernel.Tracking.Scope (because
ShouldTrackInstace was forcibly set to true by DisposableStrategy).

If your Kernel is a singleton, well, now you've got a really big a**
memory leak problem on your hands.

------------

Let's take the ASP.NET perspective now.... System.Web.UI.Page,
Control, and UserControl all have IDisposable.

And you call this in your asp.net objects:

Kernel.Inject( asp-netObject )

which happens on *every* http request. LOL... now I'm feeling sick.
*stabs self*

This perfectly explains the memory leak issue and why WinDbg had so
many gcroots through Ninject.

Seriously though, am I the only one running Ninject in on a production
system loaded enough to see the dramatic effects caused by this bug???

Load on this app is growing by the day and I need to get this
situation UNDER CONTROL __SOON__. Thank goodness I tested our engine
on a smaller project before I put Ninject in one of our larger
projects. OMG, it would have brought our cluster to its knees and
cost me thousands in support and really pissed off customers.

------------

Anyhow, Nate, I know you're doing all this for free, but can you give
me any guidance on how to deal with this issue?

I'll be more than happy to patch Ninject itself to restore my sanity.

I'm currently using the latest Ninject trunk version. I cannot use
Ninject 1.x release because I had another issue with ASP.NET that I
posted here:

http://groups.google.com/group/ninject/browse_thread/thread/193f304624f3d4d/7a24f065ba0c9243

Furthermore, I have figured out a way to create bindings OUTSIDE of
the Kernel container, and avoid the use of implicit binding by using:

var targetType = target.GetType();
var context = Kernel.Components.ContextFactory.Create(targetType);
context.Instance = target;
var binding = Kernel.Components.BindingFactory.Create(targetType);
binding.Provider = Kernel.Components.ProviderFactory.Create
(binding.Service);
context.PrepareForActivation(binding);
context.ShouldTrackInstance = false;

Kernel.Inject(target, context);

This works with ImplicitSelfBinding=false and avoids implicit
binding. However, I've found that
DisposableStrategy.AfterInitialization
forces ShouldTrackInstance = true because of the problem i've outlined
above.

I'm thinking what I need to do is completely remove DisposableStrategy
from the kernel and I'll get my memory back (how ironic, lol).

I don't think I can override Dispose() on Pages and Controls because
the impact would be big, so I'm looking towards patching Ninject or
removing DisposableStrategy activation to solve the problem.

Thanks,
Brian

Also, it would be nice to have ManyPerRequest [or TransientPerRequest]
behavior where behavior would be transient within the life type of an
HTTP request, then disposed of *properly* at the end of the an HTTP
request. I have a working patch (assuming we fix this
DisposableStrategy problem) submitted on the bug tracker.

> ASP.NETintegration for Ninject, I was tempted to do this, but I wasn't

> sure that it
> was a good idea to make that decision for users.
>
> Please let me know if you continue to run into trouble. I'll do my best to
> be more responsive next time!
>
> Thanks,
> Nate
>

> ...
>
> read more »

[Nate Kohari]

Brian:

As far as passing the objects back to Kernel.Release() during disposal, I'm assuming you extended PageBase, ControlBase, etc. from the Ninject.Framework.Web project. You could easily make the necessary change in the Ninject codebase.

If you don't want to manage it that way, then you're right, you can either patch the DisposableStrategy so it doesn't force ShouldTrackInstance to be true, or remove the DisposableStrategy completely.

To be honest, you may be the only person using Ninject in this way with WebForms. I've personally used it in high-volume MonoRail and MVC apps, but not anything major with WebForms.

-Nate

[Brian Chavez]

LOL. We finally have this issue under control in production.

To get a feel of what we were dealing with, here's what it looks like
with the memory leak in Ninject (24-hour footprint):

http://tinyurl.com/aoaxjl

All the sharp spikes in the graph are a result of IIS recycling the
application's memory. All IIS recycles were a direct result of
Ninject's memory leak.

After patching Ninject, we now have:

http://tinyurl.com/b2mmm5

Success! Finally, after 3 weeks, we have our server under control.

Hey Nate, while working with creating bindings outside of the kernel,
I noticed IContext is not marked IDisposable. Would it be wise to
mark IContext : IDisposable? Thanks.

-Brian

On Feb 8, 6:32 am, Nate Kohari <nkoh...@gmail.com> wrote:
> Brian:

> As far as passing the objects back to Kernel.Release() during disposal, I'm
> assuming you extended PageBase, ControlBase, etc. from the
> Ninject.Framework.Web project. You could easily make the necessary change in
> the Ninject codebase.
>
> If you don't want to manage it that way, then you're right, you can either
> patch the DisposableStrategy so it doesn't force ShouldTrackInstance to be
> true, or remove the DisposableStrategy completely.
>
> To be honest, you may be the only person using Ninject in this way with
> WebForms. I've personally used it in high-volume MonoRail and MVC apps, but
> not anything major with WebForms.
>
> -Nate
>

> >http://groups.google.com/group/ninject/browse_thread/thread/193f30462...

> ...
>
> read more »

[Nate Kohari]

Brian:

Glad to hear the problem has been fixed. What was the resolution?

Thanks,

Nate