Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure handling_crash flag is atomic #1639

Merged
merged 1 commit into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 18 additions & 1 deletion bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#ifndef BUGSNAG_NDK_H
#define BUGSNAG_NDK_H

#include <stdatomic.h>
#include <stdbool.h>

#include "../assets/include/bugsnag.h"
Expand Down Expand Up @@ -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
*/
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,18 +168,19 @@ void bsg_handle_signal(int signum, siginfo_t *info,
if (bsg_global_env == NULL) {
return;
}
if (bsg_global_env->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
// point, the handler only needs to be uninstalled.
bsg_handler_uninstall_signal();
bsg_invoke_previous_signal_handler(signum, info, user_context);
}
if (!bsg_begin_handling_crash()) {
return;
}

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
// point, the handler only needs to be uninstalled.
bsg_handler_uninstall_signal();
bsg_invoke_previous_signal_handler(signum, info, user_context);
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(
Expand Down Expand Up @@ -209,6 +210,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);
}