fix(cli/napi): handle finalizers (#19168)

Fixes https://github.com/denoland/deno/issues/17325

cli/napi/js_native_api.rs

@@ -497,18 +497,21 @@ fn napi_create_range_error(
 
 #[napi_sym::napi_sym]
 fn napi_create_external(
-  env: *mut Env,
+  env_ptr: *mut Env,
   value: *mut c_void,
-  _finalize_cb: napi_finalize,
+  finalize_cb: napi_finalize,
-  _finalize_hint: *mut c_void,
+  finalize_hint: *mut c_void,
   result: *mut napi_value,
 ) -> Result {
-  check_env!(env);
+  check_env!(env_ptr);
-  let env = unsafe { &mut *env };
+  let env = unsafe { &mut *env_ptr };
-  let value: v8::Local<v8::Value> =
+
+  let external: v8::Local<v8::Value> =
     v8::External::new(&mut env.scope(), value).into();
-  // TODO: finalization
+
-  *result = value.into();
+  let value = weak_local(env_ptr, external, value, finalize_cb, finalize_hint);
+
+  *result = transmute(value);
   Ok(())
 }
 
@@ -517,6 +520,7 @@ pub type BackingStoreDeleterCallback = unsafe extern "C" fn(
   byte_length: usize,
   deleter_data: *mut c_void,
 );
+
 extern "C" {
   fn v8__ArrayBuffer__NewBackingStore__with_data(
     data: *mut c_void,
@@ -526,69 +530,104 @@ extern "C" {
   ) -> *mut BackingStore;
 }
 
+struct BufferFinalizer {
+  env: *mut Env,
+  finalize_cb: napi_finalize,
+  finalize_data: *mut c_void,
+  finalize_hint: *mut c_void,
+}
+
+impl BufferFinalizer {
+  fn into_raw(self) -> *mut BufferFinalizer {
+    Box::into_raw(Box::new(self))
+  }
+}
+
+impl Drop for BufferFinalizer {
+  fn drop(&mut self) {
+    unsafe {
+      (self.finalize_cb)(self.env as _, self.finalize_data, self.finalize_hint);
+    }
+  }
+}
+
 pub extern "C" fn backing_store_deleter_callback(
   data: *mut c_void,
-  byte_length: usize,
+  _byte_length: usize,
-  _deleter_data: *mut c_void,
+  deleter_data: *mut c_void,
 ) {
-  let slice_ptr = ptr::slice_from_raw_parts_mut(data as *mut u8, byte_length);
+  let mut finalizer =
-  let b = unsafe { Box::from_raw(slice_ptr) };
+    unsafe { Box::from_raw(deleter_data as *mut BufferFinalizer) };
-  drop(b);
+
+  finalizer.finalize_data = data;
 }
 
 #[napi_sym::napi_sym]
 fn napi_create_external_arraybuffer(
-  env: *mut Env,
+  env_ptr: *mut Env,
   data: *mut c_void,
   byte_length: usize,
-  _finalize_cb: napi_finalize,
+  finalize_cb: napi_finalize,
   finalize_hint: *mut c_void,
   result: *mut napi_value,
 ) -> Result {
-  check_env!(env);
+  check_env!(env_ptr);
-  let env = unsafe { &mut *env };
+  let env = unsafe { &mut *env_ptr };
-  let _slice = std::slice::from_raw_parts(data as *mut u8, byte_length);
+
-  // TODO: finalization
+  let finalizer = BufferFinalizer {
+    env: env_ptr,
+    finalize_data: ptr::null_mut(),
+    finalize_cb,
+    finalize_hint,
+  };
+
   let store: UniqueRef<BackingStore> =
     transmute(v8__ArrayBuffer__NewBackingStore__with_data(
       data,
       byte_length,
       backing_store_deleter_callback,
-      finalize_hint,
+      finalizer.into_raw() as _,
     ));
 
   let ab =
     v8::ArrayBuffer::with_backing_store(&mut env.scope(), &store.make_shared());
   let value: v8::Local<v8::Value> = ab.into();
+
   *result = value.into();
   Ok(())
 }
 
 #[napi_sym::napi_sym]
 fn napi_create_external_buffer(
-  env: *mut Env,
+  env_ptr: *mut Env,
-  byte_length: isize,
+  byte_length: usize,
   data: *mut c_void,
-  _finalize_cb: napi_finalize,
+  finalize_cb: napi_finalize,
-  _finalize_hint: *mut c_void,
+  finalize_hint: *mut c_void,
   result: *mut napi_value,
 ) -> Result {
-  check_env!(env);
+  check_env!(env_ptr);
-  let env = unsafe { &mut *env };
+  let env = unsafe { &mut *env_ptr };
-  let slice = if byte_length == -1 {
+
-    std::ffi::CStr::from_ptr(data as *const _).to_bytes()
+  let finalizer = BufferFinalizer {
-  } else {
+    env: env_ptr,
-    std::slice::from_raw_parts(data as *mut u8, byte_length as usize)
+    finalize_data: ptr::null_mut(),
+    finalize_cb,
+    finalize_hint,
   };
-  // TODO: make this not copy the slice
+
-  // TODO: finalization
+  let store: UniqueRef<BackingStore> =
-  let store = v8::ArrayBuffer::new_backing_store_from_boxed_slice(
+    transmute(v8__ArrayBuffer__NewBackingStore__with_data(
-    slice.to_vec().into_boxed_slice(),
+      data,
-  );
+      byte_length,
+      backing_store_deleter_callback,
+      finalizer.into_raw() as _,
+    ));
+
   let ab =
     v8::ArrayBuffer::with_backing_store(&mut env.scope(), &store.make_shared());
   let value =
-    v8::Uint8Array::new(&mut env.scope(), ab, 0, slice.len()).unwrap();
+    v8::Uint8Array::new(&mut env.scope(), ab, 0, byte_length).unwrap();
   let value: v8::Local<v8::Value> = value.into();
   *result = value.into();
   Ok(())
@@ -1223,17 +1262,25 @@ fn napi_get_value_uint32(
   Ok(())
 }
 
-// TODO
 #[napi_sym::napi_sym]
 fn napi_add_finalizer(
-  _env: *mut Env,
+  env_ptr: *mut Env,
-  _js_object: napi_value,
+  js_object: napi_value,
-  _native_object: *const c_void,
+  native_object: *mut c_void,
-  _finalize_cb: napi_finalize,
+  finalize_cb: napi_finalize,
-  _finalize_hint: *const c_void,
+  finalize_hint: *mut c_void,
-  _result: *mut napi_ref,
+  result: *mut napi_ref,
 ) -> Result {
-  log::info!("napi_add_finalizer is not yet supported.");
+  check_env!(env_ptr);
+
+  let value = napi_value_unchecked(js_object);
+  let value =
+    weak_local(env_ptr, value, native_object, finalize_cb, finalize_hint);
+
+  if !result.is_null() {
+    *result = transmute(value);
+  }
+
   Ok(())
 }
 

cli/napi/threadsafe_functions.rs

@@ -18,6 +18,8 @@ pub struct TsFn {
   pub context: *mut c_void,
   pub thread_counter: usize,
   pub ref_counter: Arc<AtomicUsize>,
+  finalizer: Option<napi_finalize>,
+  finalizer_data: *mut c_void,
   sender: mpsc::UnboundedSender<PendingNapiAsyncWork>,
   tsfn_sender: mpsc::UnboundedSender<ThreadSafeFunctionStatus>,
 }
@@ -25,7 +27,12 @@ pub struct TsFn {
 impl Drop for TsFn {
   fn drop(&mut self) {
     let env = unsafe { self.env.as_mut().unwrap() };
-    env.remove_threadsafe_function_ref_counter(self.id)
+    env.remove_threadsafe_function_ref_counter(self.id);
+    if let Some(finalizer) = self.finalizer {
+      unsafe {
+        (finalizer)(self.env as _, self.finalizer_data, ptr::null_mut());
+      }
+    }
   }
 }
 
@@ -126,8 +133,8 @@ fn napi_create_threadsafe_function(
   _async_resource_name: napi_value,
   _max_queue_size: usize,
   initial_thread_count: usize,
-  _thread_finialize_data: *mut c_void,
+  thread_finialize_data: *mut c_void,
-  _thread_finalize_cb: napi_finalize,
+  thread_finalize_cb: Option<napi_finalize>,
   context: *mut c_void,
   maybe_call_js_cb: Option<napi_threadsafe_function_call_js>,
   result: *mut napi_threadsafe_function,
@@ -153,10 +160,13 @@ fn napi_create_threadsafe_function(
     context,
     thread_counter: initial_thread_count,
     sender: env_ref.async_work_sender.clone(),
+    finalizer: thread_finalize_cb,
+    finalizer_data: thread_finialize_data,
     tsfn_sender: env_ref.threadsafe_function_sender.clone(),
     ref_counter: Arc::new(AtomicUsize::new(1)),
     env,
   };
+
   env_ref
     .add_threadsafe_function_ref_counter(tsfn.id, tsfn.ref_counter.clone());
 

ext/napi/lib.rs

@@ -592,6 +592,50 @@ pub trait NapiPermissions {
     -> std::result::Result<(), AnyError>;
 }
 
+/// # Safety
+///
+/// This function is unsafe because it dereferences raw pointer Env.
+/// - The caller must ensure that the pointer is valid.
+/// - The caller must ensure that the pointer is not freed.
+pub unsafe fn weak_local(
+  env_ptr: *mut Env,
+  value: v8::Local<v8::Value>,
+  data: *mut c_void,
+  finalize_cb: napi_finalize,
+  finalize_hint: *mut c_void,
+) -> Option<v8::Local<v8::Value>> {
+  use std::cell::Cell;
+
+  let env = &mut *env_ptr;
+
+  let weak_ptr = Rc::new(Cell::new(None));
+  let scope = &mut env.scope();
+
+  let weak = v8::Weak::with_finalizer(
+    scope,
+    value,
+    Box::new({
+      let weak_ptr = weak_ptr.clone();
+      move |isolate| {
+        finalize_cb(env_ptr as _, data as _, finalize_hint as _);
+
+        // Self-deleting weak.
+        if let Some(weak_ptr) = weak_ptr.get() {
+          let weak: v8::Weak<v8::Value> =
+            unsafe { v8::Weak::from_raw(isolate, Some(weak_ptr)) };
+          drop(weak);
+        }
+      }
+    }),
+  );
+
+  let value = weak.to_local(scope);
+  let raw = weak.into_raw();
+  weak_ptr.set(raw);
+
+  value
+}
+
 #[op(v8)]
 fn op_napi_open<NP, 'scope>(
   scope: &mut v8::HandleScope<'scope>,

test_napi/object_wrap_test.js

@@ -1,6 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-import { assertEquals, loadTestLibrary } from "./common.js";
+import { assert, assertEquals, loadTestLibrary } from "./common.js";
 
 const objectWrap = loadTestLibrary();
 
@@ -16,3 +16,26 @@ Deno.test("napi object wrap new", function () {
   assertEquals(obj.get_value(), 10);
   assertEquals(objectWrap.NapiObject.factory(), 64);
 });
+
+Deno.test("napi bind finalizer", function () {
+  const obj = {};
+  objectWrap.test_bind_finalizer(obj);
+});
+
+Deno.test("napi external finalizer", function () {
+  let obj = objectWrap.test_external_finalizer();
+  assert(obj);
+  obj = null;
+});
+
+Deno.test("napi external buffer", function () {
+  let buf = objectWrap.test_external_buffer();
+  assertEquals(buf, new Uint8Array([1, 2, 3]));
+  buf = null;
+});
+
+Deno.test("napi external arraybuffer", function () {
+  let buf = objectWrap.test_external_arraybuffer();
+  assertEquals(new Uint8Array(buf), new Uint8Array([1, 2, 3]));
+  buf = null;
+});

test_napi/src/finalizer.rs

@@ -0,0 +1,141 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+use crate::assert_napi_ok;
+use crate::napi_get_callback_info;
+use crate::napi_new_property;
+use napi_sys::ValueType::napi_object;
+use napi_sys::*;
+use std::ptr;
+
+unsafe extern "C" fn finalize_cb(
+  _env: napi_env,
+  data: *mut ::std::os::raw::c_void,
+  hint: *mut ::std::os::raw::c_void,
+) {
+  assert!(data.is_null());
+  assert!(hint.is_null());
+}
+
+extern "C" fn test_bind_finalizer(
+  env: napi_env,
+  info: napi_callback_info,
+) -> napi_value {
+  let (args, argc, _) = napi_get_callback_info!(env, info, 1);
+  assert_eq!(argc, 1);
+
+  let mut ty = -1;
+  assert_napi_ok!(napi_typeof(env, args[0], &mut ty));
+  assert_eq!(ty, napi_object);
+
+  let obj = args[0];
+  unsafe {
+    napi_add_finalizer(
+      env,
+      obj,
+      ptr::null_mut(),
+      Some(finalize_cb),
+      ptr::null_mut(),
+      ptr::null_mut(),
+    )
+  };
+  obj
+}
+
+struct Thing {
+  _allocation: Vec<u8>,
+}
+
+unsafe extern "C" fn finalize_cb_drop(
+  _env: napi_env,
+  data: *mut ::std::os::raw::c_void,
+  hint: *mut ::std::os::raw::c_void,
+) {
+  let _ = Box::from_raw(data as *mut Thing);
+  assert!(hint.is_null());
+}
+
+extern "C" fn test_external_finalizer(
+  env: napi_env,
+  _: napi_callback_info,
+) -> napi_value {
+  let data = Box::into_raw(Box::new(Thing {
+    _allocation: vec![1, 2, 3],
+  }));
+
+  let mut result = ptr::null_mut();
+  assert_napi_ok!(napi_create_external(
+    env,
+    data as _,
+    Some(finalize_cb_drop),
+    ptr::null_mut(),
+    &mut result
+  ));
+  result
+}
+
+unsafe extern "C" fn finalize_cb_vec(
+  _env: napi_env,
+  data: *mut ::std::os::raw::c_void,
+  hint: *mut ::std::os::raw::c_void,
+) {
+  let _ = Vec::from_raw_parts(data as *mut u8, 3, 3);
+  assert!(hint.is_null());
+}
+
+extern "C" fn test_external_buffer(
+  env: napi_env,
+  _: napi_callback_info,
+) -> napi_value {
+  let mut result = ptr::null_mut();
+  let buf: Vec<u8> = vec![1, 2, 3];
+  assert_napi_ok!(napi_create_external_buffer(
+    env,
+    3,
+    buf.as_ptr() as _,
+    Some(finalize_cb_vec),
+    ptr::null_mut(),
+    &mut result
+  ));
+  std::mem::forget(buf);
+
+  result
+}
+
+extern "C" fn test_external_arraybuffer(
+  env: napi_env,
+  _: napi_callback_info,
+) -> napi_value {
+  let mut result = ptr::null_mut();
+  let buf: Vec<u8> = vec![1, 2, 3];
+  assert_napi_ok!(napi_create_external_arraybuffer(
+    env,
+    buf.as_ptr() as _,
+    3,
+    Some(finalize_cb_vec),
+    ptr::null_mut(),
+    &mut result
+  ));
+  std::mem::forget(buf);
+
+  result
+}
+
+pub fn init(env: napi_env, exports: napi_value) {
+  let properties = &[
+    napi_new_property!(env, "test_bind_finalizer", test_bind_finalizer),
+    napi_new_property!(env, "test_external_finalizer", test_external_finalizer),
+    napi_new_property!(env, "test_external_buffer", test_external_buffer),
+    napi_new_property!(
+      env,
+      "test_external_arraybuffer",
+      test_external_arraybuffer
+    ),
+  ];
+
+  assert_napi_ok!(napi_define_properties(
+    env,
+    exports,
+    properties.len(),
+    properties.as_ptr()
+  ));
+}

test_napi/src/lib.rs

@@ -15,6 +15,7 @@ pub mod coerce;
 pub mod date;
 pub mod env;
 pub mod error;
+pub mod finalizer;
 pub mod mem;
 pub mod numbers;
 pub mod object_wrap;
@@ -147,6 +148,7 @@ unsafe extern "C" fn napi_register_module_v1(
   array::init(env, exports);
   env::init(env, exports);
   error::init(env, exports);
+  finalizer::init(env, exports);
   primitives::init(env, exports);
   properties::init(env, exports);
   promise::init(env, exports);