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

Fix GC hole with collectible assemblies and tailcalls #51315

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

jakobbotsch
Copy link
Member

We must take special care to keep the tailcall dispatcher targets alive
while tailcalls are in-flight. In particular, given the following
callstack:

[B]M2()
[SPC]DispatchTailCalls
[A]M()

it could happen that [B]M2() queued a tail call to a function [B]M3().
Since there is a live dispatcher on the call stack, this would result in
[B]M2() storing a function pointer pointing to [B]M3() and returning to
this dispatcher to let it take care of the tailcall.

If B was loaded in a collectible ALC, it would then be possible for
there to be nothing keeping this ALC alive, and for the assembly to be
unloaded before the dispatcher invoked the function pointer.

I was unable to come up with a test case where this happened without
making changes to the dispatcher; the window otherwise seems to be too
small. To reproduce the problem I thus had to add a Thread.Sleep(50)
into the dispatcher, which quickly resulted in an
AccessViolationException in the scenario above. With the changes in this
commit I was then no logner able to reproduce the problem.

Fix #41314

We must take special care to keep the tailcall dispatcher targets alive
while tailcalls are in-flight. In particular, given the following
callstack:

[B]M2()
[SPC]DispatchTailCalls
[A]M()

it could happen that [B]M2() queued a tail call to a function [B]M3().
Since there is a live dispatcher on the call stack, this would result in
[B]M2() storing a function pointer pointing to [B]M3() and returning to
this dispatcher to let it take care of the tailcall.

If B was loaded in a collectible ALC, it would then be possible for
there to be nothing keeping this ALC alive, and for the assembly to be
unloaded before the dispatcher invoked the function pointer.

I was unable to come up with a test case where this happened without
making changes to the dispatcher; the window otherwise seems to be too
small. To reproduce the problem I thus had to add a Thread.Sleep(50)
into the dispatcher, which quickly resulted in an
AccessViolationException in the scenario above. With the changes in this
commit I was then no logner able to reproduce the problem.

Fix dotnet#41314
@jakobbotsch jakobbotsch marked this pull request as ready for review April 15, 2021 20:26
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.

Looks great. Thank you!

@jkotas jkotas merged commit 4573751 into dotnet:main Apr 16, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 17, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@jakobbotsch jakobbotsch deleted the fix-41314 branch February 11, 2023 12:11
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.

Handling of collectible types in tailcall dispatcher
3 participants