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

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

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

lemnik
Copy link
Contributor

@lemnik lemnik commented Jan 21, 2022

Goal

Fix SIGABRT crashes caused by multiple threads reading and writing to the global bsg_jni_cache simultaneously.

Design

Replace the global JNI cache with a single cache for each JNI function call. A stack-allocated cache is populated on-entry to the NDK layer and a pointer to the cache is then passed into each function that requires it. This avoids any allocation errors, and ensures that JNI objects don't accidentally cross thread-boundaries.

Testing

Manual testing with an app known to cause this issue. Due to the race-condition nature of this crash, no additional automated tests can be added at this time.

@lemnik lemnik requested a review from kstenerud January 21, 2022 11:46
@lemnik lemnik force-pushed the PLAT-7876/threadsafe-jni-cache branch from 581c764 to fb806f1 Compare January 21, 2022 11:46
if (!bsg_jni_cache_refresh(env)) {
bsg_jni_cache jni_cache;

if (!bsg_jni_cache_refresh(env, &jni_cache)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name doesn't really describe it properly anymore. It's not refreshing the cache, but rather filling it completely. Can we change it?

@lemnik lemnik force-pushed the PLAT-7876/threadsafe-jni-cache branch 2 times, most recently from a581ac3 to 9f49860 Compare January 21, 2022 11:57
@lemnik lemnik force-pushed the PLAT-7876/threadsafe-jni-cache branch from 9f49860 to 64795b4 Compare January 21, 2022 12:05
@bugsnagbot
Copy link
Collaborator

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1431.41 1185.52
arm64_v8a 545.16 303.5
armeabi -492.59 -21.55
armeabi_v7a 520.59 274.83
x86 586.11 344.44
x86_64 569.73 328.07

Generated by 🚫 Danger

@lemnik lemnik merged commit 36daff6 into next Jan 21, 2022
@lemnik lemnik deleted the PLAT-7876/threadsafe-jni-cache branch January 21, 2022 13:51
@lemnik lemnik mentioned this pull request Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants