Decoration memory problem

Jan 24, 2013 at 3:42 AM
Edited Jan 24, 2013 at 3:43 AM

Hi,

I am using SimpleInjector in my ASP.NET MVC 4 web app, and some time ago I found that my app takes a lot of memory on my WFE server. Each request adds more and more memory and after few days application takes about few gigabytes. I can easy press down F5 button in my browser and see how memory is growing up.

I noticed that this problem started after migration on SimpleInjetor and I start to discover this problem.

On each request my app restore user context information from DB using EntityFramework. Registration in my composition root:

_container.Register<IUsersService, UsersService>();
_container.RegisterDecorator(typeof(IUsersService), typeof(SomeUsersServiceDecorator));

I found that when I disable SomeUsersServiceDecorator memory problem is stopped. After that I try to remove all additional dependencies and code from the decorator but it has no effect:

public SomeUsersServiceDecorator(IUsersService wrapped)
{
	_wrapped = _wrapped;
}
... other methods without additional code

Can you help me? Where it may be a problem?

Coordinator
Jan 24, 2013 at 6:45 AM
Are you creating more than one Container instance during the lifetime of your application? A new instance per request perhaps?
Jan 24, 2013 at 7:00 AM

Yes, Container instance creates in UoW object, which creates per web request. Is any problems to create more than one instance of Container?

Coordinator
Jan 24, 2013 at 12:00 PM

Typically you should only create one container instance per application/AppDomain, as stated here. There are several reasons for this.

First of all, the framework is highly optimized for use of a single container. It compiles delegates (and caches them), which means that any second request for a certain type will hit the cached delegate. This optimization (among others) make Simple Injector one of (if not the) fastest containers for .NET.  When you create a new container for each request however, it means that each request the whole preparation and compilation cycle is triggered again. If your configuration is complex, this could give a multiple seconds delay.

Another reason to have a single container instance is because configuring the container correctly can become utterly complex. Just imagine how you must register types that should get a lifetime that outlives the request. Types registered as singleton (by calling RegisterSingle) will only live for as long as the container lives, which is in your case a request. It is doable to do this registration, but it will soon get pretty hard.

In your case however, there is an unfortunate third reason. There is a design flaw in the RegisterDecorator methods that I never thought anyone would notice and you would only notice when you mix the use of RegisterDecorator with the infinite creation of new Container instances (as you are doing). There's an internal class that contains a thread-static dictionary that stores a reference to the container the decorator in question is applied to. It does so for thread-safety reasons. In your case however, this results in those container instances to be referenced for as long as the thread lives. In ASP.NET threads are pooled, and they tend to live forever. And this results in the memory leak.

I am trying to address this issue in the coming major release of Simple Injector, but still I urge you to not create a container per request, since this will result in many other problems along the way. This advice does not only hold for Simple Injector, but this basically holds for all DI frameworks out there.

Instead create a single container during the start-up path of the application. For registrations that should only live for the during of the request, use the Web Request Lifestyle.

Marked as answer by dot_NET_Junkie on 11/5/2013 at 7:36 AM
Jan 24, 2013 at 2:03 PM
Edited Jan 24, 2013 at 2:04 PM

Thank you, I think I need to redesign my app.

By the way, I would like to ask one more question: why the reason that SimpleInjector throws the exception when Register delegate returns null?

My situation: I would like to register IUserInfo:

 

_container.Register(() => { /* code to retrive user info */ });

But sometimes I cant create an instance of IUserInfo (for example user not authentificated) and would like to inject null. Some modules can work with nullable user context, otherwise not, and throws the exception.

Coordinator
Jan 24, 2013 at 4:15 PM

The reason not to allow null to be returned is a deliberate design decision. A service can either be resolved or an exception is thrown. This makes it easier to find configuration errors and it prevents your application logic to be polluted with if-null checks. Most frameworks have similar constraints.

In your case you should either return some sort of Null Object (not to confuse with a null reference) that represents an unauthenticated user or register a factory instead that allows returning null (although in general it is best to prefer the Null Object Pattern and prefer returning null references). This UnauthenticatedUserInfo info could throw an exception when one of its members is accessed, or perhaps it would be good to apply security at a higher level in the application. For instance, you could write a decorator that wraps all business operations. And this decorator could check whether the user is authorized. Nice about this is that this moves the responsibility from your MVC layer to the infrastructure (the DI registration). Here is an article that describes how to model these kinds of business operations.

Another option is to let the registered delegate throw an exception when their is no logged in user. But be careful with this; this could easily lead to a configuration that is not easily verifiable. Try to keep your container's configuration verifiable.

Coordinator
Feb 25, 2013 at 1:29 PM
Simple Injector 2.0 has just been released and it solves the memory problem with decorators. Note though that creating an infinite number of containers (such as one per request) is still not advised.