diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c79f5b03..fc96b7c97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Windows: fix HTTP rate limit response header parsing. ([#1732](https://github.com/getsentry/sentry-native/pull/1732)) - POSIX: prevent condition-variable timeout overflow from busy-spinning flush and shutdown waits. ([#1731](https://github.com/getsentry/sentry-native/pull/1731)) - Native/macOS: fix thread stack descriptor. ([#1726](https://github.com/getsentry/sentry-native/pull/1726)) +- Native/macOS: honor the `system_crash_reporter_enabled` option. ([#1743](https://github.com/getsentry/sentry-native/pull/1743)) - Cap rate-limit retry-after values at 24 hours to prevent a MITM-provided response from disabling event delivery for the process lifetime. ([#1744](https://github.com/getsentry/sentry-native/pull/1744)) - Native: validate ELF header entry sizes. ([#1746](https://github.com/getsentry/sentry-native/pull/1746)) diff --git a/include/sentry.h b/include/sentry.h index dc68aab2a..45009667d 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1836,10 +1836,10 @@ SENTRY_EXPERIMENTAL_API int sentry_set_thread_stack_guarantee( /** * Enables forwarding to the system crash reporter. Disabled by default. * - * This setting only has an effect when using Crashpad on macOS. If enabled, - * Crashpad forwards crashes to the macOS system crash reporter. Depending - * on the crash, this may impact the crash time. Even if enabled, Crashpad - * may choose not to forward certain crashes. + * This setting only has an effect when using the crashpad or native backend on + * macOS. If enabled, the crash handler forwards crashes to the macOS system + * crash reporter. Depending on the crash, this may impact the crash time. Even + * if enabled, the crash handler may choose not to forward certain crashes. */ SENTRY_API void sentry_options_set_system_crash_reporter_enabled( sentry_options_t *opts, int enabled); diff --git a/src/backends/native/sentry_crash_context.h b/src/backends/native/sentry_crash_context.h index ee6af9707..35ec680c2 100644 --- a/src/backends/native/sentry_crash_context.h +++ b/src/backends/native/sentry_crash_context.h @@ -285,6 +285,7 @@ typedef struct { bool enable_large_attachments; bool http_retry; uint64_t shutdown_timeout; + bool system_crash_reporter_enabled; // Atomic user consent (sentry_user_consent_t), updated whenever user // consent changes so the daemon can honor it at crash time. diff --git a/src/backends/native/sentry_crash_handler.c b/src/backends/native/sentry_crash_handler.c index fe588d938..b48a46e43 100644 --- a/src/backends/native/sentry_crash_handler.c +++ b/src/backends/native/sentry_crash_handler.c @@ -100,6 +100,47 @@ static sentry_crash_ipc_t *g_crash_ipc = NULL; static struct sigaction g_previous_handlers[16]; static stack_t g_signal_stack = { 0 }; +static void +reset_signal_handlers(void) +{ + for (size_t i = 0; i < g_crash_signal_count; i++) { + sigaction(g_crash_signals[i], &g_previous_handlers[i], NULL); + } +} + +static void +reraise_signal(int signum) +{ + signal(signum, SIG_DFL); + raise(signum); +} + +static void +invoke_previous_signal_handler(int signum, siginfo_t *info, void *context) +{ + // Re-enable previous signal handlers before re-raising to prevent loops + reset_signal_handlers(); + + for (size_t i = 0; i < g_crash_signal_count; i++) { + if (g_crash_signals[i] != signum) { + continue; + } + + struct sigaction *handler = &g_previous_handlers[i]; + if (handler->sa_handler == SIG_DFL || handler->sa_handler == SIG_IGN) { + return; + } + + if (handler->sa_flags & SA_SIGINFO) { + handler->sa_sigaction(signum, info, context); + } else { + handler->sa_handler(signum); + } + + return; + } +} + /** * Get current thread ID (signal-safe) */ @@ -251,6 +292,15 @@ safe_build_stack_path( static void crash_signal_handler(int signum, siginfo_t *info, void *context) { + sentry_crash_ipc_t *ipc = g_crash_ipc; + if (!ipc || !ipc->shmem) { + // No IPC available, forward to the previous handler + invoke_previous_signal_handler(signum, info, context); + // The previous handler returned, fall back to default termination + reraise_signal(signum); + return; + } + // Only handle crash once - check if already processing static volatile long handling_crash = 0; if (!sentry__atomic_compare_swap(&handling_crash, 0, 1)) { @@ -258,16 +308,6 @@ crash_signal_handler(int signum, siginfo_t *info, void *context) _exit(1); } - // Re-enable signal to prevent loops - signal(signum, SIG_DFL); - - sentry_crash_ipc_t *ipc = g_crash_ipc; - if (!ipc || !ipc->shmem) { - // No IPC available, just re-raise - raise(signum); - return; - } - sentry_crash_context_t *ctx = ipc->shmem; ucontext_t *uctx = (ucontext_t *)context; @@ -762,7 +802,18 @@ crash_signal_handler(int signum, siginfo_t *info, void *context) } } - raise(signum); + invoke_previous_signal_handler(signum, info, context); + +# if defined(SENTRY_PLATFORM_MACOS) + if (!ctx->system_crash_reporter_enabled) { + // By convention, use the 128 + signal exit code without re-raising + // and invoking the macOS system crash reporter + _exit(128 + signum); + } +# endif + + // The previous handler returned, fall back to default termination + reraise_signal(signum); } int @@ -855,9 +906,7 @@ void sentry__crash_handler_shutdown(void) { // Restore previous signal handlers - for (size_t i = 0; i < g_crash_signal_count; i++) { - sigaction(g_crash_signals[i], &g_previous_handlers[i], NULL); - } + reset_signal_handlers(); // Clean up signal stack if (g_signal_stack.ss_sp) { diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index fe1af9322..abb72d920 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -293,6 +293,7 @@ native_backend_startup( // Set crash reporting mode from options ctx->crash_reporting_mode = options->crash_reporting_mode; + ctx->system_crash_reporter_enabled = options->system_crash_reporter_enabled; // Pass debug logging setting to daemon ctx->debug_enabled = options->debug; diff --git a/tests/unit/test_native_backend.c b/tests/unit/test_native_backend.c index 821598499..5a616a235 100644 --- a/tests/unit/test_native_backend.c +++ b/tests/unit/test_native_backend.c @@ -395,6 +395,7 @@ SENTRY_TEST(crash_context_options_propagation) sentry_options_set_ca_certs(options, "/path/to/ca-bundle.crt"); sentry_options_set_proxy(options, "http://myproxy:3128"); sentry_options_set_shutdown_timeout(options, 12345); + sentry_options_set_system_crash_reporter_enabled(options, true); // Verify options were set correctly TEST_CHECK_STRING_EQUAL( @@ -422,6 +423,7 @@ SENTRY_TEST(crash_context_options_propagation) ctx->user_agent[sizeof(ctx->user_agent) - 1] = '\0'; } ctx->shutdown_timeout = options->shutdown_timeout; + ctx->system_crash_reporter_enabled = options->system_crash_reporter_enabled; // Verify crash context received the values TEST_CHECK_STRING_EQUAL(ctx->ca_certs, "/path/to/ca-bundle.crt"); @@ -429,6 +431,7 @@ SENTRY_TEST(crash_context_options_propagation) // user_agent should have the default SDK user agent TEST_CHECK(ctx->user_agent[0] != '\0'); TEST_CHECK_UINT64_EQUAL(ctx->shutdown_timeout, 12345); + TEST_CHECK(ctx->system_crash_reporter_enabled); sentry_free(ctx); sentry_options_free(options);