Dnn.platform: Possible memory leak (dependency injection)?

[allanedk]

Description of bug

I'm sorry if this is not the right place to post this. But I suspect that Dependency Injection implemented in DNN 9.4 is causing memory leaks and if so it might be important for others too.

After upgrading from DNN 8.0.4 to 9.4.1 the memory consumption of my website (the w3wp process) is increasing indefinitely until all server resources is consumed. The site is not slowing down or affected in any way though, i.e. no errors in the admin logs.

The site is not using any 3rd part modules. But only "thin" frontend modules pulling data from custom build web api services (a project with controllers that inherits from DnnApiController).

No source code was changed when upgrading other than changing target .NET Framework version of the project to 4.7.2. No compiler errors and no errors in the DNN log.

I've been "debugging" in production by deactivating the use of different parts of the service, but without luck.

When analyzing a memory dump of the IIS w3wp process (using dotMemory from Jetbrains and DebugDiag from MS) I see the the garbage collector heap usage (generation 2) is quite large with about 2/3 of the total memory usage. Also there are a lot of "objects ready for finalization". (see https://ibb.co/Yt1GVkG )

And at the top of memory usage list is _Microsoft.Extensions.DependencyInjection.ServiceProvider_ (see https://ibb.co/vjK5rsc ).

I've run out of ideas. If this is caused by Dependency Injection, then others might experience the same. Perhaps without knowing it yet. I have a lot of traffic on the site (20-25k user session per day) and the memory consumption hits 2 GB in about 6 hours at which point the app pool recycles.

Is it possible for this to be related to DI, perhaps in combination with using DnnApiController (my code was not changed during upgrade)? Could DI prevent the garbage collector from eliminating dead objects?

If needed I can add source code examples (even though no exceptions are thrown) or more info from the memory profiler.

Thanks a lot.

Steps to reproduce

I have not been able to reproduce this locally. Perhaps a different stress testing tool is needed. Or maybe it's related to the production server environment (Windows Server 2008 R2 + IIS 7.5)

Current result

Ever increasing memory consumption.

Expected result

Memory usage should be stable. It was stable at 600 MB in DNN8 using the same code.

Screenshots

This is screenshots of some of the memory profiler reports:
https://ibb.co/Yt1GVkG
https://ibb.co/vjK5rsc

Error log

No errors in error log.

Affected version

• [?] 10.0.0 alpha build
• [x] 9.4.1
• [?] 9.4.0

[mitchelsellers]

At first glance, the existence of a single instance of the DI items is expected (they stay there for resolution and by the nature of how .NET Pushes objects through the GC levels it should make its way to Gen2 after some time.

@ahoefling thoughts on this?

[ahoefling]

The theory behind the Dependency Injection implementation is to resolve everything at request time. Which is how DNN has implemented it's many module patterns (MVC, SPA, WebForms, etc.). The Dependency Injection container will try and find the type requested and resolve it.

We may be running into an issue where the type is being resolved but the Garbage Collector is never being ran on those types because they are not being disposed correctly.

Here is a snippet of the DnnApiController
```c#
public abstract class DnnApiController : ApiController

Where the `ApiController` implementes `IDisposable`
```c#
public abstract class ApiController : IHttpController, IDisposable

In SPA Modules the WebAPI Controller Activator resolves an instance of the requested controller when it is requested:
c# public class DnnHttpControllerActivator : IHttpControllerActivator { public IHttpController Create(HttpRequestMessage request, HttpControllerDescriptor controllerDescriptor, Type controllerType) { return (IHttpController)Globals.DependencyProvider.GetService(controllerType); } }

The DNN Web API implementation leverages the default ASP.NET Web API implementation, which should handle this use-case

My Thoughts (Potential Fixes)

With all of this information since DnnApiController is implementing IDisposable it is possible that the Dispose() method is not being invoked, but since it is a method that lives in the ApiController I would find it hard to believe that the Web API pattern is not calling that.

Registration Problems

We could try updating all of our Web API controllers to be registered as a Singleton and they may prevent this from happening, but the memory footprint of maintaining a reference to EVERY Web API Controller is pretty large. The idea was to create and destroy as needed.

Controller Activator

The ControllerActivator is used to resolve an instance of the correct controller and return it. There is a built-in Dependency Injection mechanism that we did not use (which may be by design). We could try using the GlobalConfiguration.Configuration.DependencyResolver and assign the DNN DependencyProvider. This would prevent the controllers from being stored in the Container and only inject the types.

This potential solution has many complications.

Next Steps

To properly test any solution, we will need a reproducible module or full website that we could test a fix on.

@allanedk

• is this something you would be able to assist on?
• Where you able to reproduce this on a production backup or was it only on the live production server?

[mitchelsellers]

@ahoefling I have a feeling that the Dispose() theory might be the most logical suspect, given the memory root items reflected in the linked trace.

[allanedk]

Thank you very much for looking into this and for your thorough replies! I'll try to find a way to reproduce this locally and provide you with a debugable version.

In the meantime: I was thinking about the Dispose() theory too. In one analysis using the memory profiler I saw 16.000 objects/instances of a fairly simple HttpResponseMessage method.

It is called from the user clients (on most pageviews) using HttpGet. The only thing happening in the code is 1) writing a record in the database and 2) returning a response to the user.

It should not cause any memory leak. The DB-call is encapsulated in a using-statement.

Can I test it by somehow forcing a Dispose() somewhere?

[ahoefling]

The only way to test is to generate a new-installer with a patch and apply it or side-load dlls with the changes. In the SPA Web API modules there is no direct access to the Controller like there is in the MVC module which means we can't just add a new line of code that invokes the Dispose() method. I am actually quite surprised that the Web API lifecycle manager is not disposing the resources correctly since all ApiControllers are implementing IDisposable.

Perhaps a very easy test for us to run would be updating all DnnApiController Registrations to be a Singleton.

Currently when the DnnApiControllers are registered (this happens automatically) it registers it as Transient which means every time Dnn tries to resolve the Controller it gets a new instance. If we update this to Singleton we may be able to reduce the memory footprint of large request cycles, but this will bloat the container and slow-down first-time startups.

@mitchelsellers what do you think?

• Option1: Update all DnnApiControllers to singletons
• Option2: Update SPA Web API dependency injection to use the DependencyResolver
  
  
  
  • I think this is the appropriate path, but much harder than option 1
  
  

[mitchelsellers]

I would agree, this "should" be handled for us by the framework already....

I'm concerned that we might have unintended impacts if we use Singletons for Controllers since that scope would apply to any demanded resource. (For example, if a user was using EF it would also be singleton which isn't good.)

I think the Option2 route is a good start, but it would be ideal if @allanedk could re-create consistently

[allanedk]

I was able to re-create it locally by stress testing a single method in the DnnApiController directly instead of loading a full DNN page - it's way faster and makes it easy to see the problem.

Memory consumption skyrockets and doesn't seem to drop again.

Let me create a new project with the same structure as my current prod project but only one method. I'll post it here when done.

[ahoefling]

@allanedk

I was able to re-create it locally by stress testing a single method in the DnnApiController

This is great news! Thanks for the hard work in getting us something concrete to test this.

@mitchelsellers

I think the Option2 route is a good start

I agree 100% as the singleton route sounded scary to me as well.

Once we get a sample module from @allanedk with instructions on how to reproduce this I'll put together a Pull Request with a proposed fix to this problem.

[allanedk]

Ok, I have created a new project with only one function in order to test this.

It's using DnnApiController and is returning a transparent GIF image (pixel), so it's easily testable. Reproduce by following these steps:

1. Download and unzip this: MyServices.zip

2. Copy MyServices.dll from debug folder to bin-folder of local DNN 9.4.1 instance (no need for installing/adding as module in DNN)

3. Open this in browser: http://localhost/desktopmodules/myservices/api/log/logtopicview?topicid=1

4. To speed up the test, use a stress testing tool like West Wind WebSurge (free edition available). I have added a config file for use with this tool. Just unzip and open this "Session File" in WebSurge:
   myservices-stresstest.zip

5. Run load test and watch Task Monitor for increasing memory consumption. Even if you stop hitting the server, the memory consumption will not decrease (until the app pool is recycled of course)

On my machine it hits approx. 1 GB of memory consumption (w3wp process) at 10.000 requests. After that it adds about 100 mb per 10.000 requests until all memory is consumed at which point it slows down and begins throwing exceptions. On my prod server it seems to be increasing at a higher rate (although I haven't measured it). I guess its because the function is performing more work (DB call) than this one.

I hope it's behaving the same way when you try it :)

[ahoefling]

Thanks for sharing detailed steps on how to reproduce this, I have confirmed and verified that this is a problem based on your sample module and load testing tool:

VERIFIED

My Results

• IIS Idled with a fresh recycle: 137.2 MB of memory
  

• 11,795 Requests (6 seconds): 1,329.0 MB of memory
  

• 52,772 Requests (27 seconds): 2,530.3 MB of memory
  

• 102,894 Requests (55 seconds): 3,582.9 MB of memory
  

• 497,417 Requests (360 seconds): 11,553.2 MB of memory - towards the end the site was almost unusable
  

Next Steps

Now we have a way to reproduce this and confirm it, we can start working on a fix for this

[kurtwilbies]

And what about 9.3.x and previous releases?

[mitchelsellers]

@kurtwilbies If this is, in fact, something introduced by the DI changes, 9.3.x would be not impacted.

I don't have time to repeat the test on a 9.3.x installation today, however.

[kurtwilbies]

The question remains: is this because of DI changes? I'll do some tests.

[ahoefling]

@mitchelsellers

@kurtwilbies If this is, in fact, something introduced by the DI changes

This should not exist in 9.3.x but we have not verified. We are pretty sure this is related to the new DNN DI changes added in 9.4.0, per the issue being created as an upgrade.

@allanedk

After upgrading from DNN 8.0.4 to 9.4.1 the memory consumption of my website (the w3wp process) is increasing

[ahoefling]

@allanedk @mitchelsellers
I have found a fix to this and the Memory Leak appears to be fixed, I submitted a PR so we can get this in for the next release

@allanedk
Can you checkout the installer generated by the build and verify it works?

[allanedk]

Great!!! Thanks a lot @ahoefling !

I'm not sure how to test it, though. The PR (#3192 ) was rejected, so I can't pull it from here.

Should I wait? Or perhaps clone your repo (web_api_performance branch) and use that for testing?

[ahoefling]

@allanedk there are some failing unit tests you'll need to clone and build it locally, I put instructions in the PR https://github.com/dnnsoftware/Dnn.Platform/pull/3192#issuecomment-545431840

[allanedk]

I have problems building it. It seems to be a problem with MSBuild.

The default XML namespace of the project must be the MSBuild XML namespace

Is it necessary to have and older version of that installed, i.e. by installing VS2017? I only have VS 2019 installed.

[ahoefling]

I am not 100% sure why you are seeing that error, I know multiple people have ran the build without issue. The only thing I can think of is make sure you clone the repo as close to the root of your drive as possible otherwise you run into path too long errors. For example I cloned my repo in D:\x

@valadas do you have any thoughts on why @allanedk is getting this error?

[allanedk]

I moved it to c:\git\ but still fails. I agree that it can't be a problem with the project, but an local issue on my side. I was just wondering if it's a known "limitation" / "prerequisite" that an older version of MSBuild is needed. The full error message is (there are multiple of similar errors):

C:\git\Dnn.Platform\DNN Platform\DotNetNuke.ModulePipeline\DotNetNuke.ModulePipeline.csproj(1,1): error MSB4041: The default XML namespace of the project must be the MSBuild XML namespace. If the project is authored in the MSBuild 2003 format, please add xmlns="http://schemas.microsoft.com/developer/msbuild/2003" to the <Project> element. If the project has been authored in the old 1.0 or 1.2 format, please convert it to MSBuild 2003 format.

But please, @ahoefling, don't spend more time on my build issues - I'm just happy that you helped with the leak! I'll figure it out on my own.

Of course if @valadas has the answer, I would be happy :)

[ahoefling]

Sorry you are having so much difficulty getting it to work, I know the team is working hard on solving a lot of the build problems and hopefully after the work is completed you will be able to build locally.

I just pushed changes to the PR which generated a working installer, everything appears to be working as it did before.

https://dev.azure.com/dotnet/DNN/_build/results?buildId=23677&view=results

[allanedk]

Great! I have tested this and can confirm that it works like a charm and solves the problem.

I'm closing this issue and looking forward to the final release.

Thank you so much for the great work, @ahoefling !

[valadas]

My guess is that something locally uses an older version of msbuild...

Can you run msbuild /? to see which version you are using. Here it comes up as 16.1.76

If you have both VS 2017 and VS 2019 installed, I know there was a bug where it would pickup the wrong 'latest' version of MSBuild and that got resolved by updating both VS2017 and VS2019

[allanedk]

Thanks @valadas

The build.ps1 writes:

`Restore-NuGet-Packages

MSBuild auto-detection: using msbuild version '16.3.2.50909' from 'C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\bin'.
`

I don't have VS2017 on this machine. However VS Code is installed...don't know if that's important.

[allanedk]

But again, it's not important to me to be able to build DNN. Unless - of course - it's helpful to the project that I test this fix on my site.

[valadas]

Hmm, then I am not sure what the problem could be...