fix: Fix HostObject destructors to make sure a JNI Environment is set…

… up (#2462) * fix: Fix HostObject destructors to make sure a JNI Environment is set up * Use `reset` instead of `= nullptr` * Format * Format
mrousavy · Jan 30, 2024 · 5f75b9e · 5f75b9e
1 parent 9e12975
commit 5f75b9e
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 9 deletions.
diff --git a/package/android/src/main/cpp/MutableJByteBuffer.cpp b/package/android/src/main/cpp/MutableJByteBuffer.cpp
@@ -15,6 +15,9 @@ MutableJByteBuffer::MutableJByteBuffer(jni::alias_ref<jni::JByteBuffer> byteBuff
 }
 
 MutableJByteBuffer::~MutableJByteBuffer() noexcept {
+  // Hermes GC might destroy HostObjects on an arbitrary Thread which might not be
+  // connected to the JNI environment. To make sure fbjni can properly destroy
+  // the Java method, we connect to a JNI environment first.
   jni::ThreadScope::WithClassLoader([&] { _byteBuffer.reset(); });
 }
 
@@ -30,4 +33,4 @@ jni::global_ref<jni::JByteBuffer> MutableJByteBuffer::getByteBuffer() {
   return _byteBuffer;
 }
 
-} // namespace vision
+} // namespace vision
diff --git a/package/android/src/main/cpp/frameprocessor/FrameHostObject.cpp b/package/android/src/main/cpp/frameprocessor/FrameHostObject.cpp
@@ -22,10 +22,9 @@ using namespace facebook;
 FrameHostObject::FrameHostObject(const jni::alias_ref<JFrame::javaobject>& frame) : frame(make_global(frame)) {}
 
 FrameHostObject::~FrameHostObject() {
-  // Hermes' Garbage Collector (Hades GC) calls destructors on a separate Thread
-  // which might not be attached to JNI. Ensure that we use the JNI class loader when
-  // deallocating the `frame` HybridClass, because otherwise JNI cannot call the Java
-  // destroy() function.
+  // Hermes GC might destroy HostObjects on an arbitrary Thread which might not be
+  // connected to the JNI environment. To make sure fbjni can properly destroy
+  // the Java method, we connect to a JNI environment first.
   jni::ThreadScope::WithClassLoader([&] { frame.reset(); });
 }
 

diff --git a/package/android/src/main/cpp/frameprocessor/FrameProcessorPluginHostObject.cpp b/package/android/src/main/cpp/frameprocessor/FrameProcessorPluginHostObject.cpp
@@ -16,6 +16,9 @@ FrameProcessorPluginHostObject::FrameProcessorPluginHostObject(jni::alias_ref<JF
     : _plugin(make_global(plugin)) {}
 
 FrameProcessorPluginHostObject::~FrameProcessorPluginHostObject() {
+  // Hermes GC might destroy HostObjects on an arbitrary Thread which might not be
+  // connected to the JNI environment. To make sure fbjni can properly destroy
+  // the Java method, we connect to a JNI environment first.
   jni::ThreadScope::WithClassLoader([&] { _plugin.reset(); });
 }
 

diff --git a/package/android/src/main/cpp/frameprocessor/VisionCameraProxy.cpp b/package/android/src/main/cpp/frameprocessor/VisionCameraProxy.cpp
@@ -30,7 +30,12 @@ VisionCameraProxy::VisionCameraProxy(const jni::alias_ref<JVisionCameraProxy::ja
   _javaProxy = make_global(javaProxy);
 }
 
-VisionCameraProxy::~VisionCameraProxy() {}
+VisionCameraProxy::~VisionCameraProxy() {
+  // Hermes GC might destroy HostObjects on an arbitrary Thread which might not be
+  // connected to the JNI environment. To make sure fbjni can properly destroy
+  // the Java method, we connect to a JNI environment first.
+  jni::ThreadScope::WithClassLoader([&] { _javaProxy.reset(); });
+}
 
 std::vector<jsi::PropNameID> VisionCameraProxy::getPropertyNames(jsi::Runtime& runtime) {
   std::vector<jsi::PropNameID> result;
@@ -96,6 +101,10 @@ jsi::Value VisionCameraProxy::get(jsi::Runtime& runtime, const jsi::PropNameID&
   return jsi::Value::undefined();
 }
 
+void VisionCameraInstaller::registerNatives() {
+  javaClassStatic()->registerNatives({makeNativeMethod("install", VisionCameraInstaller::install)});
+}
+
 void VisionCameraInstaller::install(jni::alias_ref<jni::JClass>, jni::alias_ref<JVisionCameraProxy::javaobject> proxy) {
   // global.VisionCameraProxy
   auto visionCameraProxy = std::make_shared<VisionCameraProxy>(proxy);

diff --git a/package/android/src/main/cpp/frameprocessor/VisionCameraProxy.h b/package/android/src/main/cpp/frameprocessor/VisionCameraProxy.h
@@ -38,9 +38,7 @@ class VisionCameraProxy : public jsi::HostObject {
 class VisionCameraInstaller : public jni::JavaClass<VisionCameraInstaller> {
 public:
   static auto constexpr kJavaDescriptor = "Lcom/mrousavy/camera/frameprocessor/VisionCameraInstaller;";
-  static void registerNatives() {
-    javaClassStatic()->registerNatives({makeNativeMethod("install", VisionCameraInstaller::install)});
-  }
+  static void registerNatives();
   static void install(jni::alias_ref<jni::JClass> clazz, jni::alias_ref<JVisionCameraProxy::javaobject> proxy);
 };