Skip to content

[mono] Preserve signal handlers during crash chaining#125835

Open
jpnurmi wants to merge 10 commits intodotnet:mainfrom
jpnurmi:fix/mono-crash-chaining
Open

[mono] Preserve signal handlers during crash chaining#125835
jpnurmi wants to merge 10 commits intodotnet:mainfrom
jpnurmi:fix/mono-crash-chaining

Conversation

@jpnurmi
Copy link
Copy Markdown

@jpnurmi jpnurmi commented Mar 20, 2026

This change is part of the effort to fix native crash reporting for .NET Android apps (getsentry/sentry-dotnet#3954). We wish to install Sentry's signal handlers before Mono to capture native crashes. However, mono_handle_native_crash unconditionally resets SIGABRT, SIGILL, SIGCHLD, and SIGQUIT to SIG_DFL, even when crash_chaining is enabled, killing the process before the chained handler can run.

Expose static remove_signal_handler as mono_runtime_posix_remove_signal_handler and use it in mono_handle_native_crash to restore pre-Mono signal handlers instead of resetting to SIG_DFL. When signal chaining is disabled, there are no saved handlers, so it falls back to SIG_DFL, same as before.

Includes an Android functional test (CrashChaining) that verifies signal handlers are preserved during crash chaining.

When mono_set_crash_chaining(true) is enabled, mono_handle_native_crash
unconditionally resets SIGABRT, SIGILL, SIGCHLD, and SIGQUIT to SIG_DFL.
This is problematic on Android where multiple threads may trigger signals
concurrently — for example, FORTIFY detecting a destroyed mutex on an
HWUI thread raises SIGABRT, but because the handler was reset to SIG_DFL,
it kills the process before Mono can chain the original crash to the
previous handler (e.g. sentry-native).

Guard the signal handler reset with !mono_do_crash_chaining so that
when crash chaining is enabled, Mono's handlers remain installed during
native crash processing. This allows the crash to be fully handled and
chained to any previously installed handlers before the process terminates.

The fix has no effect when crash_chaining is disabled (the default), as
the existing SIG_DFL reset behavior is preserved.

Includes an Android functional test (CrashChaining) that:
- Installs a SIGSEGV handler before mono_jit_init (gated by
  TEST_CRASH_CHAINING env var) to simulate a pre-Mono crash handler
- Triggers a native SIGSEGV that Mono chains to the pre-Mono handler
- Verifies SIGABRT was not reset to SIG_DFL during crash processing
- Without the fix, the test process is killed by SIGABRT (test crashes)
- With the fix, the test returns exit code 42 (pass)
Copilot AI review requested due to automatic review settings March 20, 2026 14:59
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 20, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Mono’s native-crash handling behavior when crash chaining is enabled so that Mono does not reset key signal handlers to SIG_DFL during mono_handle_native_crash, which can otherwise cause secondary signals (notably on Android) to terminate the process before crash chaining completes.

Changes:

  • Guard signal-handler resets in mono_handle_native_crash behind !mono_do_crash_chaining.
  • Add an Android functional test that installs a pre-Mono SIGSEGV handler, triggers a native crash, and validates SIGABRT was not reset to SIG_DFL.
  • Wire the test via the Android app template using an environment-variable gate (TEST_CRASH_CHAINING).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/mono/mono/mini/mini-exceptions.c Avoids resetting signal handlers to defaults when crash chaining is enabled.
src/tasks/AndroidAppBuilder/Templates/monodroid.c Adds a crash-chaining functional test path (env-gated) and native test helpers.
src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Program.cs Managed entrypoint invoking the native test and mapping success to exit code 42.
src/tests/FunctionalTests/Android/Device_Emulator/CrashChaining/Android.Device_Emulator.CrashChaining.Test.csproj New Android functional test project definition + env var injection.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 20, 2026 15:26
@jpnurmi jpnurmi changed the title [mono] Preserve signal handlers during crash chaining in mono_handle_native_crash [mono] Preserve signal handlers during crash chaining Mar 20, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

@jpnurmi
Copy link
Copy Markdown
Author

jpnurmi commented Mar 23, 2026

@dotnet-policy-service agree company="Sentry"

Copilot AI review requested due to automatic review settings March 23, 2026 16:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings March 26, 2026 14:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 26, 2026 15:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

@jpnurmi jpnurmi marked this pull request as ready for review March 26, 2026 20:37
@BrzVlad BrzVlad requested a review from lateralusX March 27, 2026 06:04
@jpnurmi
Copy link
Copy Markdown
Author

jpnurmi commented Mar 27, 2026

The fix proposal is simple, but the test gets quite complicated. Sorry about that. :) Let me know if you'd prefer to leave it out, even though then nothing guarantees that it stays fixed in the future...

// that secondary signals (e.g. SIGABRT from FORTIFY on other threads)
// don't kill the process with SIG_DFL before we can chain the original
// crash to the previous handler.
if (!mono_do_crash_chaining) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe it's better to restore the signal handlers instead of keeping the mono crash handlers active past this point. There is a function in mini-posix.c that removes our registered signal handlers and restore them with what existed before. It is currently a static method, but you could do a mono_runtime_remove_handlers that could remove the signals handled here and restore what was set before mono runtime installed its handlers.

#include <sys/system_properties.h>
#include <sys/mman.h>
#include <assert.h>
#include <setjmp.h>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not too happy to add test specific code into the Android App Builder. Maybe this can be handled differently? Maybe we could add some private property that the tests could use to add a custom source file that gets included in build of monodroid.c? Another option is to use the NativeLibrary item collection and make sure it gets linked into the app. This is what normally used with Native AOT to link custom static libraries together with an app, it is also used together with direct pinvoke. Mono Android App Builder do support direct pinvoke as well for AOT scenarios, but currently don't handle NativeLibrary items collection, so one option could be to piggyback on that. Your library could then use __attribute__((constructor)) to run the signal setup before main gets called and your test symbol should be picked up as well when linking that library.

jpnurmi added 2 commits March 27, 2026 20:55
Extract test-specific native code into a separate source file
(test_crash_chaining.c) compiled via the new ExtraNativeSources
property. Uses __attribute__((constructor)) to auto-install the
pre-mono SIGSEGV handler at .so load time.
Use mono_runtime_posix_remove_signal_handler to restore whatever
handlers were installed before Mono, rather than unconditionally
resetting to SIG_DFL. This correctly handles crash chaining without
needing a special case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Infrastructure-mono community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants