Skip to content

Memory leak in RazorView and ViewBuffer bundle #38550

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

Closed
sergei66666 opened this issue Nov 21, 2021 · 4 comments
Closed

Memory leak in RazorView and ViewBuffer bundle #38550

sergei66666 opened this issue Nov 21, 2021 · 4 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved

Comments

@sergei66666
Copy link

Describe the bug

Hello, dear developers.
I am using the standard RazorViewToStringRenderer implementation to create emails.

Faced a memory leak when calling RenderViewToStringAsync on the same instance of RazorViewToStringRenderer many times. For example, I call 500,000 times and it leaks approximately 200 MB of memory (leakage rate depends on the size of the layout).

I get the RazorViewToStringRenderer instance by creating Scope. After the scope is freed, the memory is released gracefully.

I understand that this is not a standard usage - on the server side there is only one rendering at a time and then the Scope is destroyed.

But maybe it's worth fixing this leak?

I did a little research, localized the problem area, made changes, and then ran my code - there is no more memory leak.
Tests for Microsoft.AspNetCore.Mvc.Razor.Test and Microsoft.AspNetCore.Mvc.ViewFeatures.Test worked correctly.

My changes are located here - sergei66666@c5712c5

Do you think it is worth making a pull request or is it not a critical issue in your opinion?

To Reproduce

Here is small project to reproduce problem: https://github.com/sergei66666/net_mvc_razor_mem_leak/
Without fix in console it will show (all values in MB)

Mem0: 127,16015625
Mem1: 312,8046875

If apply fix (in Libs folder there are two dll with changes, uncomment References in Mvc.RenderViewToString.csproj) it will show

Mem0: 126,984375
Mem1: 113,6640625

Further technical details

  • ASP.NET Core version: 6.0.0
  • The IDE: Microsoft Visual Studio Community 2022 (64-bit) Version 17.0.1
  • Include the output of dotnet --info:
dotnet --info Output
Пакет SDK для .NET (отражающий любой global.json):
 Version:   6.0.100
 Commit:    9e8b04bbff

Среда выполнения:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.100\

Host (useful for support):
  Version: 6.0.0
  Commit:  4822e3c3aa

.NET SDKs installed:
  2.1.4 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.2.401 [C:\Program Files\dotnet\sdk]
  5.0.403 [C:\Program Files\dotnet\sdk]
  6.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.21 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.21 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.21 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
@javiercn javiercn added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views labels Nov 22, 2021
@javiercn
Copy link
Member

@sergei66666 thanks for contacting us.

I don't think that RazorViewToStringRenderer is anything we officially support (I believe its a sample), so its not clear that the issue is in RazorView and ViewBuffer and not on the use RazorViewToStringRenderer makes of them.

For us to consider something a memory leak there needs to be an out of memory exception that can be triggered or clear evidence that the objects are being rooted after a full Gen 2 GC collection. Otherwise, its simply the behavior of the GC not running while it doesn't need to.

I'm not confident that the fix proposed is correct, since I believe we manage the buffers outside of those types.

Finally, I would suggest that the plan to render code outside of the HttpContext is something we track separately here, so we would recommend you upvote that issue instead.

Note that we plan to do this based on Components and not MVC since they are much more lightweight

@pranavkm
Copy link
Contributor

But maybe it's worth fixing this leak?

Great job digging in to this. While we think it's an interesting problem, given that it doesn't affect the default code path, we don't think we would consider changing the runtime for this.

@pranavkm pranavkm added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Nov 22, 2021
@ghost ghost added the Status: Resolved label Nov 22, 2021
@sergei66666
Copy link
Author

sergei66666 commented Nov 23, 2021

Thanks for the quick answers. I want to say that I disagree with you. I will try to explain why.

@javiercn, I disagree with you on the issue of memory leak. If the duration of the Scope execution is long enough,
then there will be an OutOfMemoryException.

Ok, let's assume you don't support RazorViewToStringRenderer. Let me show you on the example of a server.
The screenshot shows the end of the RazorView.RenderAsync function. It seems to me, by the end of the function, all the values that were leased from the pool must be returned (_available Count should equals to _leased Count). However, this does not happen.
Mem_leak_1

I think this is happening because the buffer is requesting memory from the pool, but it does not know when to free it.
I'm talking about calling await bodyWriter.Buffer.WriteToAsync (writer, _htmlEncoder); at the end of the RazorView.RenderLayoutAsync function
WriteToAsync does not return memory to the pool, but simply copies the content to another location.
And then the reference to the bodyWriter is lost (overwritten with a different value).

And, perhaps, if all references to all created buffers were kept, then they could be cleared when calling Dispose ..
But no links are stored AND RazorView does not implement IDisposable. Although, judging by the screenshot, this was implied
Mem_leak_2

Namely therefore, I insist that this is a leak. Yes, it disappears when the Scope is destroyed, but inside the Scope the memory is leaked.

@pranavkm Yes, now it does not affect the default code path .. But if it doesn’t affect it now, it doesn’t mean that it will not affect it in the future. Considering how difficult work with buffers inside RazorView.

By the way, if I start the server with the "patched" libraries, then everything goes back to the pool.
Mem_leak_3

ps I never claim that my fix is correct. Perhaps you see it differently. I just want to show that in the current implementation, memory is not returned to the pool at the last stage.

@ghost
Copy link

ghost commented Nov 24, 2021

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Nov 24, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

3 participants