From a9f9dbb232637ba89388bf9d900e7b12742005be Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 15 Dec 2022 23:24:22 +0800 Subject: [PATCH 1/7] chore(napi): reduce Mutex usage while loading addon --- .../bindgen_runtime/js_values/arraybuffer.rs | 27 -- .../src/bindgen_runtime/js_values/buffer.rs | 33 +-- .../src/bindgen_runtime/js_values/promise.rs | 2 +- .../bindgen_runtime/js_values/value_ref.rs | 11 +- crates/napi/src/bindgen_runtime/mod.rs | 2 +- .../src/bindgen_runtime/module_register.rs | 273 ++++++------------ crates/napi/src/lib.rs | 2 - crates/napi/src/tokio_runtime.rs | 28 +- crates/sys/src/functions.rs | 4 +- crates/sys/src/lib.rs | 17 +- 10 files changed, 123 insertions(+), 276 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs index c7363b4c84..663360a35a 100644 --- a/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs +++ b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs @@ -7,8 +7,6 @@ use std::sync::{ Arc, }; -#[cfg(feature = "napi4")] -use crate::bindgen_prelude::{CUSTOM_GC_TSFN, CUSTOM_GC_TSFN_CLOSED, MAIN_THREAD_ID}; pub use crate::js_values::TypedArrayType; use crate::{check_status, sys, Error, Result, Status}; @@ -66,31 +64,6 @@ macro_rules! impl_typed_array { fn drop(&mut self) { if Arc::strong_count(&self.drop_in_vm) == 1 { if let Some((ref_, env)) = self.raw { - #[cfg(feature = "napi4")] - { - if CUSTOM_GC_TSFN_CLOSED.load(std::sync::atomic::Ordering::SeqCst) { - return; - } - if !MAIN_THREAD_ID - .get() - .map(|id| &std::thread::current().id() == id) - .unwrap_or(false) - { - let status = unsafe { - sys::napi_call_threadsafe_function( - CUSTOM_GC_TSFN.load(std::sync::atomic::Ordering::SeqCst), - ref_ as *mut c_void, - 1, - ) - }; - assert!( - status == sys::Status::napi_ok, - "Call custom GC in ArrayBuffer::drop failed {:?}", - Status::from(status) - ); - return; - } - } crate::check_status_or_throw!( env, unsafe { sys::napi_reference_unref(env, ref_, &mut 0) }, diff --git a/crates/napi/src/bindgen_runtime/js_values/buffer.rs b/crates/napi/src/bindgen_runtime/js_values/buffer.rs index c39218d6a9..c3f986772d 100644 --- a/crates/napi/src/bindgen_runtime/js_values/buffer.rs +++ b/crates/napi/src/bindgen_runtime/js_values/buffer.rs @@ -1,4 +1,4 @@ -#[cfg(debug_assertions)] +#[cfg(all(debug_assertions, not(windows)))] use std::collections::HashSet; use std::ffi::c_void; use std::mem; @@ -6,12 +6,12 @@ use std::ops::{Deref, DerefMut}; use std::ptr::{self, NonNull}; use std::slice; use std::sync::Arc; -#[cfg(debug_assertions)] +#[cfg(all(debug_assertions, not(windows)))] use std::sync::Mutex; use crate::{bindgen_prelude::*, check_status, sys, Result, ValueType}; -#[cfg(debug_assertions)] +#[cfg(all(debug_assertions, not(windows)))] thread_local! { pub (crate) static BUFFER_DATA: Mutex> = Default::default(); } @@ -32,31 +32,6 @@ impl Drop for Buffer { fn drop(&mut self) { if Arc::strong_count(&self.ref_count) == 1 { if let Some((ref_, env)) = self.raw { - #[cfg(feature = "napi4")] - { - if CUSTOM_GC_TSFN_CLOSED.load(std::sync::atomic::Ordering::SeqCst) { - return; - } - if !MAIN_THREAD_ID - .get() - .map(|id| &std::thread::current().id() == id) - .unwrap_or(false) - { - let status = unsafe { - sys::napi_call_threadsafe_function( - CUSTOM_GC_TSFN.load(std::sync::atomic::Ordering::SeqCst), - ref_ as *mut c_void, - 1, - ) - }; - assert!( - status == sys::Status::napi_ok, - "Call custom GC in Buffer::drop failed {:?}", - Status::from(status) - ); - return; - } - } let mut ref_count = 0; check_status_or_throw!( env, @@ -94,7 +69,7 @@ impl Clone for Buffer { impl From> for Buffer { fn from(mut data: Vec) -> Self { let inner_ptr = data.as_mut_ptr(); - #[cfg(debug_assertions)] + #[cfg(all(debug_assertions, not(windows)))] { let is_existed = BUFFER_DATA.with(|buffer_data| { let buffer = buffer_data.lock().expect("Unlock buffer data failed"); diff --git a/crates/napi/src/bindgen_runtime/js_values/promise.rs b/crates/napi/src/bindgen_runtime/js_values/promise.rs index 9beea5d0ac..f6168d1f73 100644 --- a/crates/napi/src/bindgen_runtime/js_values/promise.rs +++ b/crates/napi/src/bindgen_runtime/js_values/promise.rs @@ -77,7 +77,7 @@ impl ValidateNapiValue for Promise { } } -unsafe impl Send for Promise {} +unsafe impl Send for Promise {} impl FromNapiValue for Promise { unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> crate::Result { diff --git a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs index 48a88d8211..f12c9beea1 100644 --- a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs +++ b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs @@ -6,7 +6,7 @@ use std::rc::{Rc, Weak}; use once_cell::sync::Lazy; use crate::{ - bindgen_runtime::{PersistedSingleThreadHashMap, ToNapiValue}, + bindgen_runtime::{PersistedPerInstanceHashMap, ToNapiValue}, check_status, Env, Error, Result, Status, }; @@ -16,9 +16,16 @@ type RefInformation = ( *const Cell<*mut dyn FnOnce()>, ); -pub(crate) static REFERENCE_MAP: Lazy> = +pub(crate) static REFERENCE_MAP: Lazy> = Lazy::new(Default::default); +#[ctor::dtor] +fn de_init() { + REFERENCE_MAP.borrow_mut(|reference_map| { + std::mem::take(reference_map); + }); +} + /// ### Experimental feature /// /// Create a `reference` from `Class` instance. diff --git a/crates/napi/src/bindgen_runtime/mod.rs b/crates/napi/src/bindgen_runtime/mod.rs index 436e83894d..2019727be6 100644 --- a/crates/napi/src/bindgen_runtime/mod.rs +++ b/crates/napi/src/bindgen_runtime/mod.rs @@ -76,7 +76,7 @@ pub unsafe extern "C" fn drop_buffer( #[allow(unused)] finalize_data: *mut c_void, finalize_hint: *mut c_void, ) { - #[cfg(debug_assertions)] + #[cfg(all(debug_assertions, not(windows)))] { js_values::BUFFER_DATA.with(|buffer_data| { let mut buffer = buffer_data.lock().expect("Unlock Buffer data failed"); diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index f0d0024e45..a39212f8ce 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -1,21 +1,10 @@ use std::collections::{HashMap, HashSet}; -#[cfg(feature = "napi4")] -use std::ffi::c_void; -use std::ffi::CStr; -use std::mem; -#[cfg(feature = "napi4")] -use std::os::raw::c_char; +use std::ffi::{c_void, CStr}; use std::ptr; -use std::sync::{ - atomic::{AtomicBool, AtomicPtr, Ordering}, - Mutex, -}; -#[cfg(feature = "napi4")] -use std::thread::ThreadId; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; use once_cell::sync::Lazy; -#[cfg(feature = "napi4")] -use once_cell::sync::OnceCell; use crate::{ check_status, check_status_or_throw, sys, Env, JsError, JsFunction, Property, Result, Value, @@ -26,97 +15,118 @@ pub type ExportRegisterCallback = unsafe fn(sys::napi_env) -> Result Result<()>; -#[cfg(feature = "napi4")] -pub(crate) static CUSTOM_GC_TSFN: AtomicPtr = - AtomicPtr::new(ptr::null_mut()); -#[cfg(feature = "napi4")] -// CustomGC ThreadsafeFunction may be deleted during the process exit. -// And there may still some Buffer alive after that. -pub(crate) static CUSTOM_GC_TSFN_CLOSED: AtomicBool = AtomicBool::new(false); -#[cfg(feature = "napi4")] -pub(crate) static MAIN_THREAD_ID: OnceCell = OnceCell::new(); - -struct PersistedSingleThreadVec { - inner: Mutex>, +struct PersistedPerInstanceVec { + inner: AtomicPtr, + length: AtomicUsize, } -impl Default for PersistedSingleThreadVec { +impl Default for PersistedPerInstanceVec { fn default() -> Self { - PersistedSingleThreadVec { - inner: Mutex::new(Vec::new()), - } + let mut vec: Vec = Vec::with_capacity(1); + let ret = Self { + inner: AtomicPtr::new(vec.as_mut_ptr()), + length: AtomicUsize::new(0), + }; + std::mem::forget(vec); + ret } } -impl PersistedSingleThreadVec { +impl PersistedPerInstanceVec { #[allow(clippy::mut_from_ref)] fn borrow_mut(&self, f: F) where F: FnOnce(&mut [T]), { - let mut locked = self - .inner - .lock() - .expect("Acquire persisted thread vec lock failed"); - f(&mut locked); + let length = self.length.load(Ordering::Relaxed); + if length == 0 { + f(&mut []); + } else { + let inner = self.inner.load(Ordering::Relaxed); + let mut temp = unsafe { Vec::from_raw_parts(inner, length, length) }; + f(temp.as_mut_slice()); + // Inner Vec has been reallocated, so we need to update the pointer + if temp.as_mut_ptr() != inner { + self.inner.store(temp.as_mut_ptr(), Ordering::Relaxed); + } + self.length.store(temp.len(), Ordering::Relaxed); + std::mem::forget(temp); + } } fn push(&self, item: T) { - let mut locked = self - .inner - .lock() - .expect("Acquire persisted thread vec lock failed"); - locked.push(item); + let length = self.length.load(Ordering::Relaxed); + let inner = self.inner.load(Ordering::Relaxed); + let mut temp = unsafe { Vec::from_raw_parts(inner, length, length) }; + temp.push(item); + // Inner Vec has been reallocated, so we need to update the pointer + if temp.as_mut_ptr() != inner { + self.inner.store(temp.as_mut_ptr(), Ordering::Relaxed); + } + std::mem::forget(temp); + + self.length.fetch_add(1, Ordering::Relaxed); } } -unsafe impl Send for PersistedSingleThreadVec {} -unsafe impl Sync for PersistedSingleThreadVec {} +unsafe impl Send for PersistedPerInstanceVec {} +unsafe impl Sync for PersistedPerInstanceVec {} -pub(crate) struct PersistedSingleThreadHashMap(Mutex>); +pub(crate) struct PersistedPerInstanceHashMap(*mut HashMap); -impl PersistedSingleThreadHashMap { +impl PersistedPerInstanceHashMap { #[allow(clippy::mut_from_ref)] pub(crate) fn borrow_mut(&self, f: F) -> R where F: FnOnce(&mut HashMap) -> R, { - let mut lock = self - .0 - .lock() - .expect("Acquire persisted thread hash map lock failed"); - f(&mut *lock) + f(unsafe { Box::leak(Box::from_raw(self.0)) }) } } -impl Default for PersistedSingleThreadHashMap { +impl Default for PersistedPerInstanceHashMap { fn default() -> Self { - PersistedSingleThreadHashMap(Mutex::new(Default::default())) + let map = Default::default(); + Self(Box::into_raw(Box::new(map))) } } type ModuleRegisterCallback = - PersistedSingleThreadVec<(Option<&'static str>, (&'static str, ExportRegisterCallback))>; + PersistedPerInstanceVec<(Option<&'static str>, (&'static str, ExportRegisterCallback))>; -type ModuleClassProperty = PersistedSingleThreadHashMap< +type ModuleClassProperty = PersistedPerInstanceHashMap< &'static str, HashMap, (&'static str, Vec)>, >; -unsafe impl Send for PersistedSingleThreadHashMap {} -unsafe impl Sync for PersistedSingleThreadHashMap {} +unsafe impl Send for PersistedPerInstanceHashMap {} +unsafe impl Sync for PersistedPerInstanceHashMap {} type FnRegisterMap = - PersistedSingleThreadHashMap; + PersistedPerInstanceHashMap; static MODULE_REGISTER_CALLBACK: Lazy = Lazy::new(Default::default); static MODULE_CLASS_PROPERTIES: Lazy = Lazy::new(Default::default); -static MODULE_REGISTER_LOCK: Lazy> = Lazy::new(|| Mutex::new(())); -static REGISTERED: Lazy = Lazy::new(|| AtomicBool::new(false)); +static REGISTERED: AtomicBool = AtomicBool::new(false); static REGISTERED_CLASSES: Lazy>> = Lazy::new(thread_local::ThreadLocal::new); static FN_REGISTER_MAP: Lazy = Lazy::new(Default::default); +#[ctor::dtor] +fn destroy() { + { + let ptr = MODULE_REGISTER_CALLBACK.inner.load(Ordering::Relaxed); + let len = MODULE_REGISTER_CALLBACK.length.load(Ordering::Relaxed); + unsafe { Vec::from_raw_parts(ptr, len, len) }; + } + { + unsafe { Box::from_raw(MODULE_CLASS_PROPERTIES.0) }; + } + { + unsafe { Box::from_raw(FN_REGISTER_MAP.0) }; + } +} + #[inline] fn wait_first_thread_registered() { while !REGISTERED.load(Ordering::SeqCst) { @@ -130,7 +140,7 @@ type RegisteredClasses = #[cfg(feature = "compat-mode")] // compatibility for #[module_exports] -static MODULE_EXPORTS: Lazy> = +static MODULE_EXPORTS: Lazy> = Lazy::new(Default::default); #[doc(hidden)] @@ -271,20 +281,21 @@ pub fn get_c_callback(raw_fn: ExportRegisterCallback) -> Result }) } +#[cfg(windows)] +#[ctor::ctor] +fn load_host() { + unsafe { + sys::setup(); + } +} + #[no_mangle] unsafe extern "C" fn napi_register_module_v1( env: sys::napi_env, exports: sys::napi_value, ) -> sys::napi_value { - #[cfg(windows)] - unsafe { - sys::setup(); - } crate::__private::___CALL_FROM_FACTORY.get_or_default(); let registered_classes_ptr = REGISTERED_CLASSES.get_or_default(); - let lock = MODULE_REGISTER_LOCK - .lock() - .expect("Failed to acquire module register lock"); let mut exports_objects: HashSet = HashSet::default(); MODULE_REGISTER_CALLBACK.borrow_mut(|inner| { inner @@ -462,29 +473,27 @@ unsafe extern "C" fn napi_register_module_v1( }) }); - #[cfg(all(feature = "tokio_rt", feature = "napi4"))] + #[cfg(feature = "napi3")] { - let _ = crate::tokio_runtime::RT.clone(); - crate::tokio_runtime::TOKIO_RT_REF_COUNT.fetch_add(1, Ordering::SeqCst); - assert_eq!( - unsafe { - sys::napi_add_env_cleanup_hook( - env, - Some(crate::shutdown_tokio_rt), - env as *mut std::ffi::c_void, - ) - }, - sys::Status::napi_ok - ); + unsafe { + sys::napi_add_env_cleanup_hook(env, Some(remove_registered_classes), env as *mut c_void) + }; } - mem::drop(lock); - - #[cfg(feature = "napi4")] - create_custom_gc(env); REGISTERED.store(true, Ordering::SeqCst); exports } +unsafe extern "C" fn remove_registered_classes(env: *mut c_void) { + let env = env as sys::napi_env; + if let Some(s) = REGISTERED_CLASSES.get() { + let registered_classes = unsafe { Box::from_raw(s.load(Ordering::Relaxed)) }; + registered_classes.iter().for_each(|(_, v)| { + unsafe { sys::napi_delete_reference(env, *v) }; + }); + s.store(ptr::null_mut(), Ordering::Relaxed); + } +} + pub(crate) unsafe extern "C" fn noop( env: sys::napi_env, _info: sys::napi_callback_info, @@ -502,99 +511,3 @@ pub(crate) unsafe extern "C" fn noop( } ptr::null_mut() } - -#[cfg(feature = "napi4")] -fn create_custom_gc(env: sys::napi_env) { - let mut custom_gc_fn = ptr::null_mut(); - check_status_or_throw!( - env, - unsafe { - sys::napi_create_function( - env, - "custom_gc".as_ptr() as *const c_char, - 9, - Some(empty), - ptr::null_mut(), - &mut custom_gc_fn, - ) - }, - "Create Custom GC Function in napi_register_module_v1 failed" - ); - let mut async_resource_name = ptr::null_mut(); - check_status_or_throw!( - env, - unsafe { - sys::napi_create_string_utf8( - env, - "CustomGC".as_ptr() as *const c_char, - 8, - &mut async_resource_name, - ) - }, - "Create async resource string in napi_register_module_v1 napi_register_module_v1" - ); - let mut custom_gc_tsfn = ptr::null_mut(); - check_status_or_throw!( - env, - unsafe { - sys::napi_create_threadsafe_function( - env, - custom_gc_fn, - ptr::null_mut(), - async_resource_name, - 0, - 1, - ptr::null_mut(), - Some(custom_gc_finalize), - ptr::null_mut(), - Some(custom_gc), - &mut custom_gc_tsfn, - ) - }, - "Create Custom GC ThreadsafeFunction in napi_register_module_v1 failed" - ); - check_status_or_throw!( - env, - unsafe { sys::napi_unref_threadsafe_function(env, custom_gc_tsfn) }, - "Unref Custom GC ThreadsafeFunction in napi_register_module_v1 failed" - ); - CUSTOM_GC_TSFN.store(custom_gc_tsfn, Ordering::SeqCst); - MAIN_THREAD_ID.get_or_init(|| std::thread::current().id()); -} - -#[cfg(feature = "napi4")] -#[allow(unused)] -unsafe extern "C" fn empty(env: sys::napi_env, info: sys::napi_callback_info) -> sys::napi_value { - ptr::null_mut() -} - -#[cfg(feature = "napi4")] -#[allow(unused)] -unsafe extern "C" fn custom_gc_finalize( - env: sys::napi_env, - finalize_data: *mut c_void, - finalize_hint: *mut c_void, -) { - CUSTOM_GC_TSFN_CLOSED.store(true, Ordering::SeqCst); -} - -#[cfg(feature = "napi4")] -// recycle the ArrayBuffer/Buffer Reference if the ArrayBuffer/Buffer is not dropped on the main thread -extern "C" fn custom_gc( - env: sys::napi_env, - _js_callback: sys::napi_value, - _context: *mut c_void, - data: *mut c_void, -) { - let mut ref_count = 0; - check_status_or_throw!( - env, - unsafe { sys::napi_reference_unref(env, data as sys::napi_ref, &mut ref_count) }, - "Failed to unref Buffer reference in Custom GC" - ); - check_status_or_throw!( - env, - unsafe { sys::napi_delete_reference(env, data as sys::napi_ref) }, - "Failed to delete Buffer reference in Custom GC" - ); -} diff --git a/crates/napi/src/lib.rs b/crates/napi/src/lib.rs index 9d5de8a66f..09323b7550 100644 --- a/crates/napi/src/lib.rs +++ b/crates/napi/src/lib.rs @@ -109,8 +109,6 @@ pub use error::*; pub use js_values::*; pub use status::Status; pub use task::Task; -#[cfg(all(feature = "tokio_rt", feature = "napi4"))] -pub use tokio_runtime::shutdown_tokio_rt; pub use value_type::*; pub use version::NodeVersion; #[cfg(feature = "serde-json")] diff --git a/crates/napi/src/tokio_runtime.rs b/crates/napi/src/tokio_runtime.rs index 932d387df1..b2df5be29c 100644 --- a/crates/napi/src/tokio_runtime.rs +++ b/crates/napi/src/tokio_runtime.rs @@ -1,7 +1,5 @@ -use std::ffi::c_void; use std::future::Future; use std::ptr; -use std::sync::atomic::{AtomicUsize, Ordering}; use once_cell::sync::Lazy; use tokio::{ @@ -29,27 +27,17 @@ pub(crate) static RT: Lazy<(Handle, mpsc::Sender<()>)> = Lazy::new(|| { .expect("Create tokio runtime failed") }); -pub(crate) static TOKIO_RT_REF_COUNT: AtomicUsize = AtomicUsize::new(0); - -#[doc(hidden)] -#[inline(never)] -pub unsafe extern "C" fn shutdown_tokio_rt(arg: *mut c_void) { - if TOKIO_RT_REF_COUNT.fetch_sub(1, Ordering::SeqCst) == 0 { - let sender = &RT.1; - if let Err(e) = sender.clone().try_send(()) { - match e { - TrySendError::Closed(_) => {} - TrySendError::Full(_) => { - panic!("Send shutdown signal to tokio runtime failed, queue is full"); - } +#[ctor::dtor] +fn shutdown_tokio() { + let sender = &RT.1; + if let Err(e) = sender.clone().try_send(()) { + match e { + TrySendError::Closed(_) => {} + TrySendError::Full(_) => { + panic!("Send shutdown signal to tokio runtime failed, queue is full"); } } } - - unsafe { - let env: sys::napi_env = arg as *mut sys::napi_env__; - sys::napi_remove_env_cleanup_hook(env, Some(shutdown_tokio_rt), arg); - } } /// Spawns a future onto the Tokio runtime. diff --git a/crates/sys/src/functions.rs b/crates/sys/src/functions.rs index 6d24f50107..3041d1e4e7 100644 --- a/crates/sys/src/functions.rs +++ b/crates/sys/src/functions.rs @@ -738,7 +738,7 @@ pub use napi7::*; pub use napi8::*; #[cfg(windows)] -pub(super) unsafe fn load() -> Result<(), libloading::Error> { +pub(super) unsafe fn load() -> Result { let host = match libloading::os::windows::Library::this() { Ok(lib) => lib.into(), Err(err) => { @@ -764,5 +764,5 @@ pub(super) unsafe fn load() -> Result<(), libloading::Error> { napi8::load(&host)?; #[cfg(feature = "experimental")] experimental::load(&host)?; - Ok(()) + Ok(host) } diff --git a/crates/sys/src/lib.rs b/crates/sys/src/lib.rs index a0d1262132..5cf4fa5b29 100644 --- a/crates/sys/src/lib.rs +++ b/crates/sys/src/lib.rs @@ -77,28 +77,21 @@ macro_rules! generate { }; } -#[cfg(windows)] -use std::sync::Once; - mod functions; mod types; pub use functions::*; pub use types::*; -#[cfg(windows)] -static SETUP: Once = Once::new(); - /// Loads N-API symbols from host process. /// Must be called at least once before using any functions in bindings or /// they will panic. /// Safety: `env` must be a valid `napi_env` for the current thread #[cfg(windows)] #[allow(clippy::missing_safety_doc)] -pub unsafe fn setup() { - SETUP.call_once(|| { - if let Err(err) = load() { - panic!("{}", err); - } - }); +pub unsafe fn setup() -> libloading::Library { + match load() { + Err(err) => panic!("{}", err), + Ok(l) => l, + } } From c01bcecb2b053dc155138d70746ac9c091b788c4 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 16 Dec 2022 14:32:01 +0800 Subject: [PATCH 2/7] chore(napi): reduce the complex about destroying tokio runtime --- .../src/bindgen_runtime/module_register.rs | 19 +------- crates/napi/src/tokio_runtime.rs | 44 +++---------------- examples/napi/__test__/unload.spec.js | 11 +++++ 3 files changed, 19 insertions(+), 55 deletions(-) create mode 100644 examples/napi/__test__/unload.spec.js diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index a39212f8ce..41388fe384 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::ffi::{c_void, CStr}; +use std::ffi::CStr; use std::ptr; use std::sync::atomic::AtomicUsize; use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; @@ -473,27 +473,10 @@ unsafe extern "C" fn napi_register_module_v1( }) }); - #[cfg(feature = "napi3")] - { - unsafe { - sys::napi_add_env_cleanup_hook(env, Some(remove_registered_classes), env as *mut c_void) - }; - } REGISTERED.store(true, Ordering::SeqCst); exports } -unsafe extern "C" fn remove_registered_classes(env: *mut c_void) { - let env = env as sys::napi_env; - if let Some(s) = REGISTERED_CLASSES.get() { - let registered_classes = unsafe { Box::from_raw(s.load(Ordering::Relaxed)) }; - registered_classes.iter().for_each(|(_, v)| { - unsafe { sys::napi_delete_reference(env, *v) }; - }); - s.store(ptr::null_mut(), Ordering::Relaxed); - } -} - pub(crate) unsafe extern "C" fn noop( env: sys::napi_env, _info: sys::napi_callback_info, diff --git a/crates/napi/src/tokio_runtime.rs b/crates/napi/src/tokio_runtime.rs index b2df5be29c..7c7ac709c7 100644 --- a/crates/napi/src/tokio_runtime.rs +++ b/crates/napi/src/tokio_runtime.rs @@ -2,43 +2,12 @@ use std::future::Future; use std::ptr; use once_cell::sync::Lazy; -use tokio::{ - runtime::Handle, - sync::mpsc::{self, error::TrySendError}, -}; +use tokio::runtime::Runtime; use crate::{check_status, sys, JsDeferred, JsUnknown, NapiValue, Result}; -pub(crate) static RT: Lazy<(Handle, mpsc::Sender<()>)> = Lazy::new(|| { - let runtime = tokio::runtime::Runtime::new(); - let (sender, mut receiver) = mpsc::channel::<()>(1); - runtime - .map(|rt| { - let h = rt.handle(); - let handle = h.clone(); - handle.spawn(async move { - if receiver.recv().await.is_some() { - rt.shutdown_background(); - } - }); - - (handle, sender) - }) - .expect("Create tokio runtime failed") -}); - -#[ctor::dtor] -fn shutdown_tokio() { - let sender = &RT.1; - if let Err(e) = sender.clone().try_send(()) { - match e { - TrySendError::Closed(_) => {} - TrySendError::Full(_) => { - panic!("Send shutdown signal to tokio runtime failed, queue is full"); - } - } - } -} +pub(crate) static RT: Lazy = + Lazy::new(|| tokio::runtime::Runtime::new().expect("Create tokio runtime failed")); /// Spawns a future onto the Tokio runtime. /// @@ -48,7 +17,7 @@ pub fn spawn(fut: F) -> tokio::task::JoinHandle where F: 'static + Send + Future, { - RT.0.spawn(fut) + RT.spawn(fut) } /// Runs a future to completion @@ -58,7 +27,7 @@ pub fn block_on(fut: F) -> F::Output where F: 'static + Send + Future, { - RT.0.block_on(fut) + RT.block_on(fut) } // This function's signature must be kept in sync with the one in lib.rs, otherwise napi @@ -66,8 +35,9 @@ where /// If the feature `tokio_rt` has been enabled this will enter the runtime context and /// then call the provided closure. Otherwise it will just call the provided closure. +#[inline] pub fn within_runtime_if_available T, T>(f: F) -> T { - let _rt_guard = RT.0.enter(); + let _rt_guard = RT.enter(); f() } diff --git a/examples/napi/__test__/unload.spec.js b/examples/napi/__test__/unload.spec.js new file mode 100644 index 0000000000..21a32ca297 --- /dev/null +++ b/examples/napi/__test__/unload.spec.js @@ -0,0 +1,11 @@ +// use the commonjs syntax to prevent compiler from transpiling the module syntax + +const test = require('ava').default + +test('unload module', (t) => { + const { add } = require('../index.node') + t.is(add(1, 2), 3) + delete require.cache[require.resolve('../index.node')] + const { add: add2 } = require('../index.node') + t.is(add2(1, 2), 3) +}) From 5bd6c78b5afc91c3caae6c56fc9f6b050df49806 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 16 Dec 2022 14:35:30 +0800 Subject: [PATCH 3/7] chore(napi): expose tokio runtime --- crates/napi/src/tokio_runtime.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/napi/src/tokio_runtime.rs b/crates/napi/src/tokio_runtime.rs index 7c7ac709c7..a3e4910374 100644 --- a/crates/napi/src/tokio_runtime.rs +++ b/crates/napi/src/tokio_runtime.rs @@ -9,6 +9,10 @@ use crate::{check_status, sys, JsDeferred, JsUnknown, NapiValue, Result}; pub(crate) static RT: Lazy = Lazy::new(|| tokio::runtime::Runtime::new().expect("Create tokio runtime failed")); +pub fn runtime() -> &'static Runtime { + &RT +} + /// Spawns a future onto the Tokio runtime. /// /// Depending on where you use it, you should await or abort the future in your drop function. From e88fbcc404e006840decc74f4c9017525bc358ee Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 16 Dec 2022 15:54:29 +0800 Subject: [PATCH 4/7] chore(napi): remove more thread_local usage --- .../src/bindgen_runtime/module_register.rs | 74 +++++++++++++++---- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index 41388fe384..a1ca06760c 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -1,8 +1,9 @@ use std::collections::{HashMap, HashSet}; -use std::ffi::CStr; +use std::ffi::{c_void, CStr}; use std::ptr; use std::sync::atomic::AtomicUsize; use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; +use std::thread::ThreadId; use once_cell::sync::Lazy; @@ -20,6 +21,17 @@ struct PersistedPerInstanceVec { length: AtomicUsize, } +impl Drop for PersistedPerInstanceVec { + fn drop(&mut self) { + let length = self.length.load(Ordering::Relaxed); + if length == 0 { + return; + } + let inner = self.inner.load(Ordering::Relaxed); + unsafe { Vec::from_raw_parts(inner, length, length) }; + } +} + impl Default for PersistedPerInstanceVec { fn default() -> Self { let mut vec: Vec = Vec::with_capacity(1); @@ -74,7 +86,17 @@ unsafe impl Sync for PersistedPerInstanceVec {} pub(crate) struct PersistedPerInstanceHashMap(*mut HashMap); +impl Drop for PersistedPerInstanceHashMap { + fn drop(&mut self) { + unsafe { Box::from_raw(self.0) }; + } +} + impl PersistedPerInstanceHashMap { + pub(crate) fn from_hashmap(hashmap: HashMap) -> Self { + Self(Box::into_raw(Box::new(hashmap))) + } + #[allow(clippy::mut_from_ref)] pub(crate) fn borrow_mut(&self, f: F) -> R where @@ -104,12 +126,12 @@ unsafe impl Sync for PersistedPerInstanceHashMap {} type FnRegisterMap = PersistedPerInstanceHashMap; +type RegisteredClassesMap = PersistedPerInstanceHashMap; static MODULE_REGISTER_CALLBACK: Lazy = Lazy::new(Default::default); static MODULE_CLASS_PROPERTIES: Lazy = Lazy::new(Default::default); static REGISTERED: AtomicBool = AtomicBool::new(false); -static REGISTERED_CLASSES: Lazy>> = - Lazy::new(thread_local::ThreadLocal::new); +static REGISTERED_CLASSES: Lazy = Lazy::new(Default::default); static FN_REGISTER_MAP: Lazy = Lazy::new(Default::default); #[ctor::dtor] @@ -135,21 +157,22 @@ fn wait_first_thread_registered() { } type RegisteredClasses = - HashMap; + PersistedPerInstanceHashMap; #[cfg(feature = "compat-mode")] // compatibility for #[module_exports] - static MODULE_EXPORTS: Lazy> = Lazy::new(Default::default); #[doc(hidden)] pub fn get_class_constructor(js_name: &'static str) -> Option { wait_first_thread_registered(); - let registered_classes = REGISTERED_CLASSES.get().unwrap(); - let registered_classes = - Box::leak(unsafe { Box::from_raw(registered_classes.load(Ordering::Relaxed)) }); - registered_classes.get(js_name).copied() + let current_id = std::thread::current().id(); + REGISTERED_CLASSES.borrow_mut(|map| { + map + .get(¤t_id) + .map(|m| m.borrow_mut(|map| map.get(js_name).copied())) + })? } #[doc(hidden)] @@ -295,7 +318,6 @@ unsafe extern "C" fn napi_register_module_v1( exports: sys::napi_value, ) -> sys::napi_value { crate::__private::___CALL_FROM_FACTORY.get_or_default(); - let registered_classes_ptr = REGISTERED_CLASSES.get_or_default(); let mut exports_objects: HashSet = HashSet::default(); MODULE_REGISTER_CALLBACK.borrow_mut(|inner| { inner @@ -371,7 +393,7 @@ unsafe extern "C" fn napi_register_module_v1( }) }); - let mut registered_classes: RegisteredClasses = + let mut registered_classes = HashMap::with_capacity(MODULE_CLASS_PROPERTIES.borrow_mut(|inner| inner.len())); MODULE_CLASS_PROPERTIES.borrow_mut(|inner| { @@ -458,10 +480,13 @@ unsafe extern "C" fn napi_register_module_v1( } } }); - registered_classes_ptr.store( - Box::into_raw(Box::new(registered_classes)), - Ordering::Relaxed, - ); + + REGISTERED_CLASSES.borrow_mut(|map| { + map.insert( + std::thread::current().id(), + PersistedPerInstanceHashMap::from_hashmap(registered_classes), + ) + }); }); #[cfg(feature = "compat-mode")] @@ -473,10 +498,29 @@ unsafe extern "C" fn napi_register_module_v1( }) }); + #[cfg(feature = "napi3")] + { + unsafe { + sys::napi_add_env_cleanup_hook(env, Some(remove_registered_classes), env as *mut c_void) + }; + } REGISTERED.store(true, Ordering::SeqCst); exports } +unsafe extern "C" fn remove_registered_classes(env: *mut c_void) { + let env = env as sys::napi_env; + if let Some(registered_classes) = + REGISTERED_CLASSES.borrow_mut(|map| map.remove(&std::thread::current().id())) + { + registered_classes.borrow_mut(|map| { + map.iter().for_each(|(_, v)| { + unsafe { sys::napi_delete_reference(env, *v) }; + }) + }); + } +} + pub(crate) unsafe extern "C" fn noop( env: sys::napi_env, _info: sys::napi_callback_info, From 968d9e10b1d621eae69865c3b16ff403d97aaf76 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 16 Dec 2022 20:07:22 +0800 Subject: [PATCH 5/7] chore(napi): fix tokio destroy logic --- .../src/bindgen_runtime/module_register.rs | 86 ++++++------------- crates/napi/src/tokio_runtime.rs | 32 +++++-- 2 files changed, 49 insertions(+), 69 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index a1ca06760c..429ebd67d4 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::ffi::{c_void, CStr}; +use std::ffi::CStr; use std::ptr; use std::sync::atomic::AtomicUsize; use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; @@ -21,17 +21,6 @@ struct PersistedPerInstanceVec { length: AtomicUsize, } -impl Drop for PersistedPerInstanceVec { - fn drop(&mut self) { - let length = self.length.load(Ordering::Relaxed); - if length == 0 { - return; - } - let inner = self.inner.load(Ordering::Relaxed); - unsafe { Vec::from_raw_parts(inner, length, length) }; - } -} - impl Default for PersistedPerInstanceVec { fn default() -> Self { let mut vec: Vec = Vec::with_capacity(1); @@ -86,12 +75,6 @@ unsafe impl Sync for PersistedPerInstanceVec {} pub(crate) struct PersistedPerInstanceHashMap(*mut HashMap); -impl Drop for PersistedPerInstanceHashMap { - fn drop(&mut self) { - unsafe { Box::from_raw(self.0) }; - } -} - impl PersistedPerInstanceHashMap { pub(crate) fn from_hashmap(hashmap: HashMap) -> Self { Self(Box::into_raw(Box::new(hashmap))) @@ -130,32 +113,11 @@ type RegisteredClassesMap = PersistedPerInstanceHashMap = Lazy::new(Default::default); static MODULE_CLASS_PROPERTIES: Lazy = Lazy::new(Default::default); -static REGISTERED: AtomicBool = AtomicBool::new(false); +static IS_FIRST_MODULE: AtomicBool = AtomicBool::new(true); +static FIRST_MODULE_REGISTERED: AtomicBool = AtomicBool::new(false); static REGISTERED_CLASSES: Lazy = Lazy::new(Default::default); static FN_REGISTER_MAP: Lazy = Lazy::new(Default::default); -#[ctor::dtor] -fn destroy() { - { - let ptr = MODULE_REGISTER_CALLBACK.inner.load(Ordering::Relaxed); - let len = MODULE_REGISTER_CALLBACK.length.load(Ordering::Relaxed); - unsafe { Vec::from_raw_parts(ptr, len, len) }; - } - { - unsafe { Box::from_raw(MODULE_CLASS_PROPERTIES.0) }; - } - { - unsafe { Box::from_raw(FN_REGISTER_MAP.0) }; - } -} - -#[inline] -fn wait_first_thread_registered() { - while !REGISTERED.load(Ordering::SeqCst) { - std::hint::spin_loop(); - } -} - type RegisteredClasses = PersistedPerInstanceHashMap; @@ -164,9 +126,15 @@ type RegisteredClasses = static MODULE_EXPORTS: Lazy> = Lazy::new(Default::default); +#[inline] +fn wait_first_thread_registered() { + while !FIRST_MODULE_REGISTERED.load(Ordering::SeqCst) { + std::hint::spin_loop(); + } +} + #[doc(hidden)] pub fn get_class_constructor(js_name: &'static str) -> Option { - wait_first_thread_registered(); let current_id = std::thread::current().id(); REGISTERED_CLASSES.borrow_mut(|map| { map @@ -236,7 +204,6 @@ pub fn register_class( /// ``` /// pub fn get_js_function(env: &Env, raw_fn: ExportRegisterCallback) -> Result { - wait_first_thread_registered(); FN_REGISTER_MAP.borrow_mut(|inner| { inner .get(&raw_fn) @@ -290,7 +257,6 @@ pub fn get_js_function(env: &Env, raw_fn: ExportRegisterCallback) -> Result Result { - wait_first_thread_registered(); FN_REGISTER_MAP.borrow_mut(|inner| { inner .get(&raw_fn) @@ -317,6 +283,11 @@ unsafe extern "C" fn napi_register_module_v1( env: sys::napi_env, exports: sys::napi_value, ) -> sys::napi_value { + if IS_FIRST_MODULE.load(Ordering::SeqCst) { + IS_FIRST_MODULE.store(false, Ordering::SeqCst); + } else { + wait_first_thread_registered(); + } crate::__private::___CALL_FROM_FACTORY.get_or_default(); let mut exports_objects: HashSet = HashSet::default(); MODULE_REGISTER_CALLBACK.borrow_mut(|inner| { @@ -393,8 +364,7 @@ unsafe extern "C" fn napi_register_module_v1( }) }); - let mut registered_classes = - HashMap::with_capacity(MODULE_CLASS_PROPERTIES.borrow_mut(|inner| inner.len())); + let mut registered_classes = HashMap::new(); MODULE_CLASS_PROPERTIES.borrow_mut(|inner| { inner.iter().for_each(|(rust_name, js_mods)| { @@ -498,29 +468,21 @@ unsafe extern "C" fn napi_register_module_v1( }) }); - #[cfg(feature = "napi3")] + #[cfg(all(windows, feature = "napi4", feature = "tokio_rt"))] { + crate::tokio_runtime::RT_REFERENCE_COUNT.fetch_add(1, Ordering::SeqCst); unsafe { - sys::napi_add_env_cleanup_hook(env, Some(remove_registered_classes), env as *mut c_void) + sys::napi_add_env_cleanup_hook( + env, + Some(crate::tokio_runtime::drop_runtime), + ptr::null_mut(), + ) }; } - REGISTERED.store(true, Ordering::SeqCst); + FIRST_MODULE_REGISTERED.store(true, Ordering::SeqCst); exports } -unsafe extern "C" fn remove_registered_classes(env: *mut c_void) { - let env = env as sys::napi_env; - if let Some(registered_classes) = - REGISTERED_CLASSES.borrow_mut(|map| map.remove(&std::thread::current().id())) - { - registered_classes.borrow_mut(|map| { - map.iter().for_each(|(_, v)| { - unsafe { sys::napi_delete_reference(env, *v) }; - }) - }); - } -} - pub(crate) unsafe extern "C" fn noop( env: sys::napi_env, _info: sys::napi_callback_info, diff --git a/crates/napi/src/tokio_runtime.rs b/crates/napi/src/tokio_runtime.rs index a3e4910374..ef923ac58c 100644 --- a/crates/napi/src/tokio_runtime.rs +++ b/crates/napi/src/tokio_runtime.rs @@ -6,11 +6,29 @@ use tokio::runtime::Runtime; use crate::{check_status, sys, JsDeferred, JsUnknown, NapiValue, Result}; -pub(crate) static RT: Lazy = - Lazy::new(|| tokio::runtime::Runtime::new().expect("Create tokio runtime failed")); +pub(crate) static mut RT: Lazy> = Lazy::new(|| { + let runtime = tokio::runtime::Runtime::new().expect("Create tokio runtime failed"); + Some(runtime) +}); -pub fn runtime() -> &'static Runtime { - &RT +#[cfg(windows)] +pub(crate) static RT_REFERENCE_COUNT: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + +#[cfg(windows)] +pub(crate) unsafe extern "C" fn drop_runtime(arg: *mut std::ffi::c_void) { + use std::sync::atomic::Ordering; + + if RT_REFERENCE_COUNT.fetch_sub(1, Ordering::SeqCst) == 1 { + if let Some(rt) = Lazy::get_mut(unsafe { &mut RT }) { + rt.take(); + } + } + + unsafe { + let env: sys::napi_env = arg as *mut sys::napi_env__; + sys::napi_remove_env_cleanup_hook(env, Some(drop_runtime), arg); + } } /// Spawns a future onto the Tokio runtime. @@ -21,7 +39,7 @@ pub fn spawn(fut: F) -> tokio::task::JoinHandle where F: 'static + Send + Future, { - RT.spawn(fut) + unsafe { RT.as_ref() }.unwrap().spawn(fut) } /// Runs a future to completion @@ -31,7 +49,7 @@ pub fn block_on(fut: F) -> F::Output where F: 'static + Send + Future, { - RT.block_on(fut) + unsafe { RT.as_ref() }.unwrap().block_on(fut) } // This function's signature must be kept in sync with the one in lib.rs, otherwise napi @@ -41,7 +59,7 @@ where /// then call the provided closure. Otherwise it will just call the provided closure. #[inline] pub fn within_runtime_if_available T, T>(f: F) -> T { - let _rt_guard = RT.enter(); + let _rt_guard = unsafe { RT.as_ref() }.unwrap().enter(); f() } From 6e4b16fe5d689a32c8d7b689c2ce67de35fd67cd Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 16 Dec 2022 20:19:39 +0800 Subject: [PATCH 6/7] style: clippy fix --- .../napi/src/bindgen_runtime/js_values/bigint.rs | 6 +++--- .../src/bindgen_runtime/js_values/value_ref.rs | 1 + crates/napi/src/env.rs | 2 +- crates/napi/src/js_values/bigint.rs | 4 ++-- crates/napi/src/js_values/string/mod.rs | 6 +++--- examples/napi-compat-mode/src/object.rs | 2 +- examples/napi/src/either.rs | 16 ++-------------- examples/napi/src/typed_array.rs | 2 +- memory-testing/src/lib.rs | 2 +- 9 files changed, 15 insertions(+), 26 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/bigint.rs b/crates/napi/src/bindgen_runtime/js_values/bigint.rs index 2710031872..67256488fe 100644 --- a/crates/napi/src/bindgen_runtime/js_values/bigint.rs +++ b/crates/napi/src/bindgen_runtime/js_values/bigint.rs @@ -52,7 +52,7 @@ impl FromNapiValue for BigInt { ptr::null_mut(), ) })?; - let mut words: Vec = Vec::with_capacity(word_count as usize); + let mut words: Vec = Vec::with_capacity(word_count); let mut sign_bit = 0; unsafe { @@ -64,7 +64,7 @@ impl FromNapiValue for BigInt { words.as_mut_ptr(), ))?; - words.set_len(word_count as usize); + words.set_len(word_count); } if word_count == 0 { words = vec![0]; @@ -155,7 +155,7 @@ impl ToNapiValue for BigInt { impl ToNapiValue for i128 { unsafe fn to_napi_value(env: sys::napi_env, val: Self) -> crate::Result { let mut raw_value = ptr::null_mut(); - let sign_bit = if val > 0 { 0 } else { 1 }; + let sign_bit = i32::from(val <= 0); let words = &val as *const i128 as *const u64; check_status!(unsafe { sys::napi_create_bigint_words(env, sign_bit, 2, words, &mut raw_value) diff --git a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs index f12c9beea1..4ee5627bf0 100644 --- a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs +++ b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs @@ -66,6 +66,7 @@ impl Drop for Reference { impl Reference { #[doc(hidden)] + #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn add_ref(env: crate::sys::napi_env, t: *mut c_void, value: RefInformation) { REFERENCE_MAP.borrow_mut(|map| { if let Some((_, previous_ref, previous_rc)) = map.insert(t, value) { diff --git a/crates/napi/src/env.rs b/crates/napi/src/env.rs index 20849d7758..f9a9e3ec68 100644 --- a/crates/napi/src/env.rs +++ b/crates/napi/src/env.rs @@ -115,7 +115,7 @@ impl Env { #[cfg(feature = "napi6")] pub fn create_bigint_from_i128(&self, value: i128) -> Result { let mut raw_value = ptr::null_mut(); - let sign_bit = if value > 0 { 0 } else { 1 }; + let sign_bit = i32::from(value <= 0); let words = &value as *const i128 as *const u64; check_status!(unsafe { sys::napi_create_bigint_words(self.0, sign_bit, 2, words, &mut raw_value) diff --git a/crates/napi/src/js_values/bigint.rs b/crates/napi/src/js_values/bigint.rs index 245cddb164..2725c7bee0 100644 --- a/crates/napi/src/js_values/bigint.rs +++ b/crates/napi/src/js_values/bigint.rs @@ -204,7 +204,7 @@ impl TryFrom for u64 { impl JsBigInt { /// pub fn get_words(&mut self) -> Result<(bool, Vec)> { - let mut words: Vec = Vec::with_capacity(self.word_count as usize); + let mut words: Vec = Vec::with_capacity(self.word_count); let word_count = &mut self.word_count; let mut sign_bit = 0; check_status!(unsafe { @@ -218,7 +218,7 @@ impl JsBigInt { })?; unsafe { - words.set_len(self.word_count as usize); + words.set_len(self.word_count); }; Ok((sign_bit == 1, words)) diff --git a/crates/napi/src/js_values/string/mod.rs b/crates/napi/src/js_values/string/mod.rs index 0e5eeff847..313e46b155 100644 --- a/crates/napi/src/js_values/string/mod.rs +++ b/crates/napi/src/js_values/string/mod.rs @@ -35,7 +35,7 @@ impl JsString { check_status!(unsafe { sys::napi_get_value_string_utf8(self.0.env, self.0.value, ptr::null_mut(), 0, &mut length) })?; - Ok(length as usize) + Ok(length) } pub fn utf16_len(&self) -> Result { @@ -43,7 +43,7 @@ impl JsString { check_status!(unsafe { sys::napi_get_value_string_utf16(self.0.env, self.0.value, ptr::null_mut(), 0, &mut length) })?; - Ok(length as usize) + Ok(length) } pub fn latin1_len(&self) -> Result { @@ -51,7 +51,7 @@ impl JsString { check_status!(unsafe { sys::napi_get_value_string_latin1(self.0.env, self.0.value, ptr::null_mut(), 0, &mut length) })?; - Ok(length as usize) + Ok(length) } pub fn into_utf8(self) -> Result { diff --git a/examples/napi-compat-mode/src/object.rs b/examples/napi-compat-mode/src/object.rs index a8f62ca63b..bf5649d95d 100644 --- a/examples/napi-compat-mode/src/object.rs +++ b/examples/napi-compat-mode/src/object.rs @@ -87,7 +87,7 @@ fn test_delete_named_property(ctx: CallContext) -> Result { fn test_get_property(ctx: CallContext) -> Result { let obj = ctx.get::(0)?; let key = ctx.get::(1)?; - obj.get_property(&key) + obj.get_property(key) } #[js_function(1)] diff --git a/examples/napi/src/either.rs b/examples/napi/src/either.rs index ab00955bba..a316a603a0 100644 --- a/examples/napi/src/either.rs +++ b/examples/napi/src/either.rs @@ -22,13 +22,7 @@ fn either3(input: Either3) -> u32 { match input { Either3::A(s) => s.len() as u32, Either3::B(n) => n, - Either3::C(b) => { - if b { - 1 - } else { - 0 - } - } + Either3::C(b) => u32::from(b), } } @@ -42,13 +36,7 @@ fn either4(input: Either4) -> u32 { match input { Either4::A(s) => s.len() as u32, Either4::B(n) => n, - Either4::C(b) => { - if b { - 1 - } else { - 0 - } - } + Either4::C(b) => u32::from(b), Either4::D(f) => match f.v { Either::A(s) => s.len() as u32, Either::B(n) => n, diff --git a/examples/napi/src/typed_array.rs b/examples/napi/src/typed_array.rs index 9cc5ae147d..2d7caaef7b 100644 --- a/examples/napi/src/typed_array.rs +++ b/examples/napi/src/typed_array.rs @@ -69,5 +69,5 @@ impl Task for AsyncBuffer { #[napi] fn async_reduce_buffer(buf: Buffer) -> Result> { - Ok(AsyncTask::new(AsyncBuffer { buf: buf.clone() })) + Ok(AsyncTask::new(AsyncBuffer { buf })) } diff --git a/memory-testing/src/lib.rs b/memory-testing/src/lib.rs index b41c13627b..1b0c432d35 100644 --- a/memory-testing/src/lib.rs +++ b/memory-testing/src/lib.rs @@ -75,7 +75,7 @@ pub fn test_async(env: Env) -> napi::Result { #[napi] pub fn from_js(env: Env, input_object: Object) -> napi::Result { - let a: Welcome = env.from_js_value(&input_object)?; + let a: Welcome = env.from_js_value(input_object)?; Ok(serde_json::to_string(&a)?) } From e370dee545b678badb60f491ff772619ae9bd929 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 16 Dec 2022 21:50:49 +0800 Subject: [PATCH 7/7] chore(napi): remove useless de_init --- crates/napi/src/bindgen_runtime/js_values/value_ref.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs index 4ee5627bf0..0c911680fc 100644 --- a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs +++ b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs @@ -19,13 +19,6 @@ type RefInformation = ( pub(crate) static REFERENCE_MAP: Lazy> = Lazy::new(Default::default); -#[ctor::dtor] -fn de_init() { - REFERENCE_MAP.borrow_mut(|reference_map| { - std::mem::take(reference_map); - }); -} - /// ### Experimental feature /// /// Create a `reference` from `Class` instance.