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

fix(android): Avoid using AndroidJavaObject from background threads #161

Merged
merged 1 commit into from
Aug 20, 2019

Conversation

kattrali
Copy link
Contributor

@kattrali kattrali commented Aug 14, 2019

Goal

Work around an issue present in Unity 2017/2018 where instantiating AndroidJavaObject or using AndroidJavaObject.Call() (event indirectly using AndroidJNI helpers) crashes when called from a background thread on x86. This change removes use of JNI APIs when on background threads and running on x86 Android.

Changeset

Added a reference to the main thread to the Android NativeInterface class, which is checked by functions before invoking the JNI, exiting early if the current thread is not the main thread.

Tests

Tested manually in Unity 2018 / Unity 2017 by calling Notify(), logging, leaving breadcrumbs, and causing unhandled crashes from C#, Java, and C++ code and verifying:

  • the report contents match before and after the change on arm7/arm64 devices
  • the report contents match before and after the change on x86 devices on the main thread
  • attempted to log / Notify() / report a crash does not crash the app on background threads on Android x86 (though no report is sent)

The crash is known to affect:

  • 2017.4.30f1
  • 2018.4.2f1
  • 2018.4.3f1
  • 2019.1.6f1

The Android x86 target no longer builds on 2019.2 (possibly related to its deprecation), and the target is removed altogether in 2019.3 (announcement, release notes).

Builds

Bugsnag.unitypackage.zip

md5 31242f4a72103c68f61d54606f732a9c
8308d01be795a00fd7877eb516e8a2175279de0d

Bugsnag-with-android-64bit.unitypackage.zip

md5 099c31d986190ae73236229d297a1395
sha 9b1d3423023988dad05183ca2f35d6ff19c5de10

@kattrali kattrali force-pushed the kattrali/workaround-unity-2018.2-jni-crash branch 2 times, most recently from 996de47 to fc9f31a Compare August 16, 2019 09:18
Copy link
Contributor

@fractalwrench fractalwrench left a comment

Choose a reason for hiding this comment

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

I think it'd be worth adding a similar check in CallNativeObjectMethod. This method is used quite widely and it seems possible that someone might add a new method that doesn't use the JNI apart from via this, and forgets to add a CanRunJni check.

@kattrali kattrali force-pushed the kattrali/workaround-unity-2018.2-jni-crash branch from fc9f31a to 917417f Compare August 19, 2019 15:30
@kattrali
Copy link
Contributor Author

🤔 Yeah that makes sense. Updated.

@fractalwrench fractalwrench self-requested a review August 19, 2019 15:32
@kattrali kattrali merged commit ed34745 into master Aug 20, 2019
@kattrali kattrali deleted the kattrali/workaround-unity-2018.2-jni-crash branch August 20, 2019 09:24
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.

None yet

2 participants