Merge pull request #1288 from bugsnag/release-v6.16.2

Release v6.16.2
bugsnag · Jan 26, 2022 · c8454a8 · c8454a8
2 parents efe6aa8 + cb85912
commit c8454a8
Show file tree

Hide file tree

Showing 19 changed files with 200 additions and 81 deletions.
diff --git a/.jazzy.yaml b/.jazzy.yaml
@@ -2,11 +2,11 @@ author_url: "https://www.bugsnag.com"
 author: "Bugsnag Inc"
 clean: false # avoid deleting docs/.git
 framework_root: "Bugsnag"
-github_file_prefix: "https://github.com/bugsnag/bugsnag-cocoa/tree/v6.16.1/Bugsnag"
+github_file_prefix: "https://github.com/bugsnag/bugsnag-cocoa/tree/v6.16.2/Bugsnag"
 github_url: "https://github.com/bugsnag/bugsnag-cocoa"
 hide_documentation_coverage: true
 module: "Bugsnag"
-module_version: "6.16.1"
+module_version: "6.16.2"
 objc: true
 output: "docs"
 readme: "README.md"

diff --git a/Bugsnag.podspec.json b/Bugsnag.podspec.json
@@ -1,6 +1,6 @@
 {
   "name": "Bugsnag",
-  "version": "6.16.1",
+  "version": "6.16.2",
   "summary": "The Bugsnag crash reporting framework for Apple platforms.",
   "homepage": "https://bugsnag.com",
   "license": "MIT",
@@ -9,7 +9,7 @@
   },
   "source": {
     "git": "https://github.com/bugsnag/bugsnag-cocoa.git",
-    "tag": "v6.16.1"
+    "tag": "v6.16.2"
   },
   "frameworks": [
     "Foundation",

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c
@@ -36,6 +36,8 @@
 #include "BSG_KSLogger.h"
 #include "BSG_KSMach.h"
 
+#include <stdatomic.h>
+
 // ============================================================================
 #pragma mark - Globals -
 // ============================================================================
@@ -211,7 +213,50 @@ void bsg_kscrashsentry_clearContext(BSG_KSCrash_SentryContext *context) {
     memcpy(context->reservedThreads, reservedThreads, sizeof(reservedThreads));
 }
 
-void bsg_kscrashsentry_beginHandlingCrash(BSG_KSCrash_SentryContext *context) {
-    bsg_kscrashsentry_clearContext(context);
-    context->handlingCrash = true;
+// Set to true once _endHandlingCrash() has been called and it is safe to resume
+// any secondary crashed threads.
+static atomic_bool bsg_g_didHandleCrash;
+
+bool bsg_kscrashsentry_beginHandlingCrash(const thread_t offender) {
+    static _Atomic(thread_t) firstOffender;
+    static thread_t firstHandlingThread;
+
+    thread_t expected = 0;
+    if (atomic_compare_exchange_strong(&firstOffender, &expected, offender)) {
+        firstHandlingThread = bsg_ksmachthread_self();
+        BSG_KSLOG_DEBUG("Handling app crash in thread 0x%x", offender);
+        bsg_kscrashsentry_clearContext(bsg_g_context);
+        bsg_g_context->handlingCrash = true;
+        bsg_g_context->offendingThread = offender;
+        return true;
+    }
+
+    if (offender == firstHandlingThread) {
+        BSG_KSLOG_INFO("Detected crash in the crash reporter. "
+                       "Restoring original handlers.");
+        bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAsyncSafe);
+
+        // Reset the context to write a recrash report.
+        bsg_kscrashsentry_clearContext(bsg_g_context);
+        bsg_g_context->crashedDuringCrashHandling = true;
+        bsg_g_context->handlingCrash = true;
+        bsg_g_context->offendingThread = offender;
+        return true;
+    }
+
+    BSG_KSLOG_DEBUG("Ignoring secondary app crash in thread 0x%x", offender);
+    // Block this thread to prevent the crash handling thread from being
+    // interrupted while writing the crash report. If we allowed the default
+    // handler to be triggered for this thread, the process would be killed
+    // before the crash report can be written. The process will be killed by the
+    // default handler once the handling thread has finished and threads resume.
+    while (!atomic_load(&bsg_g_didHandleCrash)) {
+        usleep(USEC_PER_SEC / 10);
+    }
+    return false;
+}
+
+void bsg_kscrashsentry_endHandlingCrash(void) {
+    BSG_KSLOG_DEBUG("Noting completion of crash handling");
+    atomic_store(&bsg_g_didHandleCrash, true);
 }
diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_CPPException.mm b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_CPPException.mm
@@ -48,6 +48,7 @@
 extern "C" {
 #endif
 // Internal NSException recorder
+bool bsg_kscrashsentry_isNSExceptionHandlerInstalled(void);
 void bsg_recordException(NSException *exception);
 #ifdef __cplusplus
 }
@@ -127,9 +128,11 @@ static void CPPExceptionTerminate(void) {
                         "the current NSException handler deal with it.");
         isNSException = true;
         // recordException() doesn't call beginHandlingCrash()
-        bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);
-        bsg_recordException(exception);
-        bsg_g_context->handlingCrash = false;
+        if (bsg_kscrashsentry_isNSExceptionHandlerInstalled() &&
+            bsg_kscrashsentry_beginHandlingCrash(bsg_ksmachthread_self())) {
+            bsg_recordException(exception);
+            bsg_kscrashsentry_endHandlingCrash();
+        }
     } catch (std::exception &exc) {
         strlcpy(descriptionBuff, exc.what(), sizeof(descriptionBuff));
     } catch (std::exception *exc) {
@@ -159,22 +162,13 @@ static void CPPExceptionTerminate(void) {
     }
     bsg_g_captureNextStackTrace = (bsg_g_installed != 0);
 
-    if (!isNSException) {
-        bool wasHandlingCrash = bsg_g_context->handlingCrash;
-        bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);
-
-        if (wasHandlingCrash) {
-            BSG_KSLOG_INFO("Detected crash in the crash reporter. "
-                           "Restoring original handlers.");
-            bsg_g_context->crashedDuringCrashHandling = true;
-            bsg_kscrashsentry_uninstall((BSG_KSCrashType)BSG_KSCrashTypeAll);
-        }
+    if (!isNSException &&
+        bsg_kscrashsentry_beginHandlingCrash(bsg_ksmachthread_self())) {
 
         BSG_KSLOG_DEBUG("Suspending all threads.");
         bsg_kscrashsentry_suspendThreads();
 
         bsg_g_context->crashType = BSG_KSCrashTypeCPPException;
-        bsg_g_context->offendingThread = bsg_ksmachthread_self();
         bsg_g_context->registersAreValid = false;
         bsg_g_context->stackTrace =
             bsg_g_stackTrace + 1; // Don't record __cxa_throw stack entry
@@ -189,6 +183,7 @@ static void CPPExceptionTerminate(void) {
             "Crash handling complete. Restoring original handlers.");
         bsg_kscrashsentry_uninstall((BSG_KSCrashType)BSG_KSCrashTypeAll);
         bsg_kscrashsentry_resumeThreads();
+        bsg_kscrashsentry_endHandlingCrash();
     }
     if (bsg_g_originalTerminateHandler != NULL) {
         bsg_g_originalTerminateHandler();

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_MachException.c b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_MachException.c
@@ -238,12 +238,8 @@ void *ksmachexc_i_handleExceptions(void *const userData) {
 
     BSG_KSLOG_DEBUG("Trapped mach exception code 0x%llx, subcode 0x%llx",
                     exceptionMessage.code[0], exceptionMessage.code[1]);
-    if (bsg_g_installed) {
-        bool wasHandlingCrash = bsg_g_context->handlingCrash;
-        bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);
-
-        BSG_KSLOG_DEBUG(
-            "Exception handler is installed. Continuing exception handling.");
+    if (bsg_g_installed &&
+        bsg_kscrashsentry_beginHandlingCrash(exceptionMessage.thread.name)) {
 
         BSG_KSLOG_DEBUG("Suspending all threads");
         bsg_kscrashsentry_suspendThreads();
@@ -264,15 +260,6 @@ void *ksmachexc_i_handleExceptions(void *const userData) {
             bsg_ksmachexc_i_restoreExceptionPorts();
         }
 
-        if (wasHandlingCrash) {
-            BSG_KSLOG_INFO("Detected crash in the crash reporter. Restoring "
-                           "original handlers.");
-            // The crash reporter itself crashed. Make a note of this and
-            // uninstall all handlers so that we don't get stuck in a loop.
-            bsg_g_context->crashedDuringCrashHandling = true;
-            bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAsyncSafe);
-        }
-
         // Fill out crash information
         BSG_KSLOG_DEBUG("Fetching machine state.");
         BSG_STRUCT_MCONTEXT_L machineContext;
@@ -289,7 +276,6 @@ void *ksmachexc_i_handleExceptions(void *const userData) {
 
         BSG_KSLOG_DEBUG("Filling out context.");
         bsg_g_context->crashType = BSG_KSCrashTypeMachException;
-        bsg_g_context->offendingThread = exceptionMessage.thread.name;
         bsg_g_context->registersAreValid = true;
         bsg_g_context->mach.type = exceptionMessage.exception;
         bsg_g_context->mach.code = exceptionMessage.code[0] & (int64_t)MACH_ERROR_CODE_MASK;
@@ -302,6 +288,7 @@ void *ksmachexc_i_handleExceptions(void *const userData) {
             "Crash handling complete. Restoring original handlers.");
         bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAsyncSafe);
         bsg_kscrashsentry_resumeThreads();
+        bsg_kscrashsentry_endHandlingCrash();
     }
 
     BSG_KSLOG_DEBUG("Replying to mach exception message.");

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m
@@ -78,30 +78,20 @@
  */
 void bsg_ksnsexc_i_handleException(NSException *exception) {
     BSG_KSLOG_DEBUG("Trapped exception %s", exception.description.UTF8String);
-    if (bsg_g_installed) {
-        bool wasHandlingCrash = bsg_g_context->handlingCrash;
-        bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);
-
-        BSG_KSLOG_DEBUG(
-            "Exception handler is installed. Continuing exception handling.");
-
-        if (wasHandlingCrash) {
-            BSG_KSLOG_INFO("Detected crash in the crash reporter. Restoring "
-                           "original handlers.");
-            bsg_g_context->crashedDuringCrashHandling = true;
-            bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAll);
-        }
+    if (bsg_g_installed &&
+        bsg_kscrashsentry_beginHandlingCrash(bsg_ksmachthread_self())) {
 
         bsg_recordException(exception);
 
         BSG_KSLOG_DEBUG(
             "Crash handling complete. Restoring original handlers.");
         bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAll);
+        bsg_kscrashsentry_endHandlingCrash();
+    }
 
-        if (bsg_g_previousUncaughtExceptionHandler != NULL) {
-            BSG_KSLOG_DEBUG("Calling original exception handler.");
-            bsg_g_previousUncaughtExceptionHandler(exception);
-        }
+    if (bsg_g_previousUncaughtExceptionHandler != NULL) {
+        BSG_KSLOG_DEBUG("Calling original exception handler.");
+        bsg_g_previousUncaughtExceptionHandler(exception);
     }
 }
 
@@ -165,9 +155,10 @@ void bsg_recordException(NSException *exception) {
 static void bsg_reportException(id self, SEL _cmd, NSException *exception) {
     BSG_KSLOG_DEBUG("reportException: %s", exception.description.UTF8String);
 
-    bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);
-
-    bsg_recordException(exception);
+    if (bsg_kscrashsentry_beginHandlingCrash(bsg_ksmachthread_self())) {
+        bsg_recordException(exception);
+        bsg_kscrashsentry_endHandlingCrash();
+    }
 
 #if TARGET_OS_MACCATALYST
     // Mac Catalyst apps continue to run after an uncaught exception is thrown
@@ -241,3 +232,7 @@ void bsg_kscrashsentry_uninstallNSExceptionHandler(void) {
 
     bsg_g_installed = 0;
 }
+
+bool bsg_kscrashsentry_isNSExceptionHandlerInstalled(void) {
+    return bsg_g_installed;
+}
diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_Private.h b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_Private.h
@@ -47,9 +47,30 @@ void bsg_kscrashsentry_suspendThreads(void);
  */
 void bsg_kscrashsentry_resumeThreads(void);
 
-/** Prepare the context for handling a new crash.
+/**
+ * Prepares the context for handling a crash.
+ *
+ * Only a single crash report can be written per process lifetime, with the
+ * exception of a crash in a crash handling thread for which a "recrash" report
+ * can be written.
+ *
+ * True is returned if this crash should be processed. The caller should fill in
+ * the context's crash details and call bsg_g_context->onCrash(). The caller
+ * must call endHandlingCrash() once processing is complete. The process is
+ * then expected to die once the default crash handler executes.
+ *
+ * False is returned if a crash has already been handled, to prevent
+ * interrupting its processing and / or overwriting the existing crash report.
+ * In this case the call to beginHandlingCrash() blocks until the crash handling
+ * thread calls endHandlingCrash().
+ */
+bool bsg_kscrashsentry_beginHandlingCrash(const thread_t offender);
+
+/**
+ * Sentries must call this to unblock any sencondary crashed threads that are
+ * waiting in beginHandlingCrash().
  */
-void bsg_kscrashsentry_beginHandlingCrash(BSG_KSCrash_SentryContext *context);
+void bsg_kscrashsentry_endHandlingCrash(void);
 
 /** Clear a crash sentry context.
  */

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_Signal.c b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_Signal.c
@@ -101,26 +101,14 @@ static struct sigaction *get_previous_sigaction(int sigNum) {
 void bsg_kssighndl_i_handleSignal(int sigNum, siginfo_t *signalInfo,
                                   void *userContext) {
     BSG_KSLOG_DEBUG("Trapped signal %d", sigNum);
-    if (bsg_g_enabled) {
-        bool wasHandlingCrash = bsg_g_context->handlingCrash;
-        bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);
-
-        BSG_KSLOG_DEBUG(
-            "Signal handler is installed. Continuing signal handling.");
+    if (bsg_g_enabled &&
+        bsg_kscrashsentry_beginHandlingCrash(bsg_ksmachthread_self())) {
 
         BSG_KSLOG_DEBUG("Suspending all threads.");
         bsg_kscrashsentry_suspendThreads();
 
-        if (wasHandlingCrash) {
-            BSG_KSLOG_INFO("Detected crash in the crash reporter. Restoring "
-                           "original handlers.");
-            bsg_g_context->crashedDuringCrashHandling = true;
-            bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAsyncSafe);
-        }
-
         BSG_KSLOG_DEBUG("Filling out context.");
         bsg_g_context->crashType = BSG_KSCrashTypeSignal;
-        bsg_g_context->offendingThread = bsg_ksmachthread_self();
         bsg_g_context->registersAreValid = true;
         bsg_g_context->faultAddress = (uintptr_t)signalInfo->si_addr;
         bsg_g_context->signal.userContext = userContext;
@@ -133,6 +121,7 @@ void bsg_kssighndl_i_handleSignal(int sigNum, siginfo_t *signalInfo,
             "Crash handling complete. Restoring original handlers.");
         bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAsyncSafe);
         bsg_kscrashsentry_resumeThreads();
+        bsg_kscrashsentry_endHandlingCrash();
     }
 
     BSG_KSLOG_DEBUG(

diff --git a/Bugsnag/Payload/BugsnagNotifier.m b/Bugsnag/Payload/BugsnagNotifier.m
@@ -23,7 +23,7 @@ - (instancetype)init {
 #else
         _name = @"Bugsnag Objective-C";
 #endif
-        _version = @"6.16.1";
+        _version = @"6.16.2";
         _url = @"https://github.com/bugsnag/bugsnag-cocoa";
         _dependencies = @[];
     }

diff --git a/BugsnagNetworkRequestPlugin.podspec.json b/BugsnagNetworkRequestPlugin.podspec.json
@@ -1,16 +1,16 @@
 {
   "name": "BugsnagNetworkRequestPlugin",
-  "version": "6.16.1",
+  "version": "6.16.2",
   "summary": "Network request monitoring support for Bugsnag.",
   "homepage": "https://bugsnag.com",
   "license": "MIT",
   "authors": {
     "Bugsnag": "[email protected]"
   },
-  "readme": "https://raw.githubusercontent.com/bugsnag/bugsnag-cocoa/v6.16.1/BugsnagNetworkRequestPlugin/README.md",
+  "readme": "https://raw.githubusercontent.com/bugsnag/bugsnag-cocoa/v6.16.2/BugsnagNetworkRequestPlugin/README.md",
   "source": {
     "git": "https://github.com/bugsnag/bugsnag-cocoa.git",
-    "tag": "v6.16.1"
+    "tag": "v6.16.2"
   },
   "dependencies": {
     "Bugsnag": "~> 6.13"

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 Changelog
 =========
 
+## 6.16.2 (2022-01-26)
+
+* Improve reliability of crash reporting when multiple crashes occur concurrently.
+  [#1286](https://github.com/bugsnag/bugsnag-cocoa/pull/1286)
+
 ## 6.16.1 (2022-01-19)
 
 ### Bug fixes

diff --git a/Framework/Info.plist b/Framework/Info.plist
@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>FMWK</string>
 	<key>CFBundleShortVersionString</key>
-	<string>6.16.1</string>
+	<string>6.16.2</string>
 	<key>CFBundleVersion</key>
 	<string>1</string>
 </dict>

diff --git a/Tests/BugsnagTests/Info.plist b/Tests/BugsnagTests/Info.plist
@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string>
 	<key>CFBundleShortVersionString</key>
-	<string>6.16.1</string>
+	<string>6.16.2</string>
 	<key>CFBundleVersion</key>
 	<string>1</string>
 </dict>

diff --git a/Tests/TestHost-iOS/Info.plist b/Tests/TestHost-iOS/Info.plist
@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string>
 	<key>CFBundleShortVersionString</key>
-	<string>6.16.1</string>
+	<string>6.16.2</string>
 	<key>CFBundleVersion</key>
 	<string>1</string>
 	<key>LSRequiresIPhoneOS</key>

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-6.16.1
+6.16.2