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

BlazorServer doesn't release circuits/component instances after 25mins+forced GC2 #44062

Closed
1 task done
chrdlx opened this issue Sep 19, 2022 · 7 comments
Closed
1 task done
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed feature-blazor-server
Milestone

Comments

@chrdlx
Copy link

chrdlx commented Sep 19, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Blazor component instances and circuit objects/instances don't seem to be released from memory after 25+ minutes and forced 2nd generation garbage collection using the basic out of the box Blazor Server template. I tried to follow the recommendations from @javiercn in this post #30210 (comment) to test this.

Expected Behavior

The amount of Counter instances & Circuits should go to zero after 10/20/30 minutes and a forced garbage collection ( GC.Collect(2, GCCollectionMode.Forced);).

Steps To Reproduce

  1. Create a default Blazor Server template app in Visual Studio 17.4.0 Preview 2.
  2. Go to the Counter.razor component and change the method to

private void IncrementCount() { GC.Collect(2, GCCollectionMode.Forced); }

as recommended by @javiercn here #30210 (comment) so we can force a GC collect after 20 minutes.

  1. Set the build mode to *Release
  2. Execute the application
  3. Click on the Counter tab button so we navigate to the counter page.
  4. Now on the counter page, press F5 (refresh page) 10 times.
  5. Open the Diagnostics Tool in VS and take a memory snapshot.
  6. Search in the snapshot for *Counter and *Circuits.circuit

image
and
image

So you can see there're 11 instances (1 initial request + 10 refreshes) of the component as expected in memory, and the related circuits.

  1. Wait 10 minutes.

  2. Take a new snapshot.
    image
    image
    Component instances & circuits still there after 10 minutes.

  3. Wait another 10 minutes (just in case)

  4. Take a new snapshot

image
image

  1. So we waited 20 minutes (the double recommended by javier), all the instances are still there... So now let's trigger a forced GC collect using the Counter button for which we changed the code:

image

We can see a GC has occured.

  1. Take another snapshot to see if after 25+ minutes and a forced GC, Counter components & circuit instances have been released.
    image

image

So nothing has changed, 11 instances are still there after almost an hour, and a forced GC collection.

Just in case I forced GC a couple more times.
image

Still nothing is released.

Exceptions (if any)

No response

.NET Version

7.0.100-rc.1.22431.12

Anything else?

What I've tried to see if this gets fixed:

 builder.Services.AddServerSideBlazor(options => {
                options.DisconnectedCircuitMaxRetained = 0;
                options.DisconnectedCircuitRetentionPeriod = TimeSpan.FromSeconds(0);
            });
  1. Changing the GC mode to Workstation.

  2. Debug & Release modes.

  3. Closed the browser completely (but left the server running).

Nothing seems to make .NET to release Counter component instances & circuits after half an hour.

@mkArtakMSFT mkArtakMSFT added area-blazor Includes: Blazor, Razor Components feature-blazor-server labels Sep 19, 2022
@mkArtakMSFT mkArtakMSFT added this to the 7.0.0 milestone Sep 20, 2022
@chrdlx
Copy link
Author

chrdlx commented Sep 21, 2022

This seems to happen only with razor components that have events in them.
For example, in the default template this only happens with the Counter component, not Index nor FetchData components.
As soon as you add an event like @onclick on them, their instances start getting stuck in memory forever it seems, like the Counter component did.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Oct 3, 2022

@chrdlx I've investigated, and haven't seen any evidence that there's a memory leak or that circuit/component instances fail to get released.

The way memory management works inside .NET is more subtle than your repro allows for. For example, you only build up 10 circuits. If it was a leak, you would be able to build up an unlimited number until the process crashes with an out-of-memory error. When I tried following your steps, I do see that at 10 circuits they remain in memory, whereas if I keep refreshing (say, to 100 circuits) then they do get evicted.

As an example of the sort of thing that complicates the analysis, one of the things that was rooting the circuit instances when there were only 10 of them was this ConcurrentDictionary<JsonSerializerOptions, WeakReference<CachingContext>> used internally by System.Text.Json. The cache keys - JsonSerializerOptions - contain a chain of references that leads to the Circuit instances which in turn hold whatever components in their final render trees. But as you can see in the logic from TryEvictDanglingEntries, it does eventually remove orphaned cache entries, but in a way that varies dynamically depending on the frequency of evictions to amortise the cost of doing so.

So in general it doesn't work to do an analysis of the form "do a small amount of things, see that objects are still in memory". A leak is only present if you can build up an unlimited amount of memory pressure until the process eventually fails with an OutOfMemoryException.

It's a tricky area and easy to misread what's going on. If you are convinced there is a leak, could you please provide more information about how to consume an unlimited amount of memory? Thanks!

@SteveSandersonMS SteveSandersonMS added Done This issue has been fixed and removed investigate labels Oct 3, 2022
@chrdlx
Copy link
Author

chrdlx commented Oct 3, 2022

Yes @SteveSandersonMS , thanks!! I understand what you say, that's why I didn't use the word "leak" specifically, I report it as a bug since that's the only option to report things as far as I can see, I apologize if this wasn't the correct issue category.

So, there may not be a problem with this simple repro, the real issue are the implications of this behavior.

The implications are that all the class fields your component has will persist in RAM until the internal workings of the framework (like when json serializer) decide it's time to release, and on top of that when GC decides to finally release from memory those references.

So if you don't have much data in your component class (like this repro) it's fine, but if you have several hundreds of MBs (like my app), it becomes a problem. This also maybe not problematic in a server situation where you may let .NET Core take all the RAM possible, but not in a workstation scenario like mine.

In my case, it's an Blazor server app that runs locally on some client machines (because the app needs access to local files and Windows OS processes), so letting it grow to consume all available RAM until there's no more left to start releasing memory is not acceptable in my scenario.
Basically users complain that there's a .exe process consuming GBs of RAM while doing nothing and rendering their computers slow.

So what did I end up doing then? I set all the component's class fields to null manually in the component's Dispose method, and then force a GC Collect.

@chrdlx
Copy link
Author

chrdlx commented Oct 3, 2022

Also, people may find your explanation useful Steve, because if you read this from @javiercn here
#30210 (comment)

I suggest you capture two memory dumps. One at the beginning and one 10 minutes after you've closed your last tab and after triggering a forced generation 2 garbage collection with GC.Collect(2, Forced) and you'll observe that the memory is roughly the same as in the beginning of your experiment.

This is what I did and what Javier says it will happen, but it didn't. The reason is your explanation Steve.
So people has to take into account that even though they can:

  1. Close the browser completely.
  2. Wait 10, 20, 30 minutes or 10 hours.
  3. Force GC.Collect in any generation
  4. Set GC to workstation

Memory may be or may not be released based on further layers of framework logic (json seralizer cache/others/etc)

@willdean
Copy link

willdean commented Oct 3, 2022

Is this behaviour which has arisen in 7.0 because of new STJ caching JsonSerializerOptions? It does seem a bit unexpected that serializer options would become a root for potentially large/numerous application data structures.

Whoever carefully used WeakReference<> on that cache to prevent it blocking GC must feel particularly galled that someone's already found a way to trap huge amounts of data via the cache key... :-)

@SteveSandersonMS
Copy link
Member

Thanks for the additional context. I've filed dotnet/runtime#76548.

@chrdlx
Copy link
Author

chrdlx commented Oct 3, 2022

You're welcome! Blazor is great!

@ghost ghost locked as resolved and limited conversation to collaborators Nov 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed feature-blazor-server
Projects
None yet
Development

No branches or pull requests

6 participants
@SteveSandersonMS @willdean @danroth27 @chrdlx @mkArtakMSFT and others