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

[crossgen2] Fix memory leak in _corInfoImpls. #49764

Merged
merged 2 commits into from
Mar 19, 2021

Conversation

t-mustafin
Copy link
Contributor

This pull request fixes crossgen2 potentially memory leak.

ConditionalWeakTable _corInfoImpls elements keeps _compilation reference which keeps reference to whole table _corInfoImpls. The circular reference together with issue #12255 potentionally leads to memory leak in crossgen2 if that table is created more than once. As example _corInfoImpls table is created several times in pipeline mode #37411. WeakReference allows GC to collect the table and prevents the leak.

cc @jkotas @davidwrighton @MichalStrehovsky @gbalykov @alpencolt

@jkotas
Copy link
Member

jkotas commented Mar 17, 2021

I think it would be better to have a Dispose method on Compilation to break these cycles and allow things to be garbage collected.

_corInfoImpls elements keeps _compilation reference which keeps reference to whole table _corInfoImpls. The circular referene together with issue dotnet#12255 potentionally leads to memory leak in crossgen2 if that table is created more than once. Dispose() method clears the table and prevents memory leak.

Signed-off-by: Timur Mustafin <t.mustafin@partner.samsung.com>
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@davidwrighton davidwrighton merged commit fc80be1 into dotnet:main Mar 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 18, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@t-mustafin t-mustafin deleted the cg2_memory_leak branch August 10, 2023 14:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants