facebook
Make inspector executor safe to destroy on any thread
Summary: Changelog: [Internal]
Fixes a crash that may happen on Android when the inspectorEnableModernCDPRegistry feature flag is true, and clarifies the documentation of HostTarget::create() to avoid similar issues in future integrations.
Context
The executor provided to HostTarget::create() ("the inspector executor") is used throughout the CDP backend to schedule work on the inspector thread. (See also D53356953.) To facilitate this, the executor is a copyable std::function.
On Android, the executor is backed by a Java object, to which we hold a reference from C++ using JNI. This reference is expressed as a facebook::jni::global_ref which gets copied as part of the executor. (global_ref is a RAII wrapper around the NewGlobalRef / DeleteGlobalRef JNI functions.)
The bug
All the calls to the inspector executor from C++ happen on threads that are already properly attached to the JVM ( = the UI thread / the JS thread) and are therefore allowed to make JNI calls. However, there are cases where a copy of the inspector executor may have its destructor run on an unexpected thread, namely the (Hermes) JS GC thread. (This happens when the executor itself is captured in a lambda that's stored in a jsi::HostObject or jsi::HostFunction, which is in turn destroyed on the JS GC thread.) In such cases, the DeleteGlobalRef call mentioned above will crash, because the JS GC thread is not attached to the JVM.
The fix
First, we document that copies of the inspector executor provided to HostTarget::create may indeed be destroyed on arbitrary threads. This is an unavoidable consequence of the existing design. Second, we adapt the Android integration to this requirement.
fbjni provides the jni::ThreadScope RAII helper to manage attaching C++ threads to the JVM. If we had any explicit control over the setup and teardown of the JS GC thread, we could create a single jni::ThreadScope to globally ensure the safety of JS-finalizer-to-JNI calls in React Native. However, neither JSI nor the Hermes API provides such control.
Instead, we essentially resort to creating a temporary ThreadScope around each DeleteGlobalRef call where we don't control the calling thread. We do this using SafeReleaseJniRef, a new kind of JNI reference wrapper class that does just this. Retaining a SafeReleaseJniRef instead of a plain global_ref is all that's needed to make a particular reference safe to destroy on any thread.
Differential Revision: D56620131
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 19,460,108 | +5 |
| android | hermes | armeabi-v7a | n/a | -- |
| android | hermes | x86 | n/a | -- |
| android | hermes | x86_64 | n/a | -- |
| android | jsc | arm64-v8a | 22,833,639 | +17 |
| android | jsc | armeabi-v7a | n/a | -- |
| android | jsc | x86 | n/a | -- |
| android | jsc | x86_64 | n/a | -- |
Base commit: be06fd4e22a500128d202436600381b8bc17b3f5 Branch: main
Merged as https://github.com/facebook/react-native/commit/8689f563ed8ed8c25b73f980927e323909e6673a
