[modules][Android] Fix `JSCRuntime destroyed with a dangling API obje…

…ct` (#19487) # Why Closes #19221. # How In the current memory modal, all of our JSI references are cleared when GC runs through java unused objects. It turns out that is not the best flow, because JSC has an assertion that checks if all references were cleared. So we have to remove dangling references before the JSC runtime fully deallocates. # Test Plan - bare-expo with JSC and Heremes ✅
expo · Oct 11, 2022 · 33ae33e · 33ae33e
1 parent 6ecf7ab
commit 33ae33e
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 0 deletions.
diff --git a/packages/expo-modules-core/CHANGELOG.md b/packages/expo-modules-core/CHANGELOG.md
@@ -51,6 +51,7 @@
 - Update gradle excludes to fix detox tests. ([#19254](https://github.com/expo/expo/pull/19254) by [@esamelson](https://github.com/esamelson))
 - Fixed event listeners do not work when running with remote debugging mode on iOS. ([#19211](https://github.com/expo/expo/pull/19211) by [@kudo](https://github.com/kudo))
 - Use shared C++ runtime to reduce library size on Android. ([#19372](https://github.com/expo/expo/pull/19372) by [@kudo](https://github.com/kudo))
+- Fixed `JSCRuntime destroyed with a dangling API object` on Android. ([#19487](https://github.com/expo/expo/pull/19487) by [@lukmccall](https://github.com/lukmccall))
 
 ### 💡 Others
 

diff --git a/packages/expo-modules-core/android/src/main/cpp/ExpoModulesHostObject.cpp b/packages/expo-modules-core/android/src/main/cpp/ExpoModulesHostObject.cpp
@@ -12,6 +12,14 @@ namespace expo {
 ExpoModulesHostObject::ExpoModulesHostObject(JSIInteropModuleRegistry *installer)
   : installer(installer) {}
 
+/**
+ * Clears jsi references held by JSRegistry and JavaScriptRuntime. 
+ */
+ExpoModulesHostObject::~ExpoModulesHostObject() {
+  installer->jsRegistry.reset();
+  installer->runtimeHolder.reset();
+}
+
 jsi::Value ExpoModulesHostObject::get(jsi::Runtime &runtime, const jsi::PropNameID &name) {
   auto cName = name.utf8(runtime);
   auto module = installer->getModule(cName);

diff --git a/packages/expo-modules-core/android/src/main/cpp/ExpoModulesHostObject.h b/packages/expo-modules-core/android/src/main/cpp/ExpoModulesHostObject.h
@@ -20,6 +20,8 @@ class ExpoModulesHostObject : public jsi::HostObject {
 public:
   ExpoModulesHostObject(JSIInteropModuleRegistry *installer);
 
+  ~ExpoModulesHostObject() override;
+
   jsi::Value get(jsi::Runtime &, const jsi::PropNameID &name) override;
 
   void set(jsi::Runtime &, const jsi::PropNameID &name, const jsi::Value &value) override;

diff --git a/packages/expo-modules-core/android/src/main/cpp/JavaScriptModuleObject.cpp b/packages/expo-modules-core/android/src/main/cpp/JavaScriptModuleObject.cpp
@@ -135,6 +135,18 @@ void JavaScriptModuleObject::registerProperty(
 JavaScriptModuleObject::HostObject::HostObject(
   JavaScriptModuleObject *jsModule) : jsModule(jsModule) {}
 
+/**
+ * Clears all the JSI references held by the `JavaScriptModuleObject`.
+ */
+JavaScriptModuleObject::HostObject::~HostObject() {
+  if (jsModule->jsiObject != nullptr) {
+    jsModule->jsiObject.reset();
+  }
+  jsModule->methodsMetadata.clear();
+  jsModule->constants.clear();
+  jsModule->properties.clear();
+}
+
 jsi::Value JavaScriptModuleObject::HostObject::get(jsi::Runtime &runtime,
                                                    const jsi::PropNameID &name) {
   auto cName = name.utf8(runtime);

diff --git a/packages/expo-modules-core/android/src/main/cpp/JavaScriptModuleObject.h b/packages/expo-modules-core/android/src/main/cpp/JavaScriptModuleObject.h
@@ -104,6 +104,8 @@ class JavaScriptModuleObject : public jni::HybridClass<JavaScriptModuleObject> {
   public:
     HostObject(JavaScriptModuleObject *);
 
+  ~HostObject() override;
+
     jsi::Value get(jsi::Runtime &, const jsi::PropNameID &name) override;
 
     void set(jsi::Runtime &, const jsi::PropNameID &name, const jsi::Value &value) override;