Skip to content

Commit

Permalink
Merge pull request #1585 from bugsnag/PLAT-7876/threadsafe-jni-cache
Browse files Browse the repository at this point in the history 
Don't attempt to share the `bsg_jni_cache` between threads
  • Loading branch information
@lemnik
lemnik authored Jan 21, 2022
2 parents eb0e886 + 64795b4 commit 36daff6
Show file tree
Hide file tree
Showing 7 changed files with 360 additions and 357 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
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
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
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

0 comments on commit 36daff6

Please sign in to comment.