dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.58k stars 10.06k forks source link

memory leak for asp.net core integration tests #48047

Open chenbojian opened 1 year ago

chenbojian commented 1 year ago

Is there an existing issue for this?

Describe the bug

When there are thousands of integration tests, the memory usage become pretty large. I finally located that the WebApplicationBuilder is not garbage collected. So there will be thousands of WebApplicationBuilder instances during the tests run.

Expected Behavior

WebApplicationBuilder should be garbage collected after the WebApplicationFactory<Program> is disposed.

Steps To Reproduce

I created a github repo to demostrate the problem.

I am using the WeakReference<> type to check if an instance is garbage collected. Based on the result, you can find that the WebApplicationFactory is already garbage collected. But the WebApplicationBuilder still exists.

You can run dotnet test to see the failure.

Exceptions (if any)

No response

.NET Version

7.0.201

Anything else?

No response

sharwell commented 1 year ago

If someone creates a heap dump of the test process after many tests have run, I should be able to narrow down the source of the GC root pretty quickly. 😄

chenbojian commented 1 year ago

I tried to use dotMemory to attach to the process. The string in the first image is the environment variable hold by WebApplicaitonBuilder. I am also able to get the call tree in the second image. But I don't understand who is the GC root.

Here is the link to download the memory dump (created with dotnet-dump in windows). If you need other info, I can also get for you.

image image
chenbojian commented 1 year ago

Finally I found the related source code. WebApplicationFactory is using HostFactoryResolver to create Host in another thread.

But I still need time to understand how the memory leak happens.

@sharwell Hope the information can help you to locate the problem.

https://github.com/dotnet/aspnetcore/blob/e422fd329684f2032fd496a7f749199d8985c0df/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs#L171

https://github.com/dotnet/runtime/blob/4679e59ac4431380ff2455b471c9b3e9fda503e6/src/libraries/Microsoft.Extensions.HostFactoryResolver/src/HostFactoryResolver.cs#L62

ghost commented 1 year ago

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document. We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact work.

sharwell commented 1 year ago

Changing this field to AsyncLocal<WeakReference<HostingListener>> should resolve this:

https://github.com/dotnet/aspnetcore/blob/2dde6988c9025fbfe5222c651a0b5425f1ba98c8/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs#L2629

chenbojian commented 1 year ago

Will this issue be fixed along with the .net 8?

dotnet-policy-service[bot] commented 9 months ago

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

rvdintercept commented 3 months ago

We are facing the same issue with unreleased memory, any updates on a fix or workaround?

harlam357 commented 2 months ago

The issue just cropped up for us as our test suite has grown. We are disposing the WebApplicationFactory appropriately, but memory consumption continues to grow.

OwenPattison commented 1 month ago

I can also confirm that the same behaviour is seen in my test suite, @sharwell was your suggested change taken any further at all?

It would be nice to try and find some resolution to the issue, I would love to hear of any workarounds that have been successful on this issue.