fix(breakpad): avoid stdio deadlock in macOS exception handler

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
 

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);

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();

src/backends/sentry_backend_inproc.c

@@ -32,31 +32,6 @@
 #endif
 #include <string.h>
 
-/**
- * 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 <unistd.h>
-#    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
  *

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;
             }
 

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 <unistd.h>
+#    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

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

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

src/sentry_options.c

@@ -49,7 +49,19 @@ sentry_options_new(void)
     opts->system_crash_reporter_enabled = false;
     opts->attach_screenshot = false;
     opts->crashpad_wait_for_upload = false;
+    // 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;
+#else
     opts->enable_logging_when_crashed = true;
+#endif
     opts->propagate_traceparent = false;
     opts->crashpad_limit_stack_capture_to_sp = false;
     opts->enable_metrics = true;