diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp index c97907bc63de2d..1188971031872c 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp @@ -6,6 +6,7 @@ */ #include "ReactInstanceManagerInspectorTarget.h" +#include "SafeReleaseJniRef.h" #include #include @@ -39,7 +40,11 @@ ReactInstanceManagerInspectorTarget::ReactInstanceManagerInspectorTarget( if (inspectorFlags.getEnableModernCDPRegistry()) { inspectorTarget_ = HostTarget::create( - *this, [javaExecutor = make_global(executor)](auto callback) mutable { + *this, + [javaExecutor = + // Use a SafeReleaseJniRef because this lambda may be copied to + // arbitrary threads. + SafeReleaseJniRef(make_global(executor))](auto callback) mutable { auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(callback)); javaExecutor->execute(jrunnable); diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h new file mode 100644 index 00000000000000..65fb1f8502cd41 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/SafeReleaseJniRef.h @@ -0,0 +1,73 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include +#include + +namespace facebook::react { + +/** + * A wrapper around a JNI reference (e.g. jni::global_ref<>) that makes it safe + * to destroy (decrement refcount) from any thread. This wrapping is necessary + * when we don't have control over the thread where a given JNI reference's + * destructor will run. + * + * In particular, this is needed when a JNI reference is owned by a JSI + * HostObject or HostFunction, since those may be destroyed on an arbitrary + * thread (e.g. ther Hermes GC thread) which may not be attached to the JVM. + * (This is explicitly documented for HostObject, and is the observed behavior + * in Hermes for HostFunctions.) + */ +template +class SafeReleaseJniRef { + using T = std::remove_reference())>::type; + + public: + /* explicit */ SafeReleaseJniRef(RefT ref) : ref_(std::move(ref)) {} + SafeReleaseJniRef(const SafeReleaseJniRef& other) = default; + SafeReleaseJniRef(SafeReleaseJniRef&& other) = default; + SafeReleaseJniRef& operator=(const SafeReleaseJniRef& other) = default; + SafeReleaseJniRef& operator=(SafeReleaseJniRef&& other) = default; + + ~SafeReleaseJniRef() { + if (ref_) { + jni::ThreadScope ts; + ref_.reset(); + } + } + + operator bool() const noexcept { + return (bool)ref_; + } + + T& operator*() noexcept { + return *ref_; + } + + T* operator->() noexcept { + return &*ref_; + } + + const T& operator*() const noexcept { + return *ref_; + } + + const T* operator->() const noexcept { + return &*ref_; + } + + operator RefT() const { + return ref_; + } + + private: + RefT ref_; +}; + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp index dbcfa3f0a8b509..170baac926f8a7 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp @@ -8,6 +8,7 @@ #include "JReactHostInspectorTarget.h" #include #include +#include using namespace facebook::jni; using namespace facebook::react::jsinspector_modern; @@ -23,7 +24,10 @@ JReactHostInspectorTarget::JReactHostInspectorTarget( inspectorTarget_ = HostTarget::create( *this, [javaExecutor = - javaExecutor_](std::function&& callback) mutable { + // Use a SafeReleaseJniRef because this lambda may be copied to + // arbitrary threads. + SafeReleaseJniRef(javaExecutor_)]( + std::function&& callback) mutable { auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(callback)); javaExecutor->execute(jrunnable); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index 05fe80b97bce57..6e4fafbd30d2e6 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -162,6 +162,10 @@ class JSINSPECTOR_EXPORT HostTarget * \param executor An executor that may be used to call methods on this * HostTarget while it exists. \c create additionally guarantees that the * executor will not be called after the HostTarget is destroyed. + * \note Copies of the provided executor may be destroyed on arbitrary + * threads, including after the HostTarget is destroyed. Callers must ensure + * that such destructor calls are safe - e.g. if using a lambda as the + * executor, all captured values must be safe to destroy from any thread. */ static std::shared_ptr create( HostTargetDelegate& delegate,