From 02ac3eaa00cf8ce657e9342f2391a4b5c509d60a Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 16 Apr 2026 18:01:26 +0200 Subject: [PATCH 1/3] fix(breakpad): avoid stdio deadlock in macOS exception handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macOS, breakpad uses an in-process Mach exception server: when the process crashes, the kernel suspends all threads of the task and delivers the exception to breakpad's handler thread. Any stdio lock (e.g. the stderr file lock acquired by `flockfile` inside `vfprintf`/`fwrite`) held by a thread that was suspended at crash time is never released — so any `vfprintf`-based logging from the handler deadlocks the process. A repro sample captured in CI shows the pattern: sentry-http (worker, suspended): curl_easy_perform -> Curl_verboseconnect -> Curl_debug -> fwrite -> __sfvwrite -> __write_nocancel (holding stderr flockfile) breakpad exception handler (running): breakpad_backend_callback -> sentry__logger_log -> sentry__logger_defaultlogger -> vfprintf -> flockfile -> __psynch_mutexwait (blocked forever) The trigger in our tests is `sentry_options_set_debug(true)`, which enables `CURLOPT_VERBOSE` and routes libcurl's verbose output to stderr via `fwrite`. Under load (CI macos-14 runner, local repro under 8x CPU stress) the worker is reliably caught mid-`fwrite` when the main thread crashes. Two changes address this: 1. Default `enable_logging_when_crashed` to off for the breakpad backend on macOS, so the callback's `sentry__logger_disable()` fires by default and no reachable `vfprintf` call is made. Users who accept the risk can still opt in by calling `sentry_options_set_logger_enabled_when_crashed(opts, 1)`. 2. Move the signal-safe log macro from `sentry_backend_inproc.c` into `sentry_logger.h` and convert the five explicit log sites in the breakpad callback, plus the crash-safe flush helpers in `sentry_metrics`, `sentry_logs`, and `sentry_batcher`, to use it. `SENTRY_SIGNAL_SAFE_LOG` writes a static string directly via `write(STDERR_FILENO, ...)`, bypassing the stdio lock entirely — so those lines remain visible during crash handling even when the regular logger is disabled. A related class of deadlock still exists in breakpad's own `MinidumpGenerator` (`operator new` → `mfm_alloc`) when another thread is suspended mid-allocation; that is outside sentry-native's control and not addressed here. Co-Authored-By: Claude Opus 4.6 (1M context) --- include/sentry.h | 8 +++++++- src/backends/sentry_backend_breakpad.cpp | 12 +++++++----- src/backends/sentry_backend_inproc.c | 25 ------------------------ src/sentry_batcher.c | 6 +++--- src/sentry_logger.h | 25 ++++++++++++++++++++++++ src/sentry_logs.c | 4 ++-- src/sentry_metrics.c | 4 ++-- src/sentry_options.c | 11 +++++++++++ 8 files changed, 57 insertions(+), 38 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 14aa8ecd3a..4b132d2643 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1439,9 +1439,15 @@ SENTRY_API void sentry_options_set_logger( /** * Enables or disables console logging after a crash. + * * When disabled, Sentry will not invoke logger callbacks after a crash * has been detected. This can be useful to avoid potential issues during - * crash handling that logging might cause. This is enabled by default. + * crash handling that logging might cause. + * + * Enabled by default, except for the breakpad backend on macOS, where + * it defaults to off because the in-process Mach exception handler can + * deadlock on stdio locks held by threads that were suspended at crash + * time. */ SENTRY_API void sentry_options_set_logger_enabled_when_crashed( sentry_options_t *opts, int enabled); diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index a3f80b99b5..db329eb0d0 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -84,7 +84,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, } } - SENTRY_INFO("entering breakpad minidump callback"); + SENTRY_SIGNAL_SAFE_LOG("INFO entering breakpad minidump callback"); // this is a bit strange, according to docs, `succeeded` should be true when // a minidump file was successfully generated. however, when running our @@ -147,7 +147,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, uctx = &uctx_data; #endif - SENTRY_DEBUG("invoking `on_crash` hook"); + SENTRY_SIGNAL_SAFE_LOG("DEBUG invoking `on_crash` hook"); sentry_value_t result = options->on_crash_func(uctx, event, options->on_crash_data); should_handle = !sentry_value_is_null(result); @@ -165,7 +165,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, bool capture_screenshot = options->attach_screenshot; #ifdef SENTRY_PLATFORM_WINDOWS if (capture_screenshot && options->before_screenshot_func) { - SENTRY_DEBUG("invoking `before_screenshot` hook"); + SENTRY_SIGNAL_SAFE_LOG( + "DEBUG invoking `before_screenshot` hook"); capture_screenshot = options->before_screenshot_func( event, options->before_screenshot_data) != 0; @@ -214,7 +215,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry__path_remove(dump_path); sentry__path_free(dump_path); } else { - SENTRY_DEBUG("event was discarded by the `on_crash` hook"); + SENTRY_SIGNAL_SAFE_LOG( + "DEBUG event was discarded by the `on_crash` hook"); sentry_value_decref(event); } @@ -223,7 +225,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry__transport_dump_queue(options->transport, options->run); // and restore the old transport } - SENTRY_INFO("crash has been captured"); + SENTRY_SIGNAL_SAFE_LOG("INFO crash has been captured"); #ifndef SENTRY_PLATFORM_WINDOWS sentry__leave_signal_handler(); diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index a52371d828..e2b1d97a33 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -32,31 +32,6 @@ #endif #include -/** - * Signal/async-safe logging macro for use in signal handlers or other - * contexts where stdio and malloc are unsafe. Only supports static strings. - */ -#ifdef SENTRY_PLATFORM_UNIX -# include -# define SENTRY_SIGNAL_SAFE_LOG(msg) \ - do { \ - static const char _msg[] = "[sentry] " msg "\n"; \ - (void)!write(STDERR_FILENO, _msg, sizeof(_msg) - 1); \ - } while (0) -#elif defined(SENTRY_PLATFORM_WINDOWS) -# define SENTRY_SIGNAL_SAFE_LOG(msg) \ - do { \ - static const char _msg[] = "[sentry] " msg "\n"; \ - OutputDebugStringA(_msg); \ - HANDLE _stderr = GetStdHandle(STD_ERROR_HANDLE); \ - if (_stderr && _stderr != INVALID_HANDLE_VALUE) { \ - DWORD _written; \ - WriteFile(_stderr, _msg, (DWORD)(sizeof(_msg) - 1), &_written, \ - NULL); \ - } \ - } while (0) -#endif - /** * Inproc Backend Introduction * diff --git a/src/sentry_batcher.c b/src/sentry_batcher.c index 8bf44ebd81..6130de4eac 100644 --- a/src/sentry_batcher.c +++ b/src/sentry_batcher.c @@ -155,9 +155,9 @@ sentry__batcher_flush(sentry_batcher_t *batcher, bool crash_safe) while (!sentry__atomic_compare_swap(&batcher->flushing, 0, 1)) { const int max_attempts = 200; if (++attempts > max_attempts) { - SENTRY_WARN( - "sentry__batcher_flush: timeout waiting for flushing " - "lock in crash-safe mode"); + SENTRY_SIGNAL_SAFE_LOG( + "WARN sentry__batcher_flush: timeout waiting for " + "flushing lock in crash-safe mode"); return false; } diff --git a/src/sentry_logger.h b/src/sentry_logger.h index b6c8febedc..51ee723fd8 100644 --- a/src/sentry_logger.h +++ b/src/sentry_logger.h @@ -51,4 +51,29 @@ void sentry__logger_disable(void); #define SENTRY_FATAL(message) sentry__logger_log(SENTRY_LEVEL_FATAL, message) +/** + * Signal/async-safe logging macro for use in signal handlers or other + * contexts where stdio and malloc are unsafe. Only supports static strings. + */ +#ifdef SENTRY_PLATFORM_UNIX +# include +# define SENTRY_SIGNAL_SAFE_LOG(msg) \ + do { \ + static const char _msg[] = "[sentry] " msg "\n"; \ + (void)!write(STDERR_FILENO, _msg, sizeof(_msg) - 1); \ + } while (0) +#elif defined(SENTRY_PLATFORM_WINDOWS) +# define SENTRY_SIGNAL_SAFE_LOG(msg) \ + do { \ + static const char _msg[] = "[sentry] " msg "\n"; \ + OutputDebugStringA(_msg); \ + HANDLE _stderr = GetStdHandle(STD_ERROR_HANDLE); \ + if (_stderr && _stderr != INVALID_HANDLE_VALUE) { \ + DWORD _written; \ + WriteFile(_stderr, _msg, (DWORD)(sizeof(_msg) - 1), &_written, \ + NULL); \ + } \ + } while (0) +#endif + #endif diff --git a/src/sentry_logs.c b/src/sentry_logs.c index e75fa9c383..b176aeef54 100644 --- a/src/sentry_logs.c +++ b/src/sentry_logs.c @@ -520,12 +520,12 @@ sentry__logs_shutdown(uint64_t timeout) void sentry__logs_flush_crash_safe(void) { - SENTRY_DEBUG("crash-safe logs flush"); + SENTRY_SIGNAL_SAFE_LOG("DEBUG crash-safe logs flush"); sentry_batcher_t *batcher = sentry__batcher_peek(&g_batcher); if (batcher) { sentry__batcher_flush_crash_safe(batcher); } - SENTRY_DEBUG("crash-safe logs flush complete"); + SENTRY_SIGNAL_SAFE_LOG("DEBUG crash-safe logs flush complete"); } uintptr_t diff --git a/src/sentry_metrics.c b/src/sentry_metrics.c index 6c45923f9c..e7e83ce3f4 100644 --- a/src/sentry_metrics.c +++ b/src/sentry_metrics.c @@ -165,12 +165,12 @@ sentry__metrics_shutdown(uint64_t timeout) void sentry__metrics_flush_crash_safe(void) { - SENTRY_DEBUG("crash-safe metrics flush"); + SENTRY_SIGNAL_SAFE_LOG("DEBUG crash-safe metrics flush"); sentry_batcher_t *batcher = sentry__batcher_peek(&g_batcher); if (batcher) { sentry__batcher_flush_crash_safe(batcher); } - SENTRY_DEBUG("crash-safe metrics flush complete"); + SENTRY_SIGNAL_SAFE_LOG("DEBUG crash-safe metrics flush complete"); } uintptr_t diff --git a/src/sentry_options.c b/src/sentry_options.c index 45aa9bbc60..e63e0b4f98 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -49,7 +49,18 @@ sentry_options_new(void) opts->system_crash_reporter_enabled = false; opts->attach_screenshot = false; opts->crashpad_wait_for_upload = false; + // On macOS, breakpad's Mach exception handler runs while all other + // threads are suspended by the kernel. Any vfprintf-based log call from + // that context can deadlock waiting for a stdio lock held by a suspended + // thread (e.g. libcurl's verbose output on stderr). Default off here; + // users who accept the risk can still opt in via + // `sentry_options_set_logger_enabled_when_crashed(opts, 1)`. +#if defined(SENTRY_BACKEND_BREAKPAD) && defined(SENTRY_PLATFORM_DARWIN) \ + && !defined(SENTRY_PLATFORM_IOS) + opts->enable_logging_when_crashed = false; +#else opts->enable_logging_when_crashed = true; +#endif opts->propagate_traceparent = false; opts->crashpad_limit_stack_capture_to_sp = false; opts->enable_metrics = true; From d53a2e473ff5388f99fc5ee87f70398229cc9041 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 16 Apr 2026 18:27:04 +0200 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aee5f006a..1e6505c46c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Fixes**: - Linux: handle `ENOSYS` in `read_safely` to fix empty module list in seccomp-restricted environments. ([#1655](https://github.com/getsentry/sentry-native/pull/1655)) +- macOS: avoid stdio deadlock in breakpad exception handler. ([#1656](https://github.com/getsentry/sentry-native/pull/1656)) ## 0.13.7 From 87a0b77ea85903809ead615b3c9d3b29527cdab6 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 17 Apr 2026 10:31:48 +0200 Subject: [PATCH 3/3] fix up macOS breakpad comment per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thread suspension happens in breakpad's minidump-writing path, not via Mach exception delivery — our callback runs inside WriteMinidumpWithException() while breakpad has the other threads held. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/sentry_options.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/sentry_options.c b/src/sentry_options.c index e63e0b4f98..d8a173278d 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -49,12 +49,13 @@ sentry_options_new(void) opts->system_crash_reporter_enabled = false; opts->attach_screenshot = false; opts->crashpad_wait_for_upload = false; - // On macOS, breakpad's Mach exception handler runs while all other - // threads are suspended by the kernel. Any vfprintf-based log call from - // that context can deadlock waiting for a stdio lock held by a suspended - // thread (e.g. libcurl's verbose output on stderr). Default off here; - // users who accept the risk can still opt in via - // `sentry_options_set_logger_enabled_when_crashed(opts, 1)`. + // On macOS, breakpad suspends all other threads of the process before + // writing the minidump and invokes our backend callback from inside + // WriteMinidumpWithException() while those threads are still suspended. + // Any vfprintf-based log call from that context can deadlock waiting for + // a stdio lock held by a suspended thread (e.g. libcurl's verbose output + // on stderr). Default off here; users who accept the risk can still opt + // in via `sentry_options_set_logger_enabled_when_crashed(opts, 1)`. #if defined(SENTRY_BACKEND_BREAKPAD) && defined(SENTRY_PLATFORM_DARWIN) \ && !defined(SENTRY_PLATFORM_IOS) opts->enable_logging_when_crashed = false;