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

Memory leak: Property caches are not getting cleaned up when using AssemblyParts. #21744

Open
mrukas opened this issue May 12, 2020 · 9 comments
Labels
affected-very-few This issue impacts very few customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. feature-mvc-application-model Features related to MVC application model, discovery, application parts investigate severity-major This label is used by an internal tool unloadability The ability to unload from collectable AssemblyLoadContext
Milestone

Comments

@mrukas
Copy link

mrukas commented May 12, 2020

Describe the bug

When you load an assembly in a new AssemblyLoadContext and add it dynamically via an AssemblyPart to the ApplicationPartManager new entries are getting added to the internal property caches. These entries do not get removed when the AssemblyPart is removed and the assembly is unloaded.

These are the caches that i am talking about:
PropertiesCache
VisiblePropertiesCache.

Because same types loaded in a different AssemblyLoadContext are not reference equal more and more entries are getting added.

It would be counterproductive to clear both dictionaries. At least the entries belonging to the unloaded assembly should be removed. Although i guess that the performance impact of clearing the whole cache wouldn't be high.

If you load and unload dlls multiple times you can clearly see the issue.

Without clearing the property caches:
Without cache clear

With clearing the property caches:
with cache clear

To Reproduce

  • Load an assembly in an collectible AssemblyLoadContext
  • Add the loaded assembly to the ApplicationParts list in the ApplicationPartManager
  • Trigger a change via a registered IActionDescriptorChangeProvider
  • Remove the previously added AssemblyPart from the ApplicationParts list
  • Trigger a change via a registered IActionDescriptorChangeProvider
  • Repeat above steps and watch memory consumption

I added a repo that can be used for investigation: Link
If line 111 in AssemblyController is commented you experience a memory leak. If you trigger this method multiple times you can see in the returned result, that for every load an entry gets added to the PropertiesCache. If you comment this line and call this method in a production build you can see a significant increase in memory consumption. I did not experience an increase of the number of entries in the VisiblePropertiesCache, but I'm pretty sure both caches should be cleaned up if they contain an entry for an unloaded assembly.

Further technical details

  • ASP.NET Core version: 3.1.1
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version: Microsoft Visual Studio Professional 2019 Preview
    Version 16.5.0 Preview 1.0

.NET Core SDK (gemäß "global.json"):
Version: 3.1.100
Commit: cd82f021f4

Laufzeitumgebung:
OS Name: Windows
OS Version: 10.0.18363
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support):
Version: 3.1.1
Commit: a1388f194c

.NET Core SDKs installed:
1.0.4 [C:\Program Files\dotnet\sdk]
1.1.0 [C:\Program Files\dotnet\sdk]
2.1.2 [C:\Program Files\dotnet\sdk]
2.1.4 [C:\Program Files\dotnet\sdk]
2.1.101 [C:\Program Files\dotnet\sdk]
2.1.104 [C:\Program Files\dotnet\sdk]
2.1.105 [C:\Program Files\dotnet\sdk]
2.1.201 [C:\Program Files\dotnet\sdk]
2.1.202 [C:\Program Files\dotnet\sdk]
2.1.300 [C:\Program Files\dotnet\sdk]
2.1.301 [C:\Program Files\dotnet\sdk]
2.1.302 [C:\Program Files\dotnet\sdk]
2.1.400 [C:\Program Files\dotnet\sdk]
2.1.402 [C:\Program Files\dotnet\sdk]
2.1.403 [C:\Program Files\dotnet\sdk]
2.1.504 [C:\Program Files\dotnet\sdk]
2.2.106 [C:\Program Files\dotnet\sdk]
2.2.202 [C:\Program Files\dotnet\sdk]
3.1.100-preview3-014645 [C:\Program Files\dotnet\sdk]
3.1.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.0-preview3.19555.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 1.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 1.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.0-preview3.19553.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.0-preview3.19553.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@mrukas mrukas changed the title Memory leak: Property caches in PropertyHelper are not getting cleaned up when using AssemblyParts. Memory leak: Property caches are not getting cleaned up when using AssemblyParts. May 12, 2020
@davidfowl
Copy link
Member

I’m sure there are more things that don’t get cleared when assemblies get unloaded. This isn’t something we’ve designed for so I would expect this to be the tip of the iceberg.

If you are willing to send PRs to solve the issues as you did them, that would be great. Otherwise, we’d need to look at this all up but, that’s super unlikely because it’s low priority

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 13, 2020
@mrukas
Copy link
Author

mrukas commented May 13, 2020

It could really be that there are more things that doesn't get cleaned up. But when analyzing the memory usage this seemed to be the most prominent error. I will try to further investigate the issue and try to provide a PR if I find the time.

@davidfowl
Copy link
Member

cc @janvorli

@mrukas
Copy link
Author

mrukas commented May 19, 2020

It appears that not only the property caches do net get cleared but also the cache of the ModelBinderFactory is growing with each loading and unloading.
Link

@pranavkm pranavkm added affected-very-few This issue impacts very few customers bug This issue describes a behavior which is not expected - a bug. severity-major This label is used by an internal tool labels Nov 6, 2020
@davidfowl davidfowl added the unloadability The ability to unload from collectable AssemblyLoadContext label Apr 12, 2021
@javiercn javiercn added the feature-mvc-application-model Features related to MVC application model, discovery, application parts label Apr 18, 2021
@pranavkm pranavkm added area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Oct 19, 2021
@brunolins16
Copy link
Member

brunolins16 commented Jan 11, 2022

I am looking at this issue and what if instead of clear cache we do not cache the type at all when it was loaded from a different AssemblyLoadContext, since the issue came from the assembly been loaded in a different AssemblyLoadContext instead of the default one?

@halter73
Copy link
Member

Disabling the cache entirely for types loaded from non-default AssemblyLoadContexts seems like it would hurt perf pretty badly for affected scenarios. Could we just clear caches when the IActionDescriptorChangeProvider fires?

@brunolins16
Copy link
Member

@halter73 the perf regression is something I was concerned but I am not able to figure out yet.

I have been trying to understand where not caching the controller (loaded in the non-default context) could cause issues, but I was able to find it been called during the initial app request or when the collection is changed and notified by the ChangeToken that seems reasonable for me. Do you happen to know where else it could be affected because I am probably missing something?

@gyankov
Copy link

gyankov commented Sep 21, 2022

Has anyone been able to find all caches that reference the assembly?

@ghost
Copy link

ghost commented Oct 11, 2022

Thanks for contacting us.
We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. 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 issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@brunolins16 brunolins16 removed their assignment Feb 24, 2023
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 20, 2023
@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-very-few This issue impacts very few customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. feature-mvc-application-model Features related to MVC application model, discovery, application parts investigate severity-major This label is used by an internal tool unloadability The ability to unload from collectable AssemblyLoadContext
Projects
None yet
Development

No branches or pull requests