naskya/napi-rs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

fix(napi): memory leak while using Reference (#1954)

Browse source 
- Close https://github.com/napi-rs/napi-rs/issues/1952
This commit is contained in:
LongYinan 2024-02-17 22:14:27 +08:00 committed by GitHub
parent 09efd416e5
commit c49309fdc2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 57 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
crates/backend/src/codegen/fn.rs
View file

@ -267,11 +267,16 @@ impl NapiFn {
{
if let Some(p) = path.path.segments.first() {
if p.ident == *self.parent.as_ref().unwrap() {
args.push(
quote! { napi::bindgen_prelude::Reference::from_value_ptr(this_ptr.cast(), env)? },
);
args.push(quote! {
napi::bindgen_prelude::Reference::from_value_ptr(this_ptr.cast(), env)?
});
skipped_arg_count += 1;
continue;
} else {
bail_span!(
p,
"The `T` of Reference<T> type must be the same as the class type"
)
}
}
}
@ -551,16 +556,20 @@ impl NapiFn {
let ty_string = ty.into_token_stream().to_string();
let is_return_self = ty_string == "& Self" || ty_string == "&mut Self";
if self.kind == FnKind::Constructor {
let parent = self
.parent
.as_ref()
.expect("Parent must exist for constructor");
if self.is_ret_result {
if self.parent_is_generator {
quote! { cb.construct_generator(#js_name, #ret?) }
quote! { cb.construct_generator::<false, #parent>(#js_name, #ret?) }
} else {
quote! { cb.construct(#js_name, #ret?) }
quote! { cb.construct::<false, #parent>(#js_name, #ret?) }
}
} else if self.parent_is_generator {
quote! { cb.construct_generator(#js_name, #ret) }
quote! { cb.construct_generator::<false, #parent>(#js_name, #ret) }
} else {
quote! { cb.construct(#js_name, #ret) }
quote! { cb.construct::<false, #parent>(#js_name, #ret) }
}
} else if self.kind == FnKind::Factory {
if self.is_ret_result {

12
crates/backend/src/codegen/struct.rs
View file

@ -227,10 +227,12 @@ impl NapiStruct {
quote! { #name {#(#fields),*} }
};
let is_empty_struct_hint = fields_len == 0;
let constructor = if self.implement_iterator {
quote! { unsafe { cb.construct_generator(#js_name_str, #construct) } }
quote! { unsafe { cb.construct_generator::<#is_empty_struct_hint, #name>(#js_name_str, #construct) } }
} else {
quote! { unsafe { cb.construct(#js_name_str, #construct) } }
quote! { unsafe { cb.construct::<#is_empty_struct_hint, #name>(#js_name_str, #construct) } }
};
quote! {
@ -280,7 +282,7 @@ impl NapiStruct {
) -> napi::Result<napi::bindgen_prelude::sys::napi_value> {
if let Some(ctor_ref) = napi::__private::get_class_constructor(#js_name_str) {
let wrapped_value = Box::into_raw(Box::new(val));
let instance_value = #name::new_instance(env, wrapped_value as *mut std::ffi::c_void, ctor_ref)?;
let instance_value = #name::new_instance(env, wrapped_value.cast(), ctor_ref)?;
#iterator_implementation
Ok(instance_value)
} else {
@ -298,12 +300,12 @@ impl NapiStruct {
if let Some(ctor_ref) = napi::bindgen_prelude::get_class_constructor(#js_name_str) {
unsafe {
let wrapped_value = Box::into_raw(Box::new(val));
let instance_value = #name::new_instance(env.raw(), wrapped_value as *mut std::ffi::c_void, ctor_ref)?;
let instance_value = #name::new_instance(env.raw(), wrapped_value.cast(), ctor_ref)?;
{
let env = env.raw();
#iterator_implementation
}
napi::bindgen_prelude::Reference::<#name>::from_value_ptr(wrapped_value as *mut std::ffi::c_void, env.raw())
napi::bindgen_prelude::Reference::<#name>::from_value_ptr(wrapped_value.cast(), env.raw())
}
} else {
Err(napi::bindgen_prelude::Error::new(

34
crates/napi/src/bindgen_runtime/callback_info.rs
View file

@ -12,6 +12,10 @@ thread_local! {
pub static ___CALL_FROM_FACTORY: AtomicBool = AtomicBool::new(false);
}
#[repr(transparent)]
struct EmptyStructPlaceholder(u8);
#[doc(hidden)]
pub struct CallbackInfo<const N: usize> {
env: sys::napi_env,
pub this: sys::napi_value,
@ -84,14 +88,19 @@ impl<const N: usize> CallbackInfo<N> {
self.this
}
fn _construct<T: ObjectFinalize + 'static>(
fn _construct<const IsEmptyStructHint: bool, T: ObjectFinalize + 'static>(
&self,
js_name: &str,
obj: T,
) -> Result<(sys::napi_value, *mut T)> {
let obj = Box::new(obj);
let this = self.this();
let value_ref = Box::into_raw(obj);
let mut value_ref = Box::into_raw(obj);
// for empty struct like `#[napi] struct A;`, the `value_ref` will be `0x1`
// and it will be overwritten by the others instance of the same class
if IsEmptyStructHint || value_ref as usize == 0x1 {
value_ref = Box::into_raw(Box::new(EmptyStructPlaceholder(0))).cast();
}
let mut object_ref = ptr::null_mut();
let initial_finalize: Box<dyn FnOnce()> = Box::new(|| {});
let finalize_callbacks_ptr = Rc::into_raw(Rc::new(Cell::new(Box::into_raw(initial_finalize))));
@ -118,20 +127,25 @@ impl<const N: usize> CallbackInfo<N> {
Ok((this, value_ref))
}
pub fn construct<T: ObjectFinalize + 'static>(
pub fn construct<const IsEmptyStructHint: bool, T: ObjectFinalize + 'static>(
&self,
js_name: &str,
obj: T,
) -> Result<sys::napi_value> {
self._construct(js_name, obj).map(|(v, _)| v)
self
._construct::<IsEmptyStructHint, T>(js_name, obj)
.map(|(v, _)| v)
}
pub fn construct_generator<T: Generator + ObjectFinalize + 'static>(
pub fn construct_generator<
const IsEmptyStructHint: bool,
T: Generator + ObjectFinalize + 'static,
>(
&self,
js_name: &str,
obj: T,
) -> Result<sys::napi_value> {
let (instance, generator_ptr) = self._construct(js_name, obj)?;
let (instance, generator_ptr) = self._construct::<IsEmptyStructHint, T>(js_name, obj)?;
crate::__private::create_iterator(self.env, instance, generator_ptr);
Ok(instance)
}
@ -187,7 +201,13 @@ impl<const N: usize> CallbackInfo<N> {
let initial_finalize: Box<dyn FnOnce()> = Box::new(|| {});
let finalize_callbacks_ptr = Rc::into_raw(Rc::new(Cell::new(Box::into_raw(initial_finalize))));
let mut object_ref = ptr::null_mut();
let value_ref = Box::into_raw(obj);
let mut value_ref = Box::into_raw(obj);
// for empty struct like `#[napi] struct A;`, the `value_ref` will be `0x1`
// and it will be overwritten by the others instance of the same class
if value_ref as usize == 0x1 {
value_ref = Box::into_raw(Box::new(EmptyStructPlaceholder(0))).cast();
}
check_status!(
unsafe {
sys::napi_wrap(

19
crates/napi/src/bindgen_runtime/js_values/value_ref.rs
View file

@ -7,9 +7,9 @@ use std::rc::{Rc, Weak};
use crate::{bindgen_runtime::ToNapiValue, check_status, Env, Error, Result, Status};
type RefInformation = (
*mut c_void,
crate::sys::napi_ref,
*const Cell<*mut dyn FnOnce()>,
/* wrapped_value */ *mut c_void,
/* napi_ref */ crate::sys::napi_ref,
/* finalize_callback */ *const Cell<*mut dyn FnOnce()>,
);
thread_local! {
@ -27,9 +27,6 @@ pub struct Reference<T: 'static> {
finalize_callbacks: Rc<Cell<*mut dyn FnOnce()>>,
}
unsafe impl<T: Send> Send for Reference<T> {}
unsafe impl<T: Sync> Sync for Reference<T> {}
impl<T> Drop for Reference<T> {
fn drop(&mut self) {
let rc_strong_count = Rc::strong_count(&self.finalize_callbacks);
@ -71,8 +68,9 @@ impl<T: 'static> Reference<T> {
if let Some((wrapped_value, napi_ref, finalize_callbacks_ptr)) =
REFERENCE_MAP.with(|map| map.borrow().get(&t).cloned())
{
let mut ref_count = 0;
check_status!(
unsafe { crate::sys::napi_reference_ref(env, napi_ref, &mut 0) },
unsafe { crate::sys::napi_reference_ref(env, napi_ref, &mut ref_count) },
"Failed to ref napi reference"
)?;
let finalize_callbacks_raw = unsafe { Rc::from_raw(finalize_callbacks_ptr) };
@ -80,9 +78,9 @@ impl<T: 'static> Reference<T> {
// Leak the raw finalize callbacks
Rc::into_raw(finalize_callbacks_raw);
Ok(Self {
raw: wrapped_value as *mut T,
raw: wrapped_value.cast(),
napi_ref,
env: env as *mut c_void,
env: env.cast(),
finalize_callbacks,
})
} else {
@ -243,9 +241,6 @@ pub struct SharedReference<T: 'static, S: 'static> {
owner: Reference<T>,
}
unsafe impl<T: Send, S: Send> Send for SharedReference<T, S> {}
unsafe impl<T: Sync, S: Sync> Sync for SharedReference<T, S> {}
impl<T: 'static, S: 'static> SharedReference<T, S> {
pub fn clone(&self, env: Env) -> Result<Self> {
Ok(SharedReference {