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.

Sign up for GitHub

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

Jump to bottom

[mono][interp] Fix memory leaks for interpreted dynamic methods. #88892

Merged
merged 2 commits into from
Jul 19, 2023
Merged

[mono][interp] Fix memory leaks for interpreted dynamic methods. #88892

merged 2 commits into from
Jul 19, 2023

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Jul 14, 2023

  • Add a mempool to MonoDynamicMethod.
  • Modify the intepreter code to allocate from this mempool when interpreting dynamic methods.

@ghost ghost assigned vargaz Jul 14, 2023
@ghost
Copy link

ghost commented Jul 14, 2023

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Add a mempool to MonoDynamicMethod.
  • Modify the intepreter code to allocate from this mempool when interpreting dynamic methods.
Author: vargaz
Assignees: -
Labels:

area-Codegen-Interpreter-mono
Milestone: -

@vargaz
Copy link
Contributor Author

vargaz commented Jul 14, 2023

Ref: #87078

@vargaz
Copy link
Contributor Author

vargaz commented Jul 14, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz vargaz force-pushed the interp-memory branch 3 times, most recently from 136c83d to 066690c Compare July 14, 2023 05:03
@vargaz
Copy link
Contributor Author

vargaz commented Jul 14, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Jul 14, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz vargaz force-pushed the interp-memory branch 3 times, most recently from 14d5cd5 to 968f4c2 Compare July 14, 2023 08:31
This was referenced Jul 14, 2023
Closed
Closed
Open
Closed
Closed
@vargaz vargaz force-pushed the interp-memory branch 2 times, most recently from 97702bb to c5b7223 Compare July 14, 2023 20:29
@vargaz vargaz marked this pull request as draft July 14, 2023 20:29
@build-analysis build-analysis bot mentioned this pull request Jul 14, 2023
Closed
@vargaz vargaz force-pushed the interp-memory branch 7 times, most recently from b64f58f to df47797 Compare July 16, 2023 18:08
@vargaz
[mono] Set the 'dynamic' flag on method builders on creation so a Mon…
b7a4ea2 
…oDynamicMethod is allocated instead of a MonoMethodWrapper.
@vargaz
[mono][interp] Fix memory leaks for interpreted dynamic methods.
cf435b3 
* Add a mempool to MonoDynamicMethod.
* Modify the intepreter code to allocate from this mempool when
  interpreting dynamic methods.
@vargaz vargaz marked this pull request as ready for review July 18, 2023 00:04
@build-analysis build-analysis bot mentioned this pull request Jul 18, 2023
Closed
@vargaz vargaz merged commit fce5227 into dotnet:main Jul 19, 2023
@vargaz vargaz deleted the interp-memory branch July 19, 2023 05:41
@BrzVlad
Copy link
Member

BrzVlad commented Jul 19, 2023 •
edited
Loading

I suspect this might have caused crashes like

Assertion at /home/vbrezae/Xamarin/repos/runtime/src/mono/mono/mini/interp/transform.c:8244, condition `td->new_code [reloc->offset + reloc->skip + 1] == 0xdead'

I am able to reproduce with moderate difficulty. I'm taking a look into it.

@BrzVlad BrzVlad mentioned this pull request Jul 20, 2023
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kg kg kg left review comments

@BrzVlad BrzVlad BrzVlad approved these changes

@kotlarmilos kotlarmilos Awaiting requested review from kotlarmilos kotlarmilos is a code owner

@lambdageek lambdageek Awaiting requested review from lambdageek

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

Assignees

@vargaz vargaz

Labels
area-Codegen-Interpreter-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vargaz @BrzVlad @kg