Make inspector executor safe to destroy on any thread

packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp

@@ -6,6 +6,7 @@
  */
 
 #include "ReactInstanceManagerInspectorTarget.h"
+#include "SafeReleaseJniRef.h"
 
 #include <fbjni/NativeRunnable.h>
 #include <jsinspector-modern/InspectorFlags.h>
@@ -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);

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 <fbjni/fbjni.h>
+
+#include <type_traits>
+#include <utility>
+
+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 <typename RefT>
+class SafeReleaseJniRef {
+ using T = std::remove_reference<decltype(*std::declval<RefT>())>::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

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp

@@ -8,6 +8,7 @@
 #include "JReactHostInspectorTarget.h"
 #include <fbjni/NativeRunnable.h>
 #include <jsinspector-modern/InspectorFlags.h>
+#include <react/jni/SafeReleaseJniRef.h>
 
 using namespace facebook::jni;
 using namespace facebook::react::jsinspector_modern;
@@ -23,7 +24,10 @@ JReactHostInspectorTarget::JReactHostInspectorTarget(
  inspectorTarget_ = HostTarget::create(
  *this,
  [javaExecutor =
- javaExecutor_](std::function<void()>&& callback) mutable {
+ // Use a SafeReleaseJniRef because this lambda may be copied to
+ // arbitrary threads.
+ SafeReleaseJniRef(javaExecutor_)](
+ std::function<void()>&& callback) mutable {
  auto jrunnable =
  JNativeRunnable::newObjectCxxArgs(std::move(callback));
  javaExecutor->execute(jrunnable);

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<HostTarget> create(
  HostTargetDelegate& delegate,