Add hooks into HostObject destructors

cpp/includes/RustArcPtr.h

@@ -26,4 +26,78 @@ template <> struct Bridging<void *> {
     return uniffi_jsi::Bridging<uint64_t>::toJs(rt, callInvoker, num);
   }
 };
+
+/// Constructs a `DestructibleObject` that accepts a pointer and destructor
+/// callback. This corresponds to a `UniffiRustArcPtr` object. The idea is that
+/// this object has a C++ destructor which calls the Rust drop function
+/// for the object.
+///
+/// The JS object provides a `markDestroyed` function. This will override the
+/// behaviour of the C++ destructor.
+///
+/// This allows the generated JS to call the Rust object's `free` function
+/// in the usual manner, including checks for panics, which are translated
+/// into and thrown as JS errors.
+///
+/// It should be noted that this is a hack: the C++ destructor. The relevant
+/// comment on the HostObject's destructor is reproduced here:
+///
+///   // The C++ object's dtor will be called when the GC finalizes this
+///   // object.  (This may be as late as when the Runtime is shut down.)
+///   // You have no control over which thread it is called on.  This will
+///   // be called from inside the GC…
+///
+/// Until hermes and React Native gain a `FinalizationRegistry`, this is
+/// unlikely to get better.
+class DestructibleObject : public jsi::HostObject {
+private:
+  jsi::Runtime &rt;
+  std::mutex destructorMutex;
+  bool isDestroyed = false;
+  uint64_t pointer;
+  std::function<void(uint64_t)> destructor;
+
+public:
+  DestructibleObject(jsi::Runtime &rt, uint64_t pointer,
+                     std::function<void(uint64_t)> destructor)
+      : jsi::HostObject(), rt(rt), pointer(pointer), destructor(destructor) {}
+
+  ~DestructibleObject() {
+    /// You can use uniffiDestroy() from JS instead of relying on GC.
+    ///
+    /// From
+    /// https://github.com/facebook/hermes/issues/982#issuecomment-1771325667:
+    ///
+    /// "Generally speaking, when dealing with native resources, it is
+    /// recommended to free the native resource explicitly when you are done
+    /// with it, because garbage collection is unpredictable and should not be
+    /// relied on for freeing native resources timely."
+    destroy_object();
+  }
+
+  void destroy_object() {
+    std::lock_guard<std::mutex> lock(destructorMutex);
+    if (!isDestroyed) {
+      isDestroyed = true;
+      destructor(pointer);
+    }
+  }
+
+  jsi::Value get(jsi::Runtime &rt, const jsi::PropNameID &name) override {
+    std::string propName = name.utf8(rt);
+    if (propName == "markDestroyed") {
+      auto func = jsi::Function::createFromHostFunction(
+          rt, jsi::PropNameID::forAscii(rt, "markDestroyed"), 0,
+          [this](jsi::Runtime &rt, const jsi::Value &thisVal,
+                 const jsi::Value *args, size_t count) -> jsi::Value {
+            std::lock_guard<std::mutex> lock(this->destructorMutex);
+            this->isDestroyed = true;
+            return jsi::Value::undefined();
+          });
+      return jsi::Value(rt, func);
+    }
+    return jsi::Value::undefined();
+  }
+};
+
 } // namespace uniffi_jsi

crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/mod.rs

@@ -9,7 +9,7 @@ use crate::bindings::{
     metadata::ModuleMetadata,
     react_native::{
         ComponentInterfaceExt, FfiArgumentExt, FfiCallbackFunctionExt, FfiFieldExt, FfiStructExt,
-        FfiTypeExt,
+        FfiTypeExt, ObjectExt,
     },
 };
 use anyhow::Result;

crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/ObjectHelper.cpp

@@ -0,0 +1,18 @@
+{%- import "macros.cpp" as cpp %}
+
+{%- for obj in ci.object_definitions() %}
+{%- let bless = obj.ffi_function_bless_pointer() %}
+{%- let free = obj.ffi_object_free() %}
+{%- let uint64 = FfiType::UInt64 %}
+{%- call cpp::cpp_fn_from_js_decl(bless) %} {
+    auto pointer = {{ uint64|bridging_class(ci) }}::fromJs(rt, callInvoker, args[0]);
+    auto static destructor = [](uint64_t p) {
+        auto pointer = reinterpret_cast<void *>(static_cast<uintptr_t>(p));
+        RustCallStatus status = {0};
+        {{ free.name() }}(pointer, &status);
+    };
+    auto ptrObj = std::make_shared<{{ ci.cpp_namespace_includes() }}::DestructibleObject>(rt, pointer, destructor);
+    auto obj = jsi::Object::createFromHostObject(rt, ptrObj);
+    return jsi::Value(rt, obj);
+}
+{%- endfor %}

crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/VTableCallbackFunction.cpp

@@ -117,6 +117,19 @@ namespace {{ ns }} {
             {%- if callback.has_rust_call_status_arg() -%}
             , RustCallStatus* uniffi_call_status
             {%- endif -%}) {
+        // If the runtime has shutdown, then there is no point in trying to
+        // call into Javascript. BUT how do we tell if the runtime has shutdown?
+        //
+        // Answer: the module destructor calls into callback `cleanup` method,
+        // which nulls out the rsLamda.
+        //
+        // If rsLamda is null, then there is no runtime to call into.
+        if (rsLambda == nullptr) {
+            // This only occurs when destructors are calling into Rust free/drop,
+            // which causes the JS callback to be dropped.
+            return;
+        }
+
         // The runtime, the actual callback jsi::funtion, and the callInvoker
         // are all in the lambda.
         rsLambda(

crates/ubrn_bindgen/src/bindings/react_native/gen_cpp/templates/wrapper.cpp

@@ -144,6 +144,7 @@ void {{ module_name }}::set(jsi::Runtime& rt, const jsi::PropNameID& name, const
 }
 
 {%- include "StringHelper.cpp" %}
+{%- include "ObjectHelper.cpp" %}
 
 // Methods calling directly into the uniffi generated C API of the Rust crate.
 {%- for func in ci.iter_ffi_functions_js_to_rust() %}

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/mod.rs

@@ -30,7 +30,7 @@ use uniffi_meta::{AsType, ExternalKind};
 
 use crate::bindings::metadata::ModuleMetadata;
 use crate::bindings::react_native::{
-    ComponentInterfaceExt, FfiCallbackFunctionExt, FfiFunctionExt, FfiStructExt,
+    ComponentInterfaceExt, FfiCallbackFunctionExt, FfiFunctionExt, FfiStructExt, ObjectExt,
 };
 
 #[derive(Default)]

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/oracle.rs

@@ -129,6 +129,7 @@ impl CodeOracle {
 
     pub(crate) fn ffi_type_label_for_cpp(&self, ffi_type: &FfiType) -> String {
         match ffi_type {
+            FfiType::RustArcPtr(_) => "UniffiRustArcPtr".to_string(),
             FfiType::ForeignBytes => "ArrayBuffer".to_string(),
             FfiType::RustBuffer(_) => "string".to_string(),
             _ => self.ffi_type_label(ffi_type),

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/ObjectTemplate.ts

@@ -13,6 +13,7 @@
 {%- macro private_ctor() %}
 private constructor(pointer: UnsafeMutableRawPointer) {
     super();
+    this.__rustPointer = pointer;
     this.__rustArcPtr = {{ obj_factory }}.bless(pointer);
 }
 {%- endmacro %}
@@ -33,6 +34,7 @@ export class {{ impl_class_name }} extends AbstractUniffiObject implements {{ pr
 
     private __uniffiTypeName = "{{ impl_class_name }}";
     private __rustArcPtr: UniffiRustArcPtr;
+    private __rustPointer: UnsafeMutableRawPointer;
 
     {%- match obj.primary_constructor() %}
     {%- when Some with (cons) %}
@@ -65,7 +67,8 @@ export class {{ impl_class_name }} extends AbstractUniffiObject implements {{ pr
     uniffiDestroy(): void {
         if ((this as any).__rustArcPtr) {
             const pointer = {{ obj_factory }}.pointer(this);
-            this.__rustArcPtr.d(pointer);
+            {{ obj_factory }}.freePointer(pointer);
+            this.__rustArcPtr.markDestroyed();
             delete (this as any).__rustArcPtr;
         }
     }
@@ -106,21 +109,24 @@ export class {{ impl_class_name }} extends AbstractUniffiObject implements {{ pr
 const {{ obj_factory }}: UniffiObjectFactory<{{ type_name }}> = {
     create(pointer: UnsafeMutableRawPointer): {{ type_name }} {
         const instance = Object.create({{ impl_class_name }}.prototype);
+        instance.__rustPointer = pointer;
         instance.__rustArcPtr = this.bless(pointer);
         return instance;
     },
 
     bless(p: UnsafeMutableRawPointer): UniffiRustArcPtr {
-        const d = this.freePointer;
-        return { p, d };
+        return rustCall(
+            /*caller:*/ (status) =>
+                nativeModule().{{ obj.ffi_function_bless_pointer().name() }}(p, status),
+            /*liftString:*/ FfiConverterString.lift
+        );
     },
 
     pointer(obj: {{ type_name }}): UnsafeMutableRawPointer {
-        const ptr = (obj as any).__rustArcPtr;
-        if (ptr === undefined) {
+        if ((obj as any).__rustArcPtr === undefined) {
             throw new UniffiInternalError.UnexpectedNullPointer();
         }
-        return ptr.p;
+        return (obj as any).__rustPointer;
     },
 
     clonePointer(obj: {{ type_name }}): UnsafeMutableRawPointer {

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/macros.ts

@@ -93,6 +93,7 @@
         super();
         const pointer =
             {% call to_ffi_method_call(obj_factory, callable) %};
+        this.__rustPointer = pointer;
         this.__rustArcPtr = {{ obj_factory }}.bless(pointer);
     }
 {%- endmacro %}

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/wrapper-ffi.ts

@@ -1,6 +1,7 @@
 import {
   type StructuralEquality as UniffiStructuralEquality,
   type UniffiReferenceHolder,
+  type UniffiRustArcPtr,
   type UniffiRustCallStatus,
   type UniffiRustFutureContinuationCallback as RuntimeUniffiRustFutureContinuationCallback,
  } from 'uniffi-bindgen-react-native';

crates/ubrn_bindgen/src/bindings/react_native/gen_typescript/templates/wrapper.ts

@@ -1,5 +1,6 @@
 // This file was autogenerated by some hot garbage in the `uniffi` crate.
 // Trust me, you don't want to mess with it!
+import { console } from "@/hermes";
 
 import nativeModule, {
   {%- for def in ci.iter_ffi_definitions_exported_by_ts() %}

crates/ubrn_bindgen/src/bindings/react_native/mod.rs

@@ -18,7 +18,7 @@ use ubrn_common::{fmt, run_cmd_quietly};
 use uniffi_bindgen::{
     interface::{
         FfiArgument, FfiCallbackFunction, FfiDefinition, FfiField, FfiFunction, FfiStruct, FfiType,
-        Function,
+        Function, Method, Object,
     },
     BindingGenerator, BindingsConfig, ComponentInterface,
 };
@@ -206,6 +206,7 @@ impl ComponentInterface {
         self.iter_ffi_functions_js_to_cpp_and_back()
             .chain(self.iter_ffi_functions_js_to_rust())
             .chain(self.iter_ffi_functions_init_callback())
+            .chain(self.iter_ffi_function_bless_pointer())
     }
 
     fn iter_ffi_functions_js_to_rust(&self) -> impl Iterator<Item = FfiFunction> {
@@ -235,6 +236,12 @@ impl ComponentInterface {
             }))
     }
 
+    fn iter_ffi_function_bless_pointer(&self) -> impl Iterator<Item = FfiFunction> {
+        self.object_definitions()
+            .iter()
+            .map(|o| o.ffi_function_bless_pointer())
+    }
+
     fn iter_ffi_structs(&self) -> impl Iterator<Item = FfiStruct> {
         self.ffi_definitions().filter_map(|s| match s {
             FfiDefinition::Struct(s) => Some(s),
@@ -351,6 +358,31 @@ fn store_with_name(types: &mut HashMap<String, Type>, type_: &Type) -> String {
     name
 }
 
+#[ext]
+impl Object {
+    fn ffi_function_bless_pointer(&self) -> FfiFunction {
+        let meta = uniffi_meta::MethodMetadata {
+            module_path: "internal".to_string(),
+            self_name: self.name().to_string(),
+            name: "ffi__bless_pointer".to_owned(),
+            is_async: false,
+            inputs: Default::default(),
+            return_type: None,
+            throws: None,
+            checksum: None,
+            docstring: None,
+            takes_self_by_arc: false,
+        };
+        let func: Method = meta.into();
+        let mut ffi = func.ffi_func().clone();
+        ffi.init(
+            Some(FfiType::RustArcPtr(String::from(""))),
+            vec![FfiArgument::new("pointer", FfiType::UInt64)],
+        );
+        ffi
+    }
+}
+
 #[ext]
 impl FfiFunction {
     fn is_internal(&self) -> bool {

fixtures/callbacks/tests/bindings/test_callbacks.ts

@@ -170,6 +170,7 @@ class StoredTypeScriptStringifier implements StoredForeignStringifier {
     }
   }
 }
+
 test("A callback passed into the constructor", (t) => {
   const stringifierCallback = new StoredTypeScriptStringifier();
   const rustStringifier = new RustStringifier(stringifierCallback);
@@ -178,4 +179,15 @@ test("A callback passed into the constructor", (t) => {
   const observed = rustStringifier.fromSimpleType(42);
 
   t.assertEqual(observed, expected);
+
+  rustStringifier.uniffiDestroy();
+});
+
+test("A callback passed into the constructor is not destroyed", (t) => {
+  const stringifierCallback = new StoredTypeScriptStringifier();
+  const rustStringifier = new RustStringifier(stringifierCallback);
+
+  // The destructor calls into the Rust, which calls back into Javascript.
+  // If the jsi::Runtime has been destroyed already, then this will cause a
+  // crash at the end of the run. This should be prevented.
 });

fixtures/coverall/tests/bindings/test_coverall.ts

@@ -90,6 +90,26 @@ test("test create_none_dict() with default values", (t) => {
   t.assertEqual(d.coveralls, undefined);
 });
 
+test("Given a rust object, when it is destroyed, it cannot be re-used", (t) => {
+  const name = "To be destroyed";
+  const c = new Coveralls(name);
+
+  // check it works first.
+  t.assertEqual(name, c.getName());
+
+  // then destroy it.
+  c.uniffiDestroy();
+
+  // now check it doesn't work.
+  t.assertThrows(
+    () => true,
+    () => c.getName(),
+  );
+
+  // …destroy again, just to show it's idempotent.
+  c.uniffiDestroy();
+});
+
 test("Given 1000 objects, when they go out of scope, then they are dropped by rust", (t) => {
   // The GC test; we should have 1000 alive by the end of the loop.
   //
@@ -249,3 +269,9 @@ test("Dummy coveralls implement the Coveralls interface", (t) => {
   const extendedCoveralls = new ExtendedCoveralls("Extended coveralls");
   t.assertTrue(Coveralls.instanceOf(extendedCoveralls));
 });
+
+// This is the last test, so we can test a graceful shutdown.
+test("Given a rust object, if not destroyed, it's ok", (t) => {
+  const name = "To be destroyed by GC";
+  const c = new Coveralls(name);
+});

typescript/src/rust-call.ts

@@ -98,8 +98,5 @@ function uniffiCheckCallStatus(
 export type UniffiRustArcPtrDestructor = (pointer: bigint) => void;
 
 export type UniffiRustArcPtr = {
-  // pointer
-  p: UnsafeMutableRawPointer;
-  // destructor
-  d: UniffiRustArcPtrDestructor;
+  markDestroyed(): void;
 };