Skip to content

[mono] Restore signal handlers during crash chaining#125835

Open
jpnurmi wants to merge 1 commit intodotnet:mainfrom
jpnurmi:fix/mono-crash-chaining
Open

[mono] Restore signal handlers during crash chaining#125835
jpnurmi wants to merge 1 commit 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). Sentry would like to install 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_restore_handler and use it in mono_handle_native_crash to restore pre-Mono signal handlers instead of resetting to SIG_DFL. The saved handlers are kept in the hash table so that mono_chain_signal can still chain to them if needed. 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.

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.

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.

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.

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 added a commit to getsentry/sentry-native that referenced this pull request Mar 31, 2026
- Mono: chain-at-start (preload requires dotnet/runtime#125835)
- CoreCLR: preload (chain-at-start blocked by libsigchain)
- Auto-build the NDK AAR for preload tests
Copilot AI review requested due to automatic review settings March 31, 2026 11:21
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 10 out of 10 changed files in this pull request and generated 5 comments.

@jpnurmi jpnurmi changed the title [mono] Preserve signal handlers during crash chaining [mono] Restore signal handlers during crash chaining Mar 31, 2026
Copilot AI review requested due to automatic review settings March 31, 2026 11:59
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 10 out of 10 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings March 31, 2026 12:33
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 10 out of 10 changed files in this pull request and generated 1 comment.

jpnurmi added a commit to getsentry/sentry-native that referenced this pull request Mar 31, 2026
- Mono: chain-at-start (preload requires dotnet/runtime#125835)
- CoreCLR: preload (chain-at-start blocked by libsigchain)
- Auto-build the NDK AAR for preload tests
jpnurmi added a commit to getsentry/sentry-native that referenced this pull request Mar 31, 2026
- Mono: chain-at-start (preload requires dotnet/runtime#125835)
- CoreCLR: preload (chain-at-start blocked by libsigchain)
- Auto-build the NDK AAR for preload tests
Expose static remove_signal_handler as mono_runtime_posix_restore_handler
and use it in mono_handle_native_crash to restore pre-Mono signal handlers
instead of resetting to SIG_DFL. The saved handlers are kept in the hash
table so that mono_chain_signal can still chain to them if needed. 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 installs pre-Mono
SIGSEGV and SIGABRT handlers and verifies they are preserved during crash
chaining.
Copilot AI review requested due to automatic review settings April 8, 2026 16:45
@jpnurmi jpnurmi force-pushed the fix/mono-crash-chaining branch from 23c53f4 to d7d452d Compare April 8, 2026 16:45
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 10 out of 10 changed files in this pull request and generated 2 comments.

Copy link
Copy Markdown
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

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