From f13742cc73c561ab912e88973d24e55ccedf7d98 Mon Sep 17 00:00:00 2001 From: snek Date: Tue, 11 Jun 2024 17:40:44 -0700 Subject: [PATCH] fix: clean up some node-api details (#24178) - fix a few napi_* types - clean up env hooks - implement blocking queue in tsfn - reduce transmutes - use new `DataView::new` api from rusty_v8 --- cli/napi/js_native_api.rs | 134 +++++++++++++++------------------ cli/napi/node_api.rs | 151 +++++++++++++++++--------------------- cli/napi/util.rs | 13 ++-- ext/napi/function.rs | 63 ++++++---------- ext/napi/lib.rs | 77 +++++++++++++------ ext/napi/value.rs | 4 +- 6 files changed, 212 insertions(+), 230 deletions(-) diff --git a/cli/napi/js_native_api.rs b/cli/napi/js_native_api.rs index cbce113dc0..6b31e1fd23 100644 --- a/cli/napi/js_native_api.rs +++ b/cli/napi/js_native_api.rs @@ -179,14 +179,15 @@ fn napi_get_last_error_info( } #[napi_sym] -fn napi_create_function( - env: &mut Env, +fn napi_create_function<'s>( + env: &'s mut Env, name: *const c_char, length: usize, - cb: napi_callback, + cb: Option, cb_info: napi_callback_info, - result: *mut napi_value, + result: *mut napi_value<'s>, ) -> napi_status { + let env_ptr = env as *mut Env; check_arg!(env, result); check_arg!(env, cb); @@ -200,7 +201,9 @@ fn napi_create_function( }; unsafe { - *result = create_function(env, name, cb, cb_info).into(); + *result = + create_function(&mut env.scope(), env_ptr, name, cb.unwrap(), cb_info) + .into(); } napi_ok @@ -212,12 +215,13 @@ fn napi_define_class<'s>( env: &'s mut Env, utf8name: *const c_char, length: usize, - constructor: napi_callback, + constructor: Option, callback_data: *mut c_void, property_count: usize, properties: *const napi_property_descriptor, result: *mut napi_value<'s>, ) -> napi_status { + let env_ptr = env as *mut Env; check_arg!(env, result); check_arg!(env, constructor); @@ -230,9 +234,13 @@ fn napi_define_class<'s>( Err(status) => return status, }; - let tpl = unsafe { - create_function_template(env, Some(name), constructor, callback_data) - }; + let tpl = create_function_template( + &mut env.scope(), + env_ptr, + Some(name), + constructor.unwrap(), + callback_data, + ); let napi_properties: &[napi_property_descriptor] = if property_count > 0 { unsafe { std::slice::from_raw_parts(properties, property_count) } @@ -248,28 +256,18 @@ fn napi_define_class<'s>( continue; } - let name = match unsafe { v8_name_from_property_descriptor(env, p) } { + let name = match unsafe { v8_name_from_property_descriptor(env_ptr, p) } { Ok(name) => name, Err(status) => return status, }; - let method = p.method; - let getter = p.getter; - let setter = p.setter; - - if getter.is_some() || setter.is_some() { - let getter: Option> = if getter.is_some() - { - Some(unsafe { create_function_template(env, None, p.getter, p.data) }) - } else { - None - }; - let setter: Option> = if setter.is_some() - { - Some(unsafe { create_function_template(env, None, p.setter, p.data) }) - } else { - None - }; + if p.getter.is_some() || p.setter.is_some() { + let getter = p.getter.map(|g| { + create_function_template(&mut env.scope(), env_ptr, None, g, p.data) + }); + let setter = p.setter.map(|s| { + create_function_template(&mut env.scope(), env_ptr, None, s, p.data) + }); let mut accessor_property = v8::PropertyAttribute::NONE; if getter.is_some() @@ -290,9 +288,14 @@ fn napi_define_class<'s>( let proto = tpl.prototype_template(&mut env.scope()); proto.set_accessor_property(name, getter, setter, accessor_property); - } else if method.is_some() { - let function = - unsafe { create_function_template(env, None, p.method, p.data) }; + } else if let Some(method) = p.method { + let function = create_function_template( + &mut env.scope(), + env_ptr, + None, + method, + p.data, + ); let proto = tpl.prototype_template(&mut env.scope()); proto.set(name, function.into()); } else { @@ -301,7 +304,6 @@ fn napi_define_class<'s>( } } - let env_ptr = env as *mut Env; let value: v8::Local = tpl.get_function(&mut env.scope()).unwrap().into(); @@ -781,17 +783,19 @@ fn napi_define_properties( let configurable = property.attributes & napi_configurable != 0; if property.getter.is_some() || property.setter.is_some() { - let local_getter: v8::Local = if property.getter.is_some() { - unsafe { - create_function(env_ptr, None, property.getter, property.data).into() - } + let local_getter: v8::Local = if let Some(getter) = + property.getter + { + create_function(&mut env.scope(), env_ptr, None, getter, property.data) + .into() } else { v8::undefined(scope).into() }; - let local_setter: v8::Local = if property.setter.is_some() { - unsafe { - create_function(env_ptr, None, property.setter, property.data).into() - } + let local_setter: v8::Local = if let Some(setter) = + property.setter + { + create_function(&mut env.scope(), env_ptr, None, setter, property.data) + .into() } else { v8::undefined(scope).into() }; @@ -807,11 +811,15 @@ fn napi_define_properties( { return napi_invalid_arg; } - } else if property.method.is_some() { + } else if let Some(method) = property.method { let method: v8::Local = { - let function = unsafe { - create_function(env_ptr, None, property.method, property.data) - }; + let function = create_function( + &mut env.scope(), + env_ptr, + None, + method, + property.data, + ); function.into() }; @@ -1692,18 +1700,16 @@ fn napi_call_function( } #[napi_sym] -fn napi_get_global(env: *mut Env, result: *mut napi_value) -> napi_status { - let env = check_env!(env); +fn napi_get_global(env_ptr: *mut Env, result: *mut napi_value) -> napi_status { + let env = check_env!(env_ptr); check_arg!(env, result); + let global = v8::Local::new(&mut env.scope(), &env.global); unsafe { - *result = std::mem::transmute::, v8::Local>( - env.global, - ) - .into(); + *result = global.into(); } - return napi_clear_last_error(env); + return napi_clear_last_error(env_ptr); } #[napi_sym] @@ -2869,7 +2875,7 @@ fn napi_create_arraybuffer<'s>( if !data.is_null() { unsafe { - *data = get_array_buffer_ptr(buffer) as _; + *data = get_array_buffer_ptr(buffer); } } @@ -2915,7 +2921,7 @@ fn napi_create_external_arraybuffer<'s>( fn napi_get_arraybuffer_info( env: *mut Env, value: napi_value, - data: *mut *mut u8, + data: *mut *mut c_void, length: *mut usize, ) -> napi_status { let env = check_env!(env); @@ -3121,7 +3127,7 @@ fn napi_get_typedarray_info( fn napi_create_dataview<'s>( env: &'s mut Env, byte_length: usize, - arraybuffer: napi_value, + arraybuffer: napi_value<'s>, byte_offset: usize, result: *mut napi_value<'s>, ) -> napi_status { @@ -3144,26 +3150,8 @@ fn napi_create_dataview<'s>( } } - // let dataview = v8::DataView::new(&mut env, buffer, byte_offset, byte_length); - let dataview = { - let context = &mut env.scope().get_current_context(); - let global = context.global(&mut env.scope()); - let data_view_name = v8::String::new(&mut env.scope(), "DataView").unwrap(); - let data_view = - global.get(&mut env.scope(), data_view_name.into()).unwrap(); - let Ok(data_view) = v8::Local::::try_from(data_view) else { - return napi_function_expected; - }; - let byte_offset = v8::Number::new(&mut env.scope(), byte_offset as f64); - let byte_length = v8::Number::new(&mut env.scope(), byte_length as f64); - let Some(dv) = data_view.new_instance( - &mut env.scope(), - &[buffer.into(), byte_offset.into(), byte_length.into()], - ) else { - return napi_generic_failure; - }; - dv - }; + let dataview = + v8::DataView::new(&mut env.scope(), buffer, byte_offset, byte_length); unsafe { *result = dataview.into(); diff --git a/cli/napi/node_api.rs b/cli/napi/node_api.rs index 28a11d614d..268715fa89 100644 --- a/cli/napi/node_api.rs +++ b/cli/napi/node_api.rs @@ -9,10 +9,11 @@ use super::util::napi_set_last_error; use super::util::SendPtr; use crate::check_arg; use crate::check_env; +use deno_core::parking_lot::Condvar; +use deno_core::parking_lot::Mutex; use deno_core::V8CrossThreadTaskSpawner; use deno_runtime::deno_napi::*; use napi_sym::napi_sym; -use std::ptr::NonNull; use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU8; use std::sync::atomic::AtomicUsize; @@ -40,14 +41,7 @@ fn napi_add_env_cleanup_hook( let fun = fun.unwrap(); - let mut env_cleanup_hooks = env.cleanup_hooks.borrow_mut(); - if env_cleanup_hooks - .iter() - .any(|pair| pair.0 == fun && pair.1 == arg) - { - panic!("Cleanup hook with this data already registered"); - } - env_cleanup_hooks.push((fun, arg)); + env.add_cleanup_hook(fun, arg); napi_ok } @@ -63,27 +57,21 @@ fn napi_remove_env_cleanup_hook( let fun = fun.unwrap(); - let mut env_cleanup_hooks = env.cleanup_hooks.borrow_mut(); - // Hooks are supposed to be removed in LIFO order - let maybe_index = env_cleanup_hooks - .iter() - .rposition(|&pair| pair.0 == fun && pair.1 == arg); - - if let Some(index) = maybe_index { - env_cleanup_hooks.remove(index); - } else { - panic!("Cleanup hook with this data not found"); - } + env.remove_cleanup_hook(fun, arg); napi_ok } -type AsyncCleanupHandle = (*mut Env, napi_async_cleanup_hook, *mut c_void); +struct AsyncCleanupHandle { + env: *mut Env, + hook: napi_async_cleanup_hook, + data: *mut c_void, +} unsafe extern "C" fn async_cleanup_handler(arg: *mut c_void) { unsafe { - let (env, hook, arg) = *Box::::from_raw(arg as _); - hook(env as _, arg); + let handle = Box::::from_raw(arg as _); + (handle.hook)(arg, handle.data); } } @@ -99,12 +87,13 @@ fn napi_add_async_cleanup_hook( let hook = hook.unwrap(); - let handle = - Box::into_raw(Box::::new((env, hook, arg))) as _; + let handle = Box::into_raw(Box::new(AsyncCleanupHandle { + env, + hook, + data: arg, + })) as *mut c_void; - unsafe { - napi_add_env_cleanup_hook(env, Some(async_cleanup_handler), handle); - } + env.add_cleanup_hook(async_cleanup_handler, handle); if !remove_handle.is_null() { unsafe { @@ -123,17 +112,12 @@ fn napi_remove_async_cleanup_hook( return napi_invalid_arg; } - let handle = unsafe { &*(remove_handle as *mut AsyncCleanupHandle) }; + let handle = + unsafe { Box::::from_raw(remove_handle as _) }; - let env = unsafe { &mut *handle.0 }; + let env = unsafe { &mut *handle.env }; - unsafe { - napi_remove_env_cleanup_hook( - env, - Some(async_cleanup_handler), - remove_handle, - ); - } + env.remove_cleanup_hook(async_cleanup_handler, remove_handle); napi_ok } @@ -142,11 +126,7 @@ fn napi_remove_async_cleanup_hook( fn napi_fatal_exception(env: &mut Env, err: napi_value) -> napi_status { check_arg!(env, err); - let report_error = unsafe { - std::mem::transmute::, v8::Local>( - env.report_error, - ) - }; + let report_error = v8::Local::new(&mut env.scope(), &env.report_error); let this = v8::undefined(&mut env.scope()); if report_error @@ -262,7 +242,7 @@ fn napi_make_callback<'s>( recv: napi_value, func: napi_value, argc: usize, - argv: *const napi_value, + argv: *const napi_value<'s>, result: *mut napi_value<'s>, ) -> napi_status { check_arg!(env, recv); @@ -312,11 +292,8 @@ fn napi_create_buffer<'s>( let ab = v8::ArrayBuffer::new(&mut env.scope(), length); - let buffer_constructor = unsafe { - std::mem::transmute::, v8::Local>( - env.buffer_constructor, - ) - }; + let buffer_constructor = + v8::Local::new(&mut env.scope(), &env.buffer_constructor); let Some(buffer) = buffer_constructor.new_instance(&mut env.scope(), &[ab.into()]) else { @@ -325,7 +302,7 @@ fn napi_create_buffer<'s>( if !data.is_null() { unsafe { - *data = get_array_buffer_ptr(ab) as _; + *data = get_array_buffer_ptr(ab); } } @@ -359,11 +336,8 @@ fn napi_create_external_buffer<'s>( let ab = v8::ArrayBuffer::with_backing_store(&mut env.scope(), &store.make_shared()); - let buffer_constructor = unsafe { - std::mem::transmute::, v8::Local>( - env.buffer_constructor, - ) - }; + let buffer_constructor = + v8::Local::new(&mut env.scope(), &env.buffer_constructor); let Some(buffer) = buffer_constructor.new_instance(&mut env.scope(), &[ab.into()]) else { @@ -389,18 +363,15 @@ fn napi_create_buffer_copy<'s>( let ab = v8::ArrayBuffer::new(&mut env.scope(), length); - let buffer_constructor = unsafe { - std::mem::transmute::, v8::Local>( - env.buffer_constructor, - ) - }; + let buffer_constructor = + v8::Local::new(&mut env.scope(), &env.buffer_constructor); let Some(buffer) = buffer_constructor.new_instance(&mut env.scope(), &[ab.into()]) else { return napi_generic_failure; }; - let ptr = get_array_buffer_ptr(ab) as *mut c_void; + let ptr = get_array_buffer_ptr(ab); unsafe { std::ptr::copy(data, ptr, length); } @@ -428,11 +399,8 @@ fn napi_is_buffer( check_arg!(env, value); check_arg!(env, result); - let buffer_constructor = unsafe { - std::mem::transmute::, v8::Local>( - env.buffer_constructor, - ) - }; + let buffer_constructor = + v8::Local::new(&mut env.scope(), &env.buffer_constructor); let Some(is_buffer) = value .unwrap() @@ -464,11 +432,8 @@ fn napi_get_buffer_info( return napi_set_last_error(env, napi_invalid_arg); }; - let buffer_constructor = unsafe { - std::mem::transmute::, v8::Local>( - env.buffer_constructor, - ) - }; + let buffer_constructor = + v8::Local::new(&mut env.scope(), &env.buffer_constructor); if !ta .instance_of(&mut env.scope(), buffer_constructor.into()) @@ -703,7 +668,9 @@ extern "C" fn default_call_js_cb( struct TsFn { env: *mut Env, func: Option>, - _max_queue_size: usize, + max_queue_size: usize, + queue_size: Mutex, + queue_cond: Condvar, thread_count: AtomicUsize, thread_finalize_data: *mut c_void, thread_finalize_cb: Option, @@ -771,6 +738,7 @@ impl TsFn { .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) .is_ok() { + tsfn.queue_cond.notify_all(); let tsfnptr = SendPtr(tsfn); // drop must be queued in order to preserve ordering consistent // with Node.js and so that the finalizer runs on the main thread. @@ -811,26 +779,34 @@ impl TsFn { pub fn call( &self, data: *mut c_void, - _mode: napi_threadsafe_function_call_mode, + mode: napi_threadsafe_function_call_mode, ) -> napi_status { - // TODO: - // if self.max_queue_size > 0 && queued >= self.max_queue_size { - // if mode == napi_tsfn_blocking { - // wait somehow - // } else { - // return napi_queue_full; - // } - // } - if self.is_closing.load(Ordering::SeqCst) { return napi_closing; } + if self.max_queue_size > 0 { + let mut queue_size = self.queue_size.lock(); + while *queue_size >= self.max_queue_size { + if mode == napi_tsfn_blocking { + self.queue_cond.wait(&mut queue_size); + + if self.is_closing.load(Ordering::SeqCst) { + return napi_closing; + } + } else { + return napi_queue_full; + } + } + *queue_size += 1; + } + let is_closed = self.is_closed.clone(); let tsfn = SendPtr(self); let data = SendPtr(data); let context = SendPtr(self.context); let call_js_cb = self.call_js_cb; + self.sender.spawn(move |scope: &mut v8::HandleScope| { let data = data.take(); @@ -849,6 +825,15 @@ impl TsFn { let tsfn = unsafe { &*tsfn }; + if tsfn.max_queue_size > 0 { + let mut queue_size = tsfn.queue_size.lock(); + let size = *queue_size; + *queue_size -= 1; + if size == tsfn.max_queue_size { + tsfn.queue_cond.notify_one(); + } + } + let func = tsfn.func.as_ref().map(|f| v8::Local::new(scope, f)); unsafe { @@ -918,7 +903,9 @@ fn napi_create_threadsafe_function( let mut tsfn = Box::new(TsFn { env, func, - _max_queue_size: max_queue_size, + max_queue_size, + queue_size: Mutex::new(0), + queue_cond: Condvar::new(), thread_count: AtomicUsize::new(initial_thread_count), thread_finalize_data, thread_finalize_cb, diff --git a/cli/napi/util.rs b/cli/napi/util.rs index cd3ef14a2d..63d8effbf2 100644 --- a/cli/napi/util.rs +++ b/cli/napi/util.rs @@ -15,10 +15,11 @@ impl SendPtr { unsafe impl Send for SendPtr {} unsafe impl Sync for SendPtr {} -pub fn get_array_buffer_ptr(ab: v8::Local) -> *mut u8 { - // SAFETY: Thanks to the null pointer optimization, NonNull and Option> are guaranteed - // to have the same size and alignment. - unsafe { std::mem::transmute(ab.data()) } +pub fn get_array_buffer_ptr(ab: v8::Local) -> *mut c_void { + match ab.data() { + Some(p) => p.as_ptr(), + None => std::ptr::null_mut(), + } } struct BufferFinalizer { @@ -216,10 +217,6 @@ impl<'s> Nullable for napi_value<'s> { } } -// TODO: replace Nullable with some sort of "CheckedUnwrap" trait -// *mut T -> &mut MaybeUninit -// Option -> T -// napi_value -> Local #[macro_export] macro_rules! check_arg { ($env: expr, $ptr: expr) => { diff --git a/ext/napi/function.rs b/ext/napi/function.rs index 2d2933b954..bdfa7d7e1e 100644 --- a/ext/napi/function.rs +++ b/ext/napi/function.rs @@ -4,7 +4,7 @@ use crate::*; #[repr(C)] #[derive(Debug)] pub struct CallbackInfo { - pub env: napi_env, + pub env: *mut Env, pub cb: napi_callback, pub cb_info: napi_callback_info, pub args: *const c_void, @@ -13,7 +13,7 @@ pub struct CallbackInfo { impl CallbackInfo { #[inline] pub fn new_raw( - env: napi_env, + env: *mut Env, cb: napi_callback, cb_info: napi_callback_info, ) -> *mut Self { @@ -41,38 +41,27 @@ extern "C" fn call_fn(info: *const v8::FunctionCallbackInfo) { let info = unsafe { &mut *info_ptr }; info.args = &args as *const _ as *const c_void; - if let Some(f) = info.cb { - // SAFETY: calling user provided function pointer. - let value = unsafe { f(info.env, info_ptr as *mut _) }; - if let Some(exc) = unsafe { &mut *(info.env as *mut Env) } - .last_exception - .take() - { - let scope = unsafe { &mut v8::CallbackScope::new(callback_info) }; - let exc = v8::Local::new(scope, exc); - scope.throw_exception(exc); - } - if let Some(value) = *value { - rv.set(value); - } + // SAFETY: calling user provided function pointer. + let value = unsafe { (info.cb)(info.env as napi_env, info_ptr as *mut _) }; + if let Some(exc) = unsafe { &mut *info.env }.last_exception.take() { + let scope = unsafe { &mut v8::CallbackScope::new(callback_info) }; + let exc = v8::Local::new(scope, exc); + scope.throw_exception(exc); + } + if let Some(value) = *value { + rv.set(value); } } -/// # Safety -/// env_ptr must be valid -pub unsafe fn create_function<'a>( - env_ptr: *mut Env, +pub fn create_function<'s>( + scope: &mut v8::HandleScope<'s>, + env: *mut Env, name: Option>, cb: napi_callback, cb_info: napi_callback_info, -) -> v8::Local<'a, v8::Function> { - let env = unsafe { &mut *env_ptr }; - let scope = &mut env.scope(); - - let external = v8::External::new( - scope, - CallbackInfo::new_raw(env_ptr as _, cb, cb_info) as *mut _, - ); +) -> v8::Local<'s, v8::Function> { + let external = + v8::External::new(scope, CallbackInfo::new_raw(env, cb, cb_info) as *mut _); let function = v8::Function::builder_raw(call_fn) .data(external.into()) .build(scope) @@ -85,21 +74,15 @@ pub unsafe fn create_function<'a>( function } -/// # Safety -/// env_ptr must be valid -pub unsafe fn create_function_template<'a>( - env_ptr: *mut Env, +pub fn create_function_template<'s>( + scope: &mut v8::HandleScope<'s>, + env: *mut Env, name: Option>, cb: napi_callback, cb_info: napi_callback_info, -) -> v8::Local<'a, v8::FunctionTemplate> { - let env = unsafe { &mut *env_ptr }; - let scope = &mut env.scope(); - - let external = v8::External::new( - scope, - CallbackInfo::new_raw(env_ptr as _, cb, cb_info) as *mut _, - ); +) -> v8::Local<'s, v8::FunctionTemplate> { + let external = + v8::External::new(scope, CallbackInfo::new_raw(env, cb, cb_info) as *mut _); let function = v8::FunctionTemplate::builder_raw(call_fn) .data(external.into()) .build(scope); diff --git a/ext/napi/lib.rs b/ext/napi/lib.rs index 39b303f860..402785ce8d 100644 --- a/ext/napi/lib.rs +++ b/ext/napi/lib.rs @@ -187,12 +187,10 @@ pub struct napi_type_tag { pub upper: u64, } -pub type napi_callback = Option< - unsafe extern "C" fn( - env: napi_env, - info: napi_callback_info, - ) -> napi_value<'static>, ->; +pub type napi_callback = unsafe extern "C" fn( + env: napi_env, + info: napi_callback_info, +) -> napi_value<'static>; pub type napi_finalize = unsafe extern "C" fn( env: napi_env, @@ -213,8 +211,10 @@ pub type napi_threadsafe_function_call_js = unsafe extern "C" fn( data: *mut c_void, ); -pub type napi_async_cleanup_hook = - unsafe extern "C" fn(env: napi_env, data: *mut c_void); +pub type napi_async_cleanup_hook = unsafe extern "C" fn( + handle: napi_async_cleanup_hook_handle, + data: *mut c_void, +); pub type napi_cleanup_hook = unsafe extern "C" fn(data: *mut c_void); @@ -235,9 +235,9 @@ pub const napi_default_jsproperty: napi_property_attributes = pub struct napi_property_descriptor<'a> { pub utf8name: *const c_char, pub name: napi_value<'a>, - pub method: napi_callback, - pub getter: napi_callback, - pub setter: napi_callback, + pub method: Option, + pub getter: Option, + pub setter: Option, pub value: napi_value<'a>, pub attributes: napi_property_attributes, pub data: *mut c_void, @@ -348,13 +348,13 @@ pub struct Env { pub open_handle_scopes: usize, pub shared: *mut EnvShared, pub async_work_sender: V8CrossThreadTaskSpawner, - pub cleanup_hooks: Rc>>, - pub external_ops_tracker: ExternalOpsTracker, + cleanup_hooks: Rc>>, + external_ops_tracker: ExternalOpsTracker, pub last_error: napi_extended_error_info, pub last_exception: Option>, - pub global: NonNull, - pub buffer_constructor: NonNull, - pub report_error: NonNull, + pub global: v8::Global, + pub buffer_constructor: v8::Global, + pub report_error: v8::Global, } unsafe impl Send for Env {} @@ -365,7 +365,7 @@ impl Env { pub fn new( isolate_ptr: *mut v8::OwnedIsolate, context: v8::Global, - global: v8::Global, + global: v8::Global, buffer_constructor: v8::Global, report_error: v8::Global, sender: V8CrossThreadTaskSpawner, @@ -375,9 +375,9 @@ impl Env { Self { isolate_ptr, context: context.into_raw(), - global: global.into_raw(), - buffer_constructor: buffer_constructor.into_raw(), - report_error: report_error.into_raw(), + global, + buffer_constructor, + report_error, shared: std::ptr::null_mut(), open_handle_scopes: 0, async_work_sender: sender, @@ -435,6 +435,35 @@ impl Env { pub fn threadsafe_function_unref(&mut self) { self.external_ops_tracker.unref_op(); } + + pub fn add_cleanup_hook( + &mut self, + hook: napi_cleanup_hook, + data: *mut c_void, + ) { + let mut hooks = self.cleanup_hooks.borrow_mut(); + if hooks.iter().any(|pair| pair.0 == hook && pair.1 == data) { + panic!("Cannot register cleanup hook with same data twice"); + } + hooks.push((hook, data)); + } + + pub fn remove_cleanup_hook( + &mut self, + hook: napi_cleanup_hook, + data: *mut c_void, + ) { + let mut hooks = self.cleanup_hooks.borrow_mut(); + match hooks + .iter() + .rposition(|&pair| pair.0 == hook && pair.1 == data) + { + Some(index) => { + hooks.remove(index); + } + None => panic!("Cannot remove cleanup hook which was not registered"), + } + } } deno_core::extension!(deno_napi, @@ -468,7 +497,7 @@ fn op_napi_open( scope: &mut v8::HandleScope<'scope>, op_state: Rc>, #[string] path: String, - global: v8::Local<'scope, v8::Value>, + global: v8::Local<'scope, v8::Object>, buffer_constructor: v8::Local<'scope, v8::Function>, report_error: v8::Local<'scope, v8::Function>, ) -> std::result::Result, AnyError> @@ -499,9 +528,6 @@ where let type_tag = v8::Private::new(scope, Some(type_tag_name)); let type_tag = v8::Global::new(scope, type_tag); - // The `module.exports` object. - let exports = v8::Object::new(scope); - let env_shared = EnvShared::new(napi_wrap, type_tag, path.clone()); let ctx = scope.get_current_context(); @@ -542,6 +568,9 @@ where slot.take() }); + // The `module.exports` object. + let exports = v8::Object::new(scope); + let maybe_exports = if let Some(module_to_register) = maybe_module { // SAFETY: napi_register_module guarantees that `module_to_register` is valid. let nm = unsafe { &*module_to_register }; diff --git a/ext/napi/value.rs b/ext/napi/value.rs index 6fb96758c1..71beac07e2 100644 --- a/ext/napi/value.rs +++ b/ext/napi/value.rs @@ -31,9 +31,7 @@ where v8::Local<'s, T>: Into>, { fn from(v: v8::Local<'s, T>) -> Self { - // SAFETY: It is safe to cast v8::Local that implements Into>. - // `fn into(self)` transmutes anyways. - Self(unsafe { transmute(v) }, std::marker::PhantomData) + Self(Some(NonNull::from(&*v.into())), std::marker::PhantomData) } }