fix: AOT interop with managed .NET runtimes#6193
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6193 +/- ##
=============================================
- Coverage 85.455% 85.419% -0.037%
=============================================
Files 487 487
Lines 29166 29183 +17
Branches 12612 12643 +31
=============================================
+ Hits 24924 24928 +4
- Misses 4192 4204 +12
- Partials 50 51 +1
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
This reverts "Use pre-built version of sentry-cocoa SDK (#3727)" commit d179ec9 and restores the modules/sentry-cocoa Git module checked out at: getsentry/sentry-cocoa#6193
2f90356 to
ed98b04
Compare
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 4d1f3fb | 1217.77 ms | 1253.84 ms | 36.08 ms |
| f84c826 | 1216.38 ms | 1241.98 ms | 25.60 ms |
| ee272e8 | 1210.98 ms | 1245.10 ms | 34.12 ms |
| 2c4362a | 1231.50 ms | 1255.95 ms | 24.45 ms |
| 5b1e2a1 | 1217.63 ms | 1245.49 ms | 27.86 ms |
| bffd360 | 1218.16 ms | 1249.86 ms | 31.70 ms |
| 3060db6 | 1229.84 ms | 1257.53 ms | 27.70 ms |
| 5d67f5d | 1225.33 ms | 1262.76 ms | 37.43 ms |
| e701dc8 | 1215.89 ms | 1254.06 ms | 38.17 ms |
| a8ec727 | 1225.29 ms | 1259.59 ms | 34.30 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 4d1f3fb | 24.14 KiB | 1.13 MiB | 1.10 MiB |
| f84c826 | 24.14 KiB | 1.04 MiB | 1.02 MiB |
| ee272e8 | 24.14 KiB | 1.10 MiB | 1.08 MiB |
| 2c4362a | 24.14 KiB | 1.07 MiB | 1.04 MiB |
| 5b1e2a1 | 24.14 KiB | 1.11 MiB | 1.09 MiB |
| bffd360 | 24.14 KiB | 1.11 MiB | 1.09 MiB |
| 3060db6 | 24.14 KiB | 1.12 MiB | 1.10 MiB |
| 5d67f5d | 24.14 KiB | 1.06 MiB | 1.04 MiB |
| e701dc8 | 24.14 KiB | 1.06 MiB | 1.04 MiB |
| a8ec727 | 24.14 KiB | 1.12 MiB | 1.10 MiB |
Previous results on branch: jpnurmi/mono-interop
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| efedb0c | 1218.20 ms | 1256.60 ms | 38.40 ms |
| 4a46856 | 1236.53 ms | 1262.79 ms | 26.26 ms |
| 58546af | 1217.57 ms | 1248.98 ms | 31.40 ms |
| 3a8f309 | 1223.75 ms | 1245.77 ms | 22.02 ms |
| 2488cd0 | 1215.08 ms | 1241.24 ms | 26.16 ms |
| 446c562 | 1227.20 ms | 1256.17 ms | 28.97 ms |
| 444f214 | 1223.69 ms | 1249.73 ms | 26.04 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| efedb0c | 24.14 KiB | 1.13 MiB | 1.10 MiB |
| 4a46856 | 24.14 KiB | 1.13 MiB | 1.10 MiB |
| 58546af | 24.14 KiB | 1.13 MiB | 1.11 MiB |
| 3a8f309 | 24.14 KiB | 1.13 MiB | 1.10 MiB |
| 2488cd0 | 24.14 KiB | 1.13 MiB | 1.11 MiB |
| 446c562 | 24.14 KiB | 1.13 MiB | 1.10 MiB |
| 444f214 | 24.14 KiB | 1.13 MiB | 1.11 MiB |
Screen.Recording.2025-09-18.at.11.54.52.mov |
ed98b04 to
8559ea9
Compare
db30de0 to
dd33f14
Compare
dd33f14 to
e51c82e
Compare
|
Hey @jpnurmi what's the progress on this PR? |
|
I had quite some trouble passing the CI last time I tried. 😅 I should totally revive this because Sentry .NET is still very much in need of a solution for eliminating redundant |
be528ee to
e1a14ae
Compare
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Deps
🤖 This preview updates automatically when you update the PR. |
e1a14ae to
bab06c0
Compare
Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_Signal.c
Outdated
Show resolved
Hide resolved
When the signal monitor is disabled under managed runtime, restore the previous handler for the signal before re-raising to prevent an infinite loop. Without SA_NODEFER, the raised signal would be re-delivered to the same handler after it returns.
The flag is redundant now that handleSignal() restores individual handlers before re-raising. uninstallSignalHandler() can be a blanket no-op under SENTRY_CRASH_MANAGED_RUNTIME.
|
@supervacuus thanks for the view! 🙏 I've addressed the close/reinstall scenario. |
|
By the way, there are integration tests prepared in sentry-dotnet. This patch fixes both scenarios with redundant native exceptions:
With this PR and expected failures removed: $ pwsh integration-test/ios.Tests.ps1
[...]
[+] /Users/jpnurmi/Projects/sentry/sentry-dotnet/integration-test/ios.Tests.ps1 180.08s (178.93s|113ms)
Tests completed in 180.08s
Tests Passed: 6, Failed: 0, Skipped: 0, Inconclusive: 0, NotRun: 0 |
|
Hi @philipphofmann @philprime @noahsmartin @itaybre, this PR is ready for review when you get a chance. It adds managed runtime (.NET/Mono) signal handler interop, solving a long-standing issue for .NET on iOS (getsentry/sentry-dotnet#3954). Most of it is gated behind |
|
Thanks for the review! Sounds good, I'll take a look tomorrow. P.S. The .NET ecosystem likes to own the term "managed" but technically, many other runtimes are managed too. I might rename it to |
I would prefer to avoid DOTNET in the name, just in case some other SDK needs this in the future. |
|
Fair enough, I kept |
Reset the flag on any signal delivery, not just the matching one. Prevents a stale flag from silently suppressing a later unrelated signal.
Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_Signal.c
Outdated
Show resolved
Hide resolved
Instead of returning early from the signal handler when a signal is ignored, skip crash processing and fall through to the existing restore + raise path. This avoids undefined behavior when the ignored signal originates from abort().
Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_Signal.c
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_Signal.c
Outdated
Show resolved
Hide resolved
Remove the SENTRY_CRASH_MANAGED_RUNTIME guard so ignored signals are properly re-raised regardless of the compile flag. Harmless for the non-managed case where uninstall already restored them.

📜 Description
TLDR; reorder .NET/Mono vs. Sentry Cocoa signal handlers to allow the .NET runtime to convert certain signals to managed .NET exceptions where appropriate, and chain actual native crashes to Sentry Cocoa.
Before
After:
Changes
Mostly gated by the
SENTRY_CRASH_MANAGED_RUNTIMEcompile-time flag to minimize impact on normal SDK operation:Signal handler preloading — A
__attribute__((constructor))preloads SentryCrash's signal handlers before the managed runtime starts, ensuring the correct handler chain order: managed runtime → SentryCrash → system. This lets the managed runtime handle signals like SIGSEGV (for NullReferenceException) and SIGFPE (for DivideByZeroException) before SentryCrash sees them.Mach exception mask filtering — Excludes
EXC_MASK_BAD_ACCESSandEXC_MASK_ARITHMETICfrom Mach exception monitoring, since these correspond to signals the managed runtime handles. Other Mach exceptions (EXC_BAD_INSTRUCTION, EXC_SOFTWARE, EXC_BREAKPOINT) are still monitored.ignoreNextSignal:API — New method onPrivateSentrySDKOnlythat tells SentryCrash to ignore the next occurrence of a given signal on the calling thread (thread-local, one-shot). Used by hybrid SDKs to prevent duplicate crash reports when the managed runtime is about to raise a signal (e.g. SIGABRT viaabort()) for an exception that has already been captured: https://github.com/dotnet/macios/blob/d0d53e8230a79fd505ddf7cef2642493249ccb78/runtime/runtime.m#L1003-L1011NULL-guard
g_onExceptionEvent— Prevents a crash if a signal fires between the constructor preload andsentrycrash_install().volatilecrash pointer — The+[SentrySDK crash]test method now usesvolatileto force an actual SIGSEGV instead of letting the compiler optimize the null dereference into a trap instruction (SIGTRAP).💡 Motivation and Context
Fixes redundant native crash events when using Sentry with .NET on iOS:
ApplicationException) produced a duplicate SIGABRT eventNullReferenceExceptionin AOT mode produced a duplicate EXC_BAD_ACCESS eventSee: getsentry/sentry-dotnet#3954
💚 How did you test it?
Sentry .NET integration tests for iOS:
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.