[Mono]: Make sure interpreter gc_transitions gets reset on managed exceptions. #115052
+48
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ced. Interpreters do_icall_wrapper and ves_pinvoke_method uses a frame variable called gc_transitions that can put thread in GC SAFE mode before calling native code and switch back on return. When interpreter calls native code it also push a LMF frame with context and unwind label so in case of managed exception it will continue execution at label, but in that case it would also miss to reset the gc_transitions flag. That could then lead to future errors since other icalls would run under incorrect GC state due to incorrect GC switch. This issue was observed in dotnet#114840. In that case a custom icall could call mono_raise_exception that would hit this scenario, leaving the gc_transitions set to TRUE that would then cause issues since next icall would run in wrong GC mode causing undefined behaviour. Fix adds a mechanism to abort a GC safe region making sure thread gets back to GC unsafe mode and also make sure the cookie stack gets rebalanced on checked builds. It also makes sure gc_transitions gets reset to FALSE, if its still TRUE reaching the exit label. Fix makes sure we correctly restore the interpreter GC state in case of a managed exception in native code.
lateralusX
requested review from
steveisok,
vitek-karas,
BrzVlad and
kotlarmilos
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an imbalance in the GC safe/unsafe transitions when a managed exception occurs during interpreter calls. The changes add a new function to abort a GC safe region and adjust the behavior of both the p/invoke and icall wrappers.
- Introduces mono_threads_abort_gc_safe_region_internal to rebalance the GC state.
- Modifies ves_pinvoke_method and do_icall_wrapper to use stack‐data based GC safe region entry/exit and handle managed exceptions.
- Updates header and implementation files accordingly.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/mono/mono/utils/mono-threads-coop.h | Added declaration for aborting GC safe regions. |
| src/mono/mono/utils/mono-threads-coop.c | Introduced mono_threads_abort_gc_safe_region_internal to rebalance GC state. |
| src/mono/mono/mini/interp/interp.c | Updated wrappers to ensure balanced GC transitions on managed exceptions. |
lateralusX
changed the title
BrzVlad
approved these changes
Apr 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Interpreters
do_icall_wrapperandves_pinvoke_methoduses a frame variable calledgc_transitionsthat can put thread in GC SAFE mode before calling native code and switch back on return. When interpreter calls native code it also push a LMF frame with context and unwind label so in case of managed exception it will continue execution at label, but in that case it would also miss to reset thegc_transitionsflag. That could then lead to future errors since other icalls would run under incorrect GC state due to incorrect GC switch.This issue was observed in #114840. In that case a custom icall could call
mono_raise_exceptionthat would hit this scenario, leaving thegc_transitionsset to TRUE that would then cause issues since next icall would run in wrong GC mode causing undefined behavior.Fix adds a mechanism to abort a GC safe region making sure thread gets back to GC unsafe mode and also make sure the cookie stack gets rebalanced on checked builds. It also makes sure
gc_transitionsgets reset toFALSE, if its stillTRUEreaching the exit label.Fix makes sure we correctly restore the interpreter GC state in case of a managed exception in native code.