mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Stop accessing JVM in ~JavaTurboModule
Summary: Inside JavaTurboModule, the native `CallInvoker` is used to schedule work on the NativeModules thread. So, in ~JavaTurboModule(), I scheduled some work on the NativeModules thread. This work holds a copy of the JNI global reference to the Java NativeModule object, and when it's executed, it resets this global reference to the Java NativeModule object. This should ensure that the we don't access the JVM in ~JavaTurboModule, which could crash the program. I also removed the redundant `quitSynchronous()` in `~CatalystInstanceImpl()`, to prevent the NativeModules thread from being deleted before we delete the `jsi::Runtime`. This shouldn't cause an issue, because we delete the NativeModules thread when we call [ReactQueueConfigurationImpl.destroy()](https://fburl.com/codesearch/p7aurwn3). Changelog: [Internal] Reviewed By: ejanzer Differential Revision: D23744777 fbshipit-source-id: a5c8d3f2ac4287dfef9a4b4404a04b335aa0963d
This commit is contained in:
committed by
Facebook GitHub Bot
parent
c453dbc4cc
commit
9b094ee77a
@@ -92,12 +92,6 @@ CatalystInstanceImpl::initHybrid(jni::alias_ref<jclass>) {
|
||||
CatalystInstanceImpl::CatalystInstanceImpl()
|
||||
: instance_(std::make_unique<Instance>()) {}
|
||||
|
||||
CatalystInstanceImpl::~CatalystInstanceImpl() {
|
||||
if (moduleMessageQueue_ != NULL) {
|
||||
moduleMessageQueue_->quitSynchronous();
|
||||
}
|
||||
}
|
||||
|
||||
void CatalystInstanceImpl::registerNatives() {
|
||||
registerHybrid({
|
||||
makeNativeMethod("initHybrid", CatalystInstanceImpl::initHybrid),
|
||||
|
||||
@@ -37,7 +37,6 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
|
||||
"Lcom/facebook/react/bridge/CatalystInstanceImpl;";
|
||||
|
||||
static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jclass>);
|
||||
~CatalystInstanceImpl() override;
|
||||
|
||||
static void registerNatives();
|
||||
|
||||
|
||||
@@ -31,6 +31,26 @@ JavaTurboModule::JavaTurboModule(const InitParams ¶ms)
|
||||
instance_(jni::make_global(params.instance)),
|
||||
nativeInvoker_(params.nativeInvoker) {}
|
||||
|
||||
JavaTurboModule::~JavaTurboModule() {
|
||||
/**
|
||||
* TODO(T75896241): In E2E tests, instance_ is null. Investigate why. Can we
|
||||
* get rid of this null check?
|
||||
*/
|
||||
if (!instance_) {
|
||||
return;
|
||||
}
|
||||
|
||||
nativeInvoker_->invokeAsync([instance = std::move(instance_)]() mutable {
|
||||
/**
|
||||
* Reset the global NativeModule ref on the NativeModules thread. Why:
|
||||
* - ~JavaTurboModule() can be called on a non-JVM thread. If we reset the
|
||||
* global ref in ~JavaTurboModule(), we might access the JVM from a
|
||||
* non-JVM thread, which will crash the app.
|
||||
*/
|
||||
instance.reset();
|
||||
});
|
||||
}
|
||||
|
||||
bool JavaTurboModule::isPromiseAsyncDispatchEnabled_ = false;
|
||||
void JavaTurboModule::enablePromiseAsyncDispatch(bool enable) {
|
||||
isPromiseAsyncDispatchEnabled_ = enable;
|
||||
|
||||
@@ -41,6 +41,7 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
|
||||
};
|
||||
|
||||
JavaTurboModule(const InitParams ¶ms);
|
||||
virtual ~JavaTurboModule();
|
||||
jsi::Value invokeJavaMethod(
|
||||
jsi::Runtime &runtime,
|
||||
TurboModuleMethodValueKind valueKind,
|
||||
|
||||
Reference in New Issue
Block a user