Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asp.Net Core API controller memory leak #45098

Closed
1 task done
yarong-lifemap opened this issue Nov 15, 2022 · 25 comments
Closed
1 task done

Asp.Net Core API controller memory leak #45098

yarong-lifemap opened this issue Nov 15, 2022 · 25 comments
Milestone

Comments

@yarong-lifemap
Copy link

yarong-lifemap commented Nov 15, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

We have identified an issue with Asp.Net core API controllers, specifically related to memory allocation which does not get cleared.

Our original use case was an API controller that reads a JSON file and returns data from it. We noted that memory use was growing every time we called the API controller.

We have generated a simplified example to reproduce this issue. Here, we use the following method to generate a 50 million character string and puts it into an object that is not used at all.

[HttpGet("[action]")]
public IActionResult Test1(int testNumber)
{
    var result = new string((char)(new Random().NextInt64(1, 65535)), 50_000_000);
    var mem = Process.GetCurrentProcess().PagedMemorySize64;
    return Ok($"TEST #{testNumber} ; Mem: {mem:N0}");
}

However, the memory still seems to be allocated and retained for this object for a very long duration (GC is run but the memory doesn't seem to be released). Interestingly, at some point, this hits a peak memory allocation, which differs based on the size of the string created.

The problem is also true when deserializing JSON files (which is what we started with) and returning a subset of information from the result object. In that case, the memory allocation for the complete object doesn't seem to go away, causing a server to get overloaded.

In .Net 7, memory is never released, it seems. In .Net 6, some of the memory is released, some of the time, but we still don't understand why it's even stored in memory at all for any duration, since the object is not used for any purpose, and once the API call has completed, should be released.

You can test this issue with the attached solution. Once the project is run, monitor your Visual Studio's Diagnostic Tools window (Process Memory). The API calls also return some information about memory, but the graph represents the issue better IMHO.

Expected Behavior

Objects are released from memory after method completes execution and the API response is returned

Steps To Reproduce

TestSolution.zip

Run the TestWebAppCore project. Click on the links. Watch the memory not get released, but hit a top limit at some point per test.

Exceptions (if any)

No response

.NET Version

.Net 7.0.100 and .Net 6.0.400

Anything else?

MemoryLeak

@yarong-lifemap yarong-lifemap changed the title Asp.Net Core controller memory leak Asp.Net Core API controller memory leak Nov 15, 2022
@davidfowl
Copy link
Member

I haven't looked, but allocating a single 50MB/100MB/500MB string per request is a recipe for bad performance, see https://learn.microsoft.com/en-us/aspnet/core/performance/memory?view=aspnetcore-6.0.

Though this is bad, it should just make the GC run way more often. It's likely that you are also running into this #45037.

@yarong-lifemap
Copy link
Author

While GC.Collect very slowly reduces the memory, I don't believe it's a viable solution. In our original use case, which loads data from a JSON file and returns a subset of that information, it takes minutes and several GC collection cycles to eventually get rid of all the used-up memory.

What I'm proposing is this: The memory should be cleared immediately. Assuming we have 1000 users using our site, and each consumes 10MB of data via an API controller within a given minute or two, why would the expectation be that the machine is required to have 10GB of RAM (1000 x 10MB) to that it can hold those objects in memory?

@davidfowl
Copy link
Member

This isn't how the GC works. It's never worked this way but I'm not convinced this application you've shared is replicating you're actually experiencing. If it is, then the application needs to change to not allocate giant strings that end up on the large object heap. I've linked a high level overview that should help understand why this doesn't work well. For a deeper dive on how to think about memory in a .NET application, read https://github.com/Maoni0/mem-doc/blob/master/doc/.NETMemoryPerformanceAnalysis.md.

It's a deep dive, but answers all of the questions that you will eventually have about memory in .NET.

@davidfowl davidfowl added this to the Discussions milestone Nov 15, 2022
@oferze
Copy link

oferze commented Nov 15, 2022

It happens even in a small API endpoint footprint. In the demo app, if you add:

        [HttpGet("[action]")]
        public IActionResult GetMem(int testNumber)
        {
            var mem = Process.GetCurrentProcess().PagedMemorySize64;
            return Ok($"Mem: {mem:N0}");
        }

and call just this endpoint, you'll see that the memory keeps going up.

I also tried your advice on Twitter to test it not inside Visual Studio and it still happens (Azure App Service, published as "Release", target win-x64).

Seems like ASP.NET API on .NET 7 has an issue of not cleaning up after the endpoint return.

Either it's a serious bug, or we are missing something basic (hopefully the latter).

@davidfowl
Copy link
Member

Is there a specific issue you are having where you are seeing out of memory issues? Did you look at any of the references I posted? Is the garbage collector running? I really recommend reading:

I know time is short and you're trying to solve an issue but:

  1. I don't see an issue based on any evidence presented so far so I'd like you to collect more data so we can get a better understanding.
  2. Get a better understanding of how the GC works based on reading some of those articles.

If you want a TL;DR, the GC doesn't just collect when you think it will, it'll do so when it thinks it should.

While GC.Collect very slowly reduces the memory, I don't believe it's a viable solution.

The fact that this works tells me you don't really have a memory problem. At least, not one that I see as yet.

@ChristianPejrup
Copy link

Hi @yarong-lifemap
If i remember my .NET memory days correct (and .NET Core and Full Framework use roughly the same GC) then i think you problem is the size of the string you are allocating.

Any object bigger than 85.000 bytes will be allocated on the large object heap LOH

The garbage collection of LOH is different from the normal garbage collection in the sense that it only runs whenever .NET determines that its needed (eg. when you are running out of memory), this is because compacting memory consumes CPU cycles so if you constantly use the CPU to compact the memory then that CPU cannot also be used to serve requests (basically it's a performance optimization).

I would guess that you can reproduce this memory leak in any version of .NET and .NET Core.

I'm not going to be the judge of "if you really need this massive size json (i would however recommend that you gzip (browsers support this out of the box) the response before returning it to your consumer)".

Assuming that you need Mb size memory allocation to solve the problem you are facing I would recommend that you take a look at MemoryPool which class that can help you do memory allocations and reuse of them which will move the GC responsibility from .NET Runtime into your code (before starting to use memory pooling i can recommend this article which highlights some of the pitfalls of the technique)

@KSemenenko
Copy link

I did test

MemTest.zip

For just calls:

Total Allocated:203119
Total Memory:72812

Call GC.Collect() after requests:

Total Allocated:202525
Total Memory:-127

I also somehow subconsciously expected that there would be no alocations.

But still string is an object and it is logical that it needs memory.
And after calling GC.Collect() everything falls into place.

@oferze
Copy link

oferze commented Nov 15, 2022

@KSemenenko thank you for the test app.

Few points I wish to address:

  1. One shouldn't manually run GC.Collect(): https://learn.microsoft.com/en-us/aspnet/core/performance/memory?view=aspnetcore-7.0#call-gccollect. Yes, it's good for debugging (as we're doing right now) but the point of the issue is to show that under normal .NET 7 API usage, objects seem to not be collected automatically.

  2. Even your app shows a slow increase in memory consumption. Indeed, it only returns a short string, hence many hits are needed and the memory usage increases slowly. Without GC.Collect(), it seems like the memory doesn't get cleaned. Our demo (the project @yarong-lifemap attached above) uses the 50MB+ objects just to show the scenario quicker.

  3. This is also answering @davidfowl's question: We do have a real world scenario of returning ~4MB JSON content in our API, and we don't believe it's an unreasonable case. Even 50MB should be able to return with reasonably frequent collection.

We think it's a regression with .NET 7 since .NET 6 somehow does it better.

@oferze
Copy link

oferze commented Nov 15, 2022

@davidfowl we have a fair knowledge of the GC (albeit we'll read the articles you've linked to and no doubt we'll learn more from them) and of course we know the GENs, the LOH, and that GC will collect when it needs and not when we expect.

I'm not convinced this application you've shared is replicating you're actually experiencing

We do return 4-6MB JSON objects in our real app as I mentioned, but I guess an API should return much larger strings and not leak. The demo app does reflect and is even more "pure" because our "real" app also deserializes from a GZip stream and does other things before returning that JSON. The demo app in this issue just returns a big string with nothing else, no IDisposable objects used, very clean.

Now, we'd love to hear that it's a normal behavior and that the memory will only grow based on what's available on the machine and will collect and that we shouldn't worry, but we saw that in our very early steps of migrating a big app from .NET Framework to .NET Core - those early steps are meant to validate that our skeleton is sound before we dive into lots of implementation details - and we fear that once we go to production, the memory will quickly bloat and crash the app.

@KSemenenko
Copy link

@oferze This is an interesting thing, look, you make a 50MB line, it is stored in the memory.
the method finished - done.

Now you have a 50mb string in memory.
And now you have to wait for GC to remove it. But this is an expensive operation and it will not happen immediately.
I just tried to do a version of the tests with threads, but the steel is only worse.

So on the one hand this is understandable behavior.
On the other hand, when you need to transfer a 250 MB document or 50 MB JSON, what is the right way to manage memory here?

especially if you have a MininalAPI service, with 500 RAM in the docker, which just has to transfer data from stream to http respnce stream.

@davidfowl
Copy link
Member

davidfowl commented Nov 15, 2022

By using streams. When you use the result types with objects, it doesn’t first buffer 50Mb into memory because they would be horrible for performance. Instead, it writes your 50Mb in 16K (configurable) chunks asynchronously to the response stream.

@KSemenenko
Copy link

@davidfowl so, just in Responce.Body.WriteBytes() and periodically call Flush?

@oferze
Copy link

oferze commented Nov 16, 2022

@davidfowl to summarise, what's the right way to return 5MB Jsons in API responses without .NET increasing memory and not cleaning after itself? this continues @KSemenenko's question.

@davidfowl
Copy link
Member

Show me the code snippet you use today and I’ll rewrite it for you.

@hafizsyedirfanali
Copy link

The memory leak problem is resolved in new update Version 17.4.1.

@syska
Copy link

syska commented Nov 16, 2022

The memory leak problem is resolved in new update Version 17.4.1.

Of what package?

@InCerryGit
Copy link

Hi, have you tried running the program using dotnet run -c Release? As far as I know, the GC is not that aggressive in the VS debugger under Debug configuration.

@hafizsyedirfanali
Copy link

hafizsyedirfanali commented Nov 16, 2022 via email

@davidfowl
Copy link
Member

Things here aren’t adding up, but if the issue is resolved then great 👍🏾.

@oferze
Copy link

oferze commented Nov 16, 2022

@davidfowl I think there's a fundamental misunderstanding... the problem we have is not in our app (thus needing to learn about the GC or to fix our code). As shown in the demo app, it's with every API call and even on Azure App Service, and - much more prominent in .NET 7 than in .NET 6 (if exists in 6 at all).

Kindly referring to my question here as well as this sentence from here:

Now, we'd love to hear that it's a normal behavior and that the memory will only grow based on what's available on the machine and will collect and that we shouldn't worry, but we saw that in our very early steps of migrating a big app from .NET Framework to .NET Core - those early steps are meant to validate that our skeleton is sound before we dive into lots of implementation details - and we fear that once we go to production, the memory will quickly bloat and crash the app.

@davidfowl
Copy link
Member

Maybe, but you haven’t provided a sample that I can make sense of. The 50MB string per request isn’t a good one. Regardless, if the issue is resolved, we can close the issue.

@oferze
Copy link

oferze commented Nov 16, 2022

Please add this method to our sample app, which returns a tiny string and you'll see the memory going up and not cleaning as well. Even on Azure (i.e. not in Visual Studio).

I'll download VS 7.14.1 but the issue seems unrelated as it happens on a release environment as well, and .NET 7 itself hasn't been updated.

@chunliu
Copy link

chunliu commented Nov 17, 2022

@oferze I did a memory usage profile with the sample code you provided. I also added the GetMem method you mentioned. Nothing seems abnormal to me.

The 5 snapshots were taken at the beginning of the app and after a request to one of the endpoints in your code. As you can see, the memory was reclaimed when GC was triggered (taking snapshot will trigger a full GC).

image

@oferze
Copy link

oferze commented Nov 22, 2022

@chunliu this is not the behaviour we had. Animated gif in the original issue. Let's leave it open for now and we'll see. I really hope it's a non-issue despite our experiment.

@ghost
Copy link

ghost commented Jan 21, 2023

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Jan 21, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 20, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants