Ensure handling_crash flag is atomic

bugsnag · Mar 28, 2022 · 4e7dd67 · 4e7dd67
1 parent d6d3b48
commit 4e7dd67
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 5 deletions.
diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
@@ -58,6 +58,17 @@ bool bsg_run_on_error() {
   return true;
 }
 
+bool bsg_begin_handling_crash() {
+  static bool expected = false;
+  return atomic_compare_exchange_strong(&bsg_global_env->handling_crash,
+                                        &expected, true);
+}
+
+void bsg_finish_handling_crash() {
+  bsg_global_env->crash_handled = true;
+  bsg_global_env->handling_crash = false;
+}
+
 JNIEXPORT void JNICALL Java_com_bugsnag_android_NdkPlugin_enableCrashReporting(
     JNIEnv *env, jobject _this) {
   if (bsg_global_env == NULL) {
@@ -143,6 +154,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
   bugsnag_env->report_header.version = BUGSNAG_EVENT_VERSION;
   bugsnag_env->consecutive_launch_crashes = consecutive_launch_crashes;
   bugsnag_env->send_threads = send_threads;
+  bugsnag_env->handling_crash = ATOMIC_VAR_INIT(false);
 
   // copy event path to env struct
   const char *event_path = bsg_safe_get_string_utf_chars(env, _event_path);

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h
@@ -4,6 +4,7 @@
 #ifndef BUGSNAG_NDK_H
 #define BUGSNAG_NDK_H
 
+#include <stdatomic.h>
 #include <stdbool.h>
 
 #include "../assets/include/bugsnag.h"
@@ -53,7 +54,7 @@ typedef struct {
    * true if a crash is currently being handled. Disallows multiple crashes
    * from being processed simultaneously
    */
-  bool handling_crash;
+  _Atomic bool handling_crash;
   /**
    * true if a handler has completed crash handling
    */
@@ -78,6 +79,22 @@ typedef struct {
  */
 bool bsg_run_on_error();
 
+/**
+ * This must be called before handling a native crash to ensure that two crash
+ * handlers don't run simultaneously. If this function returns falls, DO NOT
+ * PROCEED WITH CRASH HANDLING!
+ *
+ * When done handling the crash, you must call bsg_finish_handling_crash()
+ *
+ * @return true if no other crash handler is already running.
+ */
+bool bsg_begin_handling_crash();
+
+/**
+ * Let the system know that you've finished handling the crash.
+ */
+void bsg_finish_handling_crash();
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/handlers/cpp_handler.cpp b/bugsnag-plugin-android-ndk/src/main/jni/handlers/cpp_handler.cpp
@@ -47,7 +47,10 @@ void bsg_handle_cpp_terminate() {
   if (bsg_global_env == NULL || bsg_global_env->handling_crash)
     return;
 
-  bsg_global_env->handling_crash = true;
+  if (!bsg_begin_handling_crash()) {
+    return;
+  }
+
   bsg_populate_event_as(bsg_global_env);
   bsg_global_env->next_event.unhandled = true;
   bsg_global_env->next_event.error.frame_count = bsg_unwind_crash_stack(
@@ -72,7 +75,8 @@ void bsg_handle_cpp_terminate() {
     bsg_serialize_event_to_file(bsg_global_env);
     bsg_serialize_last_run_info_to_file(bsg_global_env);
   }
-  bsg_global_env->crash_handled = true;
+
+  bsg_finish_handling_crash();
   bsg_handler_uninstall_cpp();
   if (bsg_global_terminate_previous != NULL) {
     bsg_global_terminate_previous();

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/handlers/signal_handler.c b/bugsnag-plugin-android-ndk/src/main/jni/handlers/signal_handler.c
@@ -168,7 +168,7 @@ void bsg_handle_signal(int signum, siginfo_t *info,
   if (bsg_global_env == NULL) {
     return;
   }
-  if (bsg_global_env->handling_crash) {
+  if (!bsg_begin_handling_crash()) {
     if (bsg_global_env->crash_handled) {
       // The C++ handler default action is to raise a fatal signal once
       // handling is complete. The report is already generated so at this
@@ -179,7 +179,6 @@ void bsg_handle_signal(int signum, siginfo_t *info,
     return;
   }
 
-  bsg_global_env->handling_crash = true;
   bsg_global_env->next_event.unhandled = true;
   bsg_populate_event_as(bsg_global_env);
   bsg_global_env->next_event.error.frame_count = bsg_unwind_crash_stack(
@@ -209,6 +208,8 @@ void bsg_handle_signal(int signum, siginfo_t *info,
     bsg_serialize_event_to_file(bsg_global_env);
     bsg_serialize_last_run_info_to_file(bsg_global_env);
   }
+
+  bsg_finish_handling_crash();
   bsg_handler_uninstall_signal();
   bsg_invoke_previous_signal_handler(signum, info, user_context);
 }