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

[mono][interp] Keep delegate alive during invocation #100832

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Apr 9, 2024

When invoking a delegate, we were overwritting the stack slot containing the delegate object reference. In the case of invoking a delegate for a dynamic method, we were running into issues when the delegate object is collected while the method is executed because the method code is also discarded.

When invoking a delegate, we were overwritting the stack slot containing the delegate object reference. In the case of invoking a delegate for a dynamic method, we were running into issues when the delegate object is collected while the method is executed because the method code is also discarded.
Copy link
Contributor

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

@BrzVlad
Copy link
Member Author

BrzVlad commented Apr 9, 2024

#100800
#100683
Maybe #100753
Maybe #100757

@kg
Copy link
Contributor

kg commented Apr 9, 2024

Is it bad that for long-running method bodies, we never null out the ref_slot_offset after a call? It potentially means we leak a delegate instance for a while, but I can't imagine a realistic scenario where that would cause a program to meaningfully degrade just from one delegate surviving GC during a loop.

Could break synthetic GC tests maybe.

@@ -4164,6 +4164,9 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
}
cmethod = del_imethod;
if (!is_multicast) {
Copy link
Contributor

Choose a reason for hiding this comment

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

does multicast not need this deathgrip var because multicast invokes are done by a helper method that has 'this' on the stack?

@BrzVlad BrzVlad merged commit d88c7ba into dotnet:main Apr 10, 2024
76 of 79 checks passed
matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
When invoking a delegate, we were overwritting the stack slot containing the delegate object reference. In the case of invoking a delegate for a dynamic method, we were running into issues when the delegate object is collected while the method is executed because the method code is also discarded.
@github-actions github-actions bot locked and limited conversation to collaborators May 10, 2024
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.

None yet

2 participants