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

[mini] Enter GC Unsafe mode in handle_signal_exception #88436

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Jul 5, 2023

When the runtime needs to turn some kinds of signals into managed exceptions (for example: SIGINT turns into
new ExecutionEngineException ("Interrupted (SIGINT)") if MONO_DEBUG=handle-siginit is set, and some SIGFPE turn into DivideByZeroException, and some SIGSEGV turn into a NullReferenceException) instead of unwinding the stack from inside a signal handler it instead adjusts the normal stack so that when the signal handler returns, execution will resume in
handle_signal_exception.

That means that if the runtime was in GC Safe mode when the signal was raised, even if the signal handler code transitions to GC Unsafe mode, by the time the handle_signal_exception runs, we will have undone the GC Unsafe transition and will be back in GC Safe.

That means if the code in handle_signal_exception (notably mono_handle_exception) calls anything that tries to do a transition to GC Safe, we may get an assertion.

Fixes #88405

When the runtime needs to turn some kinds of signals into managed
exceptions (for example: SIGINT turns into
`new ExecutionEngineException ("Interrupted (SIGINT)")`, and some
SIGFPE turn into `DivideByZeroException`, and some SIGSEGV turn into a
`NullReferenceException`) instead of unwinding the stack from inside a
signal handler it instead adjusts the normal stack so that when the
signal handler returns, execution will resume in
`handle_signal_exception`.

That means that if the runtime was in GC Safe mode when the signal
was raised, even if the signal handler code transitions to GC Unsafe
mode, by the time the `handle_signal_exception` runs, we will have
undone the GC Unsafe transition and will be back in GC Safe.

That means if the code in `handle_signal_exception` (notably
`mono_handle_exception`) calls anything that tries to do a transition
to GC Safe, we may get an assertion.

Fixes dotnet#88405
@lambdageek
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member Author

lambdageek commented Jul 5, 2023

The thing is, for this to matter we have to be in a method that the runtime has JitInfo for (both SIGFPE and SIGSEGV handlers only call mono_arch_handle_exception if they can find JitInfo for the method) but we were in GC Safe mode and the method hasn't yet done a transition from GC Safe to GC Unsafe before it got to a signal.

So that means we're in a wrapper and something went wrong before we called mono_threads_attach_coop - there is not a lot of code there

I'm not sure it's actually good to try and throw a managed exception at this point. Maybe it's better to just assert that we're un GC Unsafe mode. If we're not, something bad is going on.

@lambdageek lambdageek closed this Jul 17, 2023
@lambdageek lambdageek reopened this Jul 17, 2023
@lambdageek
Copy link
Member Author

/azp run runtime

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 88436 in repo dotnet/runtime

@lambdageek
Copy link
Member Author

/azp run runtime-ioslike

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek lambdageek merged commit a654a77 into dotnet:main Jul 18, 2023
95 of 98 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants