Skip to content

Conversation

@omajid
Copy link
Member

@omajid omajid commented Mar 25, 2021

It seems like we are allocating memory, but never releasing it.

Allocate pEmitter closer to where it's used, skipping a FailIfThrow as well.

@ghost
Copy link

ghost commented Mar 25, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Copy link
Member

Choose a reason for hiding this comment

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

The VM convention is to use NewHolder<ProfileEmitter> for cleanup in situations like this one. Could you please switch to that?

@omajid omajid force-pushed the ceeload-pewriter-leak branch from 2e78b90 to 2b20b4c Compare March 25, 2021 15:57
It seems like we are allocating memory, but never releasing it.
@omajid omajid force-pushed the ceeload-pewriter-leak branch from 2b20b4c to f97e6c5 Compare March 25, 2021 15:58
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.

Thanks

@jkotas jkotas merged commit 93b8e18 into dotnet:main Mar 29, 2021
clamp03 added a commit to clamp03/runtime that referenced this pull request Mar 31, 2021
jkotas pushed a commit that referenced this pull request Mar 31, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 28, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
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