Dnn.platform: Possible memory leak in MVC modules

[jsbsantos]

Description of bug

Hello.

I have a project running in production, which I upgraded to the 9.4.1 version of DNN. After the upgrade I noticed that there was an unusual memory usage, closing in on max, resulting in website restarts.

Searching in the reported issues, I found one issue related to a memory leak in the API, which was fixed in DNN 9.4.2 version release.

I went on to upgrade to the 9.4.2 on my dev environment to test things out. Turns out there was still a high memory usage.

For my next test, I installed a fresh DNN site, with no modules and ran a load test (20 concurrent users for 5 minutes) and the results were good, with normal memory usage. After that I installed a simple module, created with a DNN template based on Chris Hammond's template. This next test was done by adding this module to an empty page and running the same load test, which, turned out to be spot on: memory usage increased rapidly and was close to max.

I am attaching the module I used to test this behavior.

Steps to reproduce

1. Install the attached TestModule
2. Add TestModule to any page
3. Run a load test on the page containing the module
4. Observe memory usage for IIS process

Current result

Memory usage increases rapidly until it's maxed out.

Expected result

Memory usage should remain stable.

Screenshots

1) Memory Usage for IIS process running the clean test installation with a test module

Additional context

I took a process dump from my production website and after analysis I theorized that the leak was related to the new DI. Digging into the dump file showed that the ServiceProvider is not disposing of module controllers, as you can see by the image bellow.

Affected version

9.4.0, 9.4.1, 9.4.2

• TestModule_00.00.01.00_Install.zip
• TestModule.zip

[Bladixx]

Hello

I have a project running 9.4.1 and also had this problem. My memory is always max out until my AppPool recycle.

This is causing several crashes on my production environment.

Can you help out, base on what @jsbsantos described?

Thanks!

[valadas]

It depends if your memory leak is from mvc or web-api. The major fix in 9.4.2 is with webapi, I would upgrade to 9.4.2 to begin with.

[Bladixx]

Hello

Already done that. I upgraded my project to 9.4.2, hopping that the major fix could resolve my problem. But, doing a simple test load, the result were the same to 9.4.1.

I done the same tests listed by @jsbsantos, and obtained the same results he had. My memory is always max out when i have a MVC module, consuming all my resources.

[jsbsantos]

@valadas I ran the same tests in the 9.4.2 version and got the same result. Upgrading won't fix it.

[valadas]

@ahoefling do you have any clue on this, wasn't there already an issue for this?

[ahoefling]

MVC and Web API use different pipelines in DNN. The Web API Pipeline runs independent of DNN inside of IIS, where MVC uses a reverse-engineered MVC pipeline built when ASP.NET was closed source.

I was able to reproduce this error in the provided sample MVC Module, which will be a good testing case as we try and come up with a fix for this. ✅

If anyone is trying to solve this here are some things I have tried:
Saving the Instance of IController
```c#
public class DnnMvcControllerFactory : DefaultControllerFactory, IControllerFactory
{
private readonly Dictionary _controllers = new Dictionary();

protected override IController GetControllerInstance(RequestContext requestContext, Type controllerType)
{
    if (!_controllers.ContainsKey(controllerType))
        _controllers.Add(controllerType, (IController)Globals.DependencyProvider.GetService(controllerType));

    return _controllers[controllerType] ?? base.GetControllerInstance(requestContext, controllerType);
}

}

This doesn't work because ASP.NET throws an exception if we try re-using the same controller for multiple requests.

**Using the ActivatorUtilities and Reverting to Old Techniques**
```C#
public class DnnMvcControllerFactory : DefaultControllerFactory, IControllerFactory
{
    private readonly Dictionary<Type, IController> _controllers = new Dictionary<Type, IController>();

    protected override IController GetControllerInstance(RequestContext requestContext, Type controllerType)
    {
        var controller = (IController)ActivatorUtilities.GetServiceOrCreateInstance(Globals.DependencyProvider, controllerType);

        return controller ?? base.GetControllerInstance(requestContext, controllerType);
    }
}

This is functional but the memory leak still occurs

Forcing The Release Controller
Using the same code from the last technique, I tried forcing the Dispose and then null the controller.
```c#
public class DnnMvcControllerFactory : DefaultControllerFactory, IControllerFactory
{
private readonly Dictionary _controllers = new Dictionary();

protected override IController GetControllerInstance(RequestContext requestContext, Type controllerType)
{
    var controller = (IController)ActivatorUtilities.GetServiceOrCreateInstance(Globals.DependencyProvider, controllerType);

    return controller ?? base.GetControllerInstance(requestContext, controllerType);
}

public override void ReleaseController(IController controller)
{
    if (controller is IDisposable disposable)
        disposable.Dispose();

    controller = null;
}

}
```
This is functional but the memory leak still occurs

My Thoughts
It is still presenting as a Memory Leak even with all the techniques I have tried. I wonder if this memory leak has been a problem for awhile now, but has gone undetected. I would hope that is not the case.

The .NET Garbage Collector will hold onto anything that has a reference to the object. In the ModuleApplication.cs where the bulk of the MVC Module is processed it resolves the IController to a variable called controller this object is then released at the end of the pipeline. However this object is then passed to another variable called moduleController which is then used in the return object. In my experience with the .NET Garbage Collector that would mean the object is still being held onto and it won't properly be cleaned up and destroyed.

I am not 100% convinced the problem is in the IControllerFactory, that is just where I started because it controls the lifecycle of the resolved or instantiated IController. I am now leaning more towards poor memory management and the objects are still being referenced after the fact.

We may be able to learn something from how Dependency Injection libraries have supported this mechanism in the past with AspNetWebStack and see if we can implement a similar solution.

No Quick Fix
I don't have any temporary fixes for this, so we are going to have to keep looking at the problem. I have limited time to look into this, if anyone wants to try solving it I am more than happy to discuss solutions in this thread.

[thabaum]

I have noticed my production sites randomly restarting as well not really knowing to test like this to find the memory leak. I will take a deep look into this myself to see if I can understand anything given what was provided to help troubleshoot. Look forward to a solution. Thank you.

[valadas]

Merged, will publish 9.4.4 RC-1 very shortly for testing with only this bugfix. If everything goes smooth we would release that next week.

[jsbsantos]

@valadas @ahoefling Thank you.

I'll wait for the 9.4.4 RC and I'll test it again. I'll update if I find any problems.

[valadas]

@jsbsantos it is published now if you want to test and report back. https://github.com/dnnsoftware/Dnn.Platform/releases/tag/v9.4.4-rc1

[jsbsantos]

@valadas I saw it after submitting my comment.

I tested it on the same conditions as before and it seems to be working. No memory noticeable memory increase, which is a good sign.

I'll try to test this in a bigger and more complex website, with more customization and update with results.