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

[release/6.0] Fix memory leak at AssemblyLoadContext unload #69334

Merged
merged 1 commit into from
May 16, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 13, 2022

Backport of #69263 to release/6.0

/cc @janvorli

Customer Impact

Without this fix, an application that creates and unloads collectible AssemblyLoadContext very often gets memory consumption growing indefinitely. So after many iterations, they may get out of memory exception. A customer has experienced this in a stress testing application they have created for their plugin system. The fix in 6.0 is very important for them.

Testing

CoreCLR tests, libraries tests, customer's repro application.
The customer has also tested a patched version of coreclr.dll on their systems and verified it fixes the issues they were seeing.

Risk

Low, it affects only collectible AssemblyLoadContext and the deletion of the data structure was clearly missing.

I have found there was a small memory leak when unloading
AssemblyLoadContext. A customer had a quite extreme testing case that
ran millions of iterations of creating an AssemblyLoadContext, loading
some assemblies into it and then unloading it. After 80 million
iterations on their machine with 8GB memory, the testing app was getting
out of memory.

Using PerfView, I was able to locate the source of the leak. It was a
missing destruction of the Module::m_pJitInlinerTrackingMap.
@janvorli janvorli self-assigned this May 16, 2022
@janvorli janvorli requested a review from jkotas May 16, 2022 12:01
@janvorli janvorli added this to the 6.0.x milestone May 16, 2022
@janvorli janvorli added the Servicing-consider Issue for next servicing release review label May 16, 2022
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Please ensure we get a code review. We will take for consideration in 6.0.x

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels May 16, 2022
@jeffschwMSFT jeffschwMSFT modified the milestones: 6.0.x, 6.0.6 May 16, 2022
@jeffschwMSFT
Copy link
Member

@carlossanlop this one is approved and ready to merge when the branch is ready.

@hoyosjs hoyosjs merged commit 6a2204c into release/6.0 May 16, 2022
@hoyosjs hoyosjs deleted the backport/pr-69263-to-release/6.0 branch May 16, 2022 18:19
@ghost ghost locked as resolved and limited conversation to collaborators Jun 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants