Skip to content

Commit

Permalink
fix(napi): array buffer drop notify never be called in cloned buffer
Browse files Browse the repository at this point in the history
  • Loading branch information
Brooooooklyn committed Jan 9, 2023
1 parent 5ab4b81 commit 9c3e7ca
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs
Expand Up @@ -17,7 +17,7 @@ use super::{FromNapiValue, ToNapiValue, TypeName, ValidateNapiValue};
trait Finalizer {
type RustType;

fn finalizer(&mut self) -> Box<dyn FnOnce(*mut Self::RustType, usize)>;
fn finalizer_notify(&self) -> *mut dyn FnOnce(*mut Self::RustType, usize);

fn data_managed_type(&self) -> &DataManagedType;

Expand All @@ -37,16 +37,16 @@ macro_rules! impl_typed_array {
// Use `Arc` for ref count
// Use `AtomicBool` for flag to indicate whether the value is dropped in VM
drop_in_vm: Arc<AtomicBool>,
finalizer_notify: Box<dyn FnOnce(*mut $rust_type, usize)>,
finalizer_notify: *mut dyn FnOnce(*mut $rust_type, usize),
}

unsafe impl Send for $name {}

impl Finalizer for $name {
type RustType = $rust_type;

fn finalizer(&mut self) -> Box<dyn FnOnce(*mut Self::RustType, usize)> {
std::mem::replace(&mut self.finalizer_notify, Box::new($name::noop_finalize))
fn finalizer_notify(&self) -> *mut dyn FnOnce(*mut Self::RustType, usize) {
self.finalizer_notify
}

fn data_managed_type(&self) -> &DataManagedType {
Expand Down Expand Up @@ -110,8 +110,7 @@ macro_rules! impl_typed_array {
unsafe { Vec::from_raw_parts(self.data, length, length) };
}
DataManagedType::External => {
let mut finalizer: Box<dyn FnOnce(*mut $rust_type, usize)> = Box::new(|_a, _b| {});
std::mem::swap(&mut finalizer, &mut self.finalizer_notify);
let finalizer = unsafe { Box::from_raw(self.finalizer_notify) };
(finalizer)(self.data, self.length);
}
_ => {}
Expand All @@ -133,7 +132,7 @@ macro_rules! impl_typed_array {
byte_offset: 0,
raw: None,
drop_in_vm: Arc::new(AtomicBool::new(false)),
finalizer_notify: Box::new(Self::noop_finalize),
finalizer_notify: Box::into_raw(Box::new(Self::noop_finalize)),
};
mem::forget(data);
ret
Expand All @@ -148,7 +147,7 @@ macro_rules! impl_typed_array {
data: data_copied.as_mut_ptr(),
length: data.as_ref().len(),
data_managed_type: DataManagedType::Owned,
finalizer_notify: Box::new(Self::noop_finalize),
finalizer_notify: Box::into_raw(Box::new(Self::noop_finalize)),
raw: None,
drop_in_vm: Arc::new(AtomicBool::new(false)),
byte_offset: 0,
Expand All @@ -168,7 +167,7 @@ macro_rules! impl_typed_array {
data,
length,
data_managed_type: DataManagedType::External,
finalizer_notify: Box::new(notify),
finalizer_notify: Box::into_raw(Box::new(notify)),
raw: None,
drop_in_vm: Arc::new(AtomicBool::new(false)),
byte_offset: 0,
Expand All @@ -183,7 +182,7 @@ macro_rules! impl_typed_array {
data: self.data,
length: self.length,
data_managed_type: self.data_managed_type,
finalizer_notify: Box::new(Self::noop_finalize),
finalizer_notify: self.finalizer_notify,
raw: self.raw,
drop_in_vm: self.drop_in_vm.clone(),
byte_offset: self.byte_offset,
Expand Down Expand Up @@ -286,7 +285,7 @@ macro_rules! impl_typed_array {
raw: Some((ref_, env)),
drop_in_vm: Arc::new(AtomicBool::new(true)),
data_managed_type: DataManagedType::Vm,
finalizer_notify: Box::new(Self::noop_finalize),
finalizer_notify: Box::into_raw(Box::new(Self::noop_finalize)),
})
}
}
Expand Down Expand Up @@ -363,7 +362,7 @@ macro_rules! impl_typed_array {
data: val.data,
length: val.length,
data_managed_type: val.data_managed_type,
finalizer_notify: Box::new($name::noop_finalize),
finalizer_notify: Box::into_raw(Box::new($name::noop_finalize)),
raw: None,
byte_offset: val.byte_offset,
};
Expand All @@ -379,10 +378,9 @@ unsafe extern "C" fn finalizer<Data, T: Finalizer<RustType = Data>>(
finalize_data: *mut c_void,
finalize_hint: *mut c_void,
) {
let mut data = unsafe { *Box::from_raw(finalize_hint as *mut T) };
let data = unsafe { *Box::from_raw(finalize_hint as *mut T) };
let data_managed_type = *data.data_managed_type();
let length = *data.len();
let finalizer_notify = data.finalizer();
match data_managed_type {
DataManagedType::Vm => {
// do nothing
Expand All @@ -395,6 +393,7 @@ unsafe extern "C" fn finalizer<Data, T: Finalizer<RustType = Data>>(
}
DataManagedType::External => {
if data.ref_count() == 1 {
let finalizer_notify = unsafe { Box::from_raw(data.finalizer_notify()) };
(finalizer_notify)(finalize_data as *mut Data, length);
}
}
Expand Down

1 comment on commit 9c3e7ca

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 9c3e7ca Previous: 5ab4b81 Ratio
noop#napi-rs 66229366 ops/sec (±0.22%) 57577113 ops/sec (±0.61%) 0.87
noop#JavaScript 590583639 ops/sec (±0.77%) 635325208 ops/sec (±0.5%) 1.08
Plus number#napi-rs 20255010 ops/sec (±0.19%) 16432337 ops/sec (±0.95%) 0.81
Plus number#JavaScript 591810042 ops/sec (±0.13%) 615111906 ops/sec (±0.43%) 1.04
Create buffer#napi-rs 429063 ops/sec (±6.9%) 359275 ops/sec (±9.78%) 0.84
Create buffer#JavaScript 2005006 ops/sec (±5.22%) 1905294 ops/sec (±6.38%) 0.95
createArray#createArrayJson 45335 ops/sec (±0.12%) 33424 ops/sec (±0.33%) 0.74
createArray#create array for loop 7836 ops/sec (±0.1%) 6234 ops/sec (±0.29%) 0.80
createArray#create array with serde trait 7877 ops/sec (±0.1%) 6451 ops/sec (±0.6%) 0.82
getArrayFromJs#get array from json string 18699 ops/sec (±0.28%) 14901 ops/sec (±0.65%) 0.80
getArrayFromJs#get array from serde 9740 ops/sec (±0.04%) 8655 ops/sec (±0.52%) 0.89
getArrayFromJs#get array with for loop 12797 ops/sec (±0.05%) 10824 ops/sec (±0.69%) 0.85
Get Set property#Get Set from native#u32 395173 ops/sec (±5.25%) 389137 ops/sec (±6.13%) 0.98
Get Set property#Get Set from JavaScript#u32 331658 ops/sec (±5.19%) 317835 ops/sec (±6.11%) 0.96
Get Set property#Get Set from native#string 359029 ops/sec (±4.86%) 331477 ops/sec (±5.89%) 0.92
Get Set property#Get Set from JavaScript#string 321967 ops/sec (±5.13%) 305112 ops/sec (±6.05%) 0.95
Async task#spawn task 35603 ops/sec (±1.04%) 31717 ops/sec (±1.44%) 0.89
Async task#ThreadSafeFunction 2804 ops/sec (±3.41%) 1669 ops/sec (±13.21%) 0.60
Async task#Tokio future to Promise 32174 ops/sec (±0.54%) 28290 ops/sec (±2.03%) 0.88
Query#query * 100 2047 ops/sec (±2.13%) 1779 ops/sec (±3.75%) 0.87
Query#query * 1 31905 ops/sec (±1.11%) 28044 ops/sec (±2.05%) 0.88

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.