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.

Sign up for GitHub

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

Don't attempt to share the bsg_jni_cache between threads #1585

Merged
merged 1 commit into from
Jan 21, 2022
Merged

Don't attempt to share the bsg_jni_cache between threads #1585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

* Discarded unhandled exceptions are propagated to any previously registered handlers
[#1584](/~https://github.com/bugsnag/bugsnag-android/pull/1584)

* Fix SIGABRT crashes caused by race conditions in the NDK layer
[#1585](/~https://github.com/bugsnag/bugsnag-android/pull/1585)

## 5.19.0 (2022-01-12)

Expand Down
50 changes: 27 additions & 23 deletions bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message,
jbyteArray jname = NULL;
jbyteArray jmessage = NULL;

if (!bsg_jni_cache_refresh(env)) {
bsg_jni_cache jni_cache;

if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
goto exit;
}
Expand All @@ -130,35 +132,34 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message,

// create StackTraceElement array
jtrace = bsg_safe_new_object_array(env, frame_count,
bsg_global_jni_cache->stack_trace_element);
jni_cache.stack_trace_element);
if (jtrace == NULL) {
goto exit;
}

// populate stacktrace object
populate_notify_stacktrace(env, stacktrace, frame_count,
bsg_global_jni_cache->stack_trace_element,
bsg_global_jni_cache->ste_constructor, jtrace);
jni_cache.stack_trace_element,
jni_cache.ste_constructor, jtrace);

// get the severity field
jfieldID severity_field =
parse_jseverity(env, severity, bsg_global_jni_cache->severity);
jfieldID severity_field = parse_jseverity(env, severity, jni_cache.severity);
if (severity_field == NULL) {
goto exit;
}
// get the error severity object
jseverity = bsg_safe_get_static_object_field(
env, bsg_global_jni_cache->severity, severity_field);
jseverity =
bsg_safe_get_static_object_field(env, jni_cache.severity, severity_field);
if (jseverity == NULL) {
goto exit;
}

jname = bsg_byte_ary_from_string(env, name);
jmessage = bsg_byte_ary_from_string(env, message);

bsg_safe_call_static_void_method(env, bsg_global_jni_cache->native_interface,
bsg_global_jni_cache->ni_notify, jname,
jmessage, jseverity, jtrace);
bsg_safe_call_static_void_method(env, jni_cache.native_interface,
jni_cache.ni_notify, jname, jmessage,
jseverity, jtrace);

goto exit;

Expand All @@ -173,7 +174,10 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message,

void bugsnag_set_user_env(JNIEnv *env, const char *id, const char *email,
const char *name) {
if (!bsg_jni_cache_refresh(env)) {

bsg_jni_cache jni_cache;

if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
return;
}
Expand All @@ -182,9 +186,8 @@ void bugsnag_set_user_env(JNIEnv *env, const char *id, const char *email,
jbyteArray jemail = bsg_byte_ary_from_string(env, email);
jbyteArray jname = bsg_byte_ary_from_string(env, name);

bsg_safe_call_static_void_method(env, bsg_global_jni_cache->native_interface,
bsg_global_jni_cache->ni_set_user, jid,
jemail, jname);
bsg_safe_call_static_void_method(env, jni_cache.native_interface,
jni_cache.ni_set_user, jid, jemail, jname);

bsg_safe_release_byte_array_elements(env, jid, (jbyte *)id);
bsg_safe_delete_local_ref(env, jid);
Expand Down Expand Up @@ -220,31 +223,32 @@ static jfieldID parse_jcrumb_type(JNIEnv *env,

void bugsnag_leave_breadcrumb_env(JNIEnv *env, const char *message,
const bugsnag_breadcrumb_type type) {
bsg_jni_cache jni_cache;

jbyteArray jmessage = NULL;
jobject jtype = NULL;

if (!bsg_jni_cache_refresh(env)) {
if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
goto exit;
}

// get breadcrumb type fieldID
jfieldID crumb_type =
parse_jcrumb_type(env, type, bsg_global_jni_cache->breadcrumb_type);
jfieldID crumb_type = parse_jcrumb_type(env, type, jni_cache.breadcrumb_type);
if (crumb_type == NULL) {
goto exit;
}

// get the breadcrumb type
jtype = bsg_safe_get_static_object_field(
env, bsg_global_jni_cache->breadcrumb_type, crumb_type);
jtype = bsg_safe_get_static_object_field(env, jni_cache.breadcrumb_type,
crumb_type);
if (jtype == NULL) {
goto exit;
}
jmessage = bsg_byte_ary_from_string(env, message);
bsg_safe_call_static_void_method(env, bsg_global_jni_cache->native_interface,
bsg_global_jni_cache->ni_leave_breadcrumb,
jmessage, jtype);
bsg_safe_call_static_void_method(env, jni_cache.native_interface,
jni_cache.ni_leave_breadcrumb, jmessage,
jtype);

goto exit;

Expand Down
40 changes: 21 additions & 19 deletions bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,10 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
jboolean auto_detect_ndk_crashes, jint _api_level, jboolean is32bit,
jint send_threads) {

if (!bsg_jni_cache_refresh(env)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
bsg_jni_cache jni_cache;

if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI jni_cache.");
}

bsg_environment *bugsnag_env = calloc(1, sizeof(bsg_environment));
Expand Down Expand Up @@ -189,7 +191,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
}

// populate metadata from Java layer
bsg_populate_event(env, &bugsnag_env->next_event);
bsg_populate_event(env, &jni_cache, &bugsnag_env->next_event);
time(&bugsnag_env->start_time);
if (bugsnag_env->next_event.app.in_foreground) {
bugsnag_env->foreground_start_time = bugsnag_env->start_time;
Expand Down Expand Up @@ -224,18 +226,16 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
static pthread_mutex_t bsg_native_delivery_mutex = PTHREAD_MUTEX_INITIALIZER;
pthread_mutex_lock(&bsg_native_delivery_mutex);

bsg_jni_cache jni_cache;

const char *event_path = NULL;
bugsnag_event *event = NULL;
jbyteArray jpayload = NULL;
jbyteArray jstage = NULL;
char *payload = NULL;
jstring japi_key = NULL;

if (bsg_global_jni_cache == NULL) {
goto exit;
}

if (!bsg_jni_cache_refresh(env)) {
if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
goto exit;
}
Expand Down Expand Up @@ -277,10 +277,9 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
japi_key = bsg_safe_new_string_utf(env, event->api_key);
if (japi_key != NULL) {
bool is_launching = event->app.is_launching;
bsg_safe_call_static_void_method(env,
bsg_global_jni_cache->native_interface,
bsg_global_jni_cache->ni_deliver_report,
jstage, jpayload, japi_key, is_launching);
bsg_safe_call_static_void_method(env, jni_cache.native_interface,
jni_cache.ni_deliver_report, jstage,
jpayload, japi_key, is_launching);
}

exit:
Expand Down Expand Up @@ -362,10 +361,10 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_pausedSession(
JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb(
JNIEnv *env, jobject _this, jstring name_, jstring crumb_type,
jstring timestamp_, jobject metadata) {
if (bsg_global_env == NULL) {
return;
}
if (!bsg_jni_cache_refresh(env)) {

bsg_jni_cache jni_cache;

if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
return;
}
Expand Down Expand Up @@ -395,7 +394,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb(
crumb->type = BSG_CRUMB_MANUAL;
}

bsg_populate_crumb_metadata(env, crumb, metadata);
bsg_populate_crumb_metadata(env, &jni_cache, crumb, metadata);
request_env_write_lock();
bugsnag_event_add_breadcrumb(&bsg_global_env->next_event, crumb);
release_env_write_lock();
Expand Down Expand Up @@ -717,12 +716,15 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateMetadata(
if (bsg_global_env == NULL) {
return;
}
if (!bsg_jni_cache_refresh(env)) {

bsg_jni_cache jni_cache;
if (!bsg_jni_cache_init(env, &jni_cache)) {
BUGSNAG_LOG("Could not refresh JNI cache.");
return;
}
request_env_write_lock();
bsg_populate_metadata(env, &bsg_global_env->next_event.metadata, metadata);
bsg_populate_metadata(env, &jni_cache, &bsg_global_env->next_event.metadata,
metadata);
release_env_write_lock();
}

Expand Down
Loading