From 5f75b9e8dc8863fd38f13026253ccdcefd262898 Mon Sep 17 00:00:00 2001
From: Marc Rousavy <me@mrousavy.com>
Date: Tue, 30 Jan 2024 16:51:09 +0100
Subject: [PATCH] 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
---
 package/android/src/main/cpp/MutableJByteBuffer.cpp   |  5 ++++-
 .../src/main/cpp/frameprocessor/FrameHostObject.cpp   |  7 +++----
 .../frameprocessor/FrameProcessorPluginHostObject.cpp |  3 +++
 .../src/main/cpp/frameprocessor/VisionCameraProxy.cpp | 11 ++++++++++-
 .../src/main/cpp/frameprocessor/VisionCameraProxy.h   |  4 +---
 5 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/package/android/src/main/cpp/MutableJByteBuffer.cpp b/package/android/src/main/cpp/MutableJByteBuffer.cpp
index 080f8a5..b4d5695 100644
--- 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
\ No newline at end of file
+} // namespace vision
diff --git a/package/android/src/main/cpp/frameprocessor/FrameHostObject.cpp b/package/android/src/main/cpp/frameprocessor/FrameHostObject.cpp
index 27265fd..773cde6 100644
--- 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
index ecd5983..075232a 100644
--- 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
index 73dcf85..397fe79 100644
--- 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
index ab086f7..fe18c53 100644
--- a/package/android/src/main/cpp/frameprocessor/VisionCameraProxy.h
+++ b/package/android/src/main/cpp/frameprocessor/VisionCameraProxy.h
@@ -38,9 +38,7 @@ private:
 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);
 };