From 8f40ebd206ff27d8ec7aa04c58dd2d2bb28d8288 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 5 Oct 2025 19:35:20 +0800 Subject: [PATCH] fix(napi): cleanup memory issues (#2949) --- .github/workflows/asan.yml | 91 +++++++++++++++++-- .github/workflows/test-release.yaml | 8 +- cli/package.json | 3 +- .../bindgen_runtime/js_values/arraybuffer.rs | 43 +++++++-- .../src/bindgen_runtime/js_values/buffer.rs | 16 ++-- .../src/bindgen_runtime/js_values/object.rs | 8 +- crates/napi/src/bindgen_runtime/mod.rs | 7 +- crates/napi/src/env.rs | 17 +++- crates/napi/src/error.rs | 9 +- crates/napi/src/js_values/buffer.rs | 41 +++++++-- crates/napi/src/js_values/mod.rs | 26 +++--- crates/napi/src/js_values/string/latin1.rs | 13 ++- crates/napi/src/js_values/string/mod.rs | 10 +- crates/napi/src/js_values/string/utf16.rs | 10 +- examples/napi/Cargo.toml | 6 ++ examples/napi/__tests__/worker-thread.spec.ts | 61 ++++++++----- examples/napi/package.json | 1 + examples/napi/src/lib.rs | 6 +- 18 files changed, 279 insertions(+), 97 deletions(-) diff --git a/.github/workflows/asan.yml b/.github/workflows/asan.yml index 366de57b..fd8dbfcb 100644 --- a/.github/workflows/asan.yml +++ b/.github/workflows/asan.yml @@ -15,8 +15,12 @@ on: jobs: test: - name: Address sanitizer - runs-on: ubuntu-24.04 + name: ASAN - ${{ matrix.os }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-24.04, windows-latest] steps: - uses: actions/checkout@v5 @@ -27,27 +31,44 @@ jobs: node-version: 22 cache: 'yarn' - - name: Install + # Linux-specific setup + - name: Install Rust (Linux) + if: matrix.os == 'ubuntu-24.04' uses: dtolnay/rust-toolchain@stable with: toolchain: nightly components: rust-src - - name: Install rust-src + - name: Install rust-src (Linux) + if: matrix.os == 'ubuntu-24.04' run: rustup component add rust-src --toolchain nightly-x86_64-unknown-linux-gnu + # Windows-specific setup + - name: Install Rust (Windows) + if: matrix.os == 'windows-latest' + uses: dtolnay/rust-toolchain@stable + with: + toolchain: nightly + targets: x86_64-pc-windows-msvc + components: rust-src + - name: Cache cargo uses: actions/cache@v4 with: path: | ~/.cargo/registry ~/.cargo/git - key: nightly-ubuntu-node@18-cargo-cache + target + key: ${{ matrix.os }}-asan-cargo-cache-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ matrix.os }}-asan-cargo-cache- - - name: 'Install dependencies' + - name: Install dependencies run: yarn install --immutable --mode=skip-build - - name: Unit tests with address sanitizer + # Linux build and test + - name: Build and test with ASAN (Linux) + if: matrix.os == 'ubuntu-24.04' run: | yarn workspace @examples/napi build -- -Z build-std yarn workspace @examples/compat-mode build -- -Z build-std @@ -61,7 +82,63 @@ jobs: DISABLE_V8_COMPILE_CACHE: 1 CARGO_PROFILE_DEV_OPT_LEVEL: 1 + # Windows build and test + - name: Build with ASAN (Windows) + if: matrix.os == 'windows-latest' + shell: pwsh + run: | + # Build the examples with ASAN enabled + # Note: -Z build-std flag cannot be passed through napi CLI on Windows + # The CARGO_UNSTABLE_BUILD_STD env var provides similar functionality + yarn workspace @examples/napi build --target x86_64-pc-windows-msvc + yarn workspace @examples/compat-mode build --target x86_64-pc-windows-msvc + env: + RUSTFLAGS: -Zsanitizer=address + RUSTDOCFLAGS: -Zsanitizer=address + RUST_BACKTRACE: 1 + CARGO_PROFILE_DEV_OPT_LEVEL: 1 + CARGO_UNSTABLE_BUILD_STD: std,panic_abort + + - name: Test with ASAN (Windows) + if: matrix.os == 'windows-latest' + shell: pwsh + run: | + # Set ASAN environment variables for Windows + $env:ASAN_OPTIONS = "windows_hook_rtl_allocators=true:detect_leaks=0:print_stats=1:check_initialization_order=true:strict_string_checks=true" + $env:NODE_OPTIONS = "--max-old-space-size=8192" + $env:DISABLE_V8_COMPILE_CACHE = "1" + $env:RUST_BACKTRACE = "full" + + # Find and set the path to the ASAN runtime DLL + $vsPath = & "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath + $asanDllPath = Get-ChildItem -Path "$vsPath\VC\Tools\MSVC" -Recurse -Filter "clang_rt.asan_dynamic-x86_64.dll" | Select-Object -First 1 + if ($asanDllPath) { + $env:PATH = "$($asanDllPath.DirectoryName);$env:PATH" + Write-Host "Found ASAN DLL at: $($asanDllPath.FullName)" + } + + yarn test + continue-on-error: true + + - name: Upload ASAN logs (Windows) + if: failure() && matrix.os == 'windows-latest' + uses: actions/upload-artifact@v4 + with: + name: windows-asan-logs + path: | + asan.log* + *.asan.log + - name: Clear the cargo caches run: | cargo install cargo-cache --no-default-features --features ci-autoclean cargo-cache + + asan-done: + name: Address sanitizer + runs-on: ubuntu-latest + needs: + - test + steps: + - run: exit 1 + if: ${{ always() && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) }} diff --git a/.github/workflows/test-release.yaml b/.github/workflows/test-release.yaml index 45ab0ab2..46d04da4 100644 --- a/.github/workflows/test-release.yaml +++ b/.github/workflows/test-release.yaml @@ -93,10 +93,10 @@ jobs: toolchain: stable - host: windows-latest target: x86_64-pc-windows-msvc - build: yarn build:test + build: yarn workspace @examples/napi build test: | yarn test:cli - yarn test + yarn workspace @examples/napi test yarn tsc -p examples/napi/tsconfig.json --noEmit --skipLibCheck yarn test:macro cargo test -p napi-cargo-test --features noop @@ -115,7 +115,7 @@ jobs: yarn workspace @examples/napi build --target i686-pc-windows-msvc --release yarn workspace @examples/compat-mode build --target i686-pc-windows-msvc --release test: | - $env:NODE_OPTIONS = "--max-old-space-size=3072" + export NODE_OPTIONS="--max-old-space-size=3072" yarn workspace @examples/napi test -s node ./node_modules/electron/install.js yarn test:electron @@ -200,6 +200,8 @@ jobs: - name: Unit tests if: matrix.settings.test run: ${{ matrix.settings.test }} + shell: bash + timeout-minutes: 10 env: NODE_OPTIONS: '--max-old-space-size=8192' DISABLE_V8_COMPILE_CACHE: 1 diff --git a/cli/package.json b/cli/package.json index 088895f1..7cf355dd 100644 --- a/cli/package.json +++ b/cli/package.json @@ -114,7 +114,8 @@ "extensions": { "ts": "module" }, - "timeout": "1m", + "timeout": "2m", + "workerThreads": false, "files": [ "**/__tests__/**/*.spec.ts", "e2e/**/*.spec.ts" diff --git a/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs index 78870c39..e7431d9c 100644 --- a/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs +++ b/crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs @@ -159,17 +159,22 @@ impl<'env> ArrayBuffer<'env> { } } let len = data.len(); + let cap = data.capacity(); + let finalize_hint = Box::into_raw(Box::new((len, cap))); let mut status = unsafe { sys::napi_create_external_arraybuffer( env.0, inner_ptr.cast(), data.len(), Some(finalize_slice::), - Box::into_raw(Box::new(len)).cast(), + finalize_hint.cast(), &mut buf, ) }; if status == napi_sys::Status::napi_no_external_buffers_allowed { + unsafe { + let _ = Box::from_raw(finalize_hint); + } let mut underlying_data = ptr::null_mut(); status = unsafe { sys::napi_create_arraybuffer(env.0, data.len(), &mut underlying_data, &mut buf) }; @@ -426,6 +431,7 @@ macro_rules! impl_typed_array { pub struct $name { data: *mut $rust_type, length: usize, + capacity: usize, #[allow(unused)] byte_offset: usize, raw: Option<(crate::sys::napi_ref, crate::sys::napi_env)>, @@ -498,8 +504,7 @@ macro_rules! impl_typed_array { return; } if !self.data.is_null() { - let length = self.length; - unsafe { Vec::from_raw_parts(self.data, length, length) }; + unsafe { Vec::from_raw_parts(self.data, self.length, self.capacity) }; } } } @@ -553,6 +558,7 @@ macro_rules! impl_typed_array { let ret = $name { data: data.as_mut_ptr(), length: data.len(), + capacity: data.capacity(), byte_offset: 0, raw: None, finalizer_notify: ptr::null_mut::(), @@ -569,6 +575,7 @@ macro_rules! impl_typed_array { let ret = $name { data: data_copied.as_mut_ptr(), length: data.as_ref().len(), + capacity: data_copied.capacity(), finalizer_notify: ptr::null_mut::(), raw: None, byte_offset: 0, @@ -587,6 +594,7 @@ macro_rules! impl_typed_array { $name { data, length, + capacity: length, finalizer_notify: Box::into_raw(Box::new(notify)), raw: None, byte_offset: 0, @@ -694,6 +702,7 @@ macro_rules! impl_typed_array { Ok($name { data: data.cast(), length, + capacity: length, byte_offset, raw: Some((ref_, env)), finalizer_notify: ptr::null_mut::(), @@ -808,6 +817,7 @@ macro_rules! impl_typed_array { let val_copy = $name { data: val.data, length: val.length, + capacity: val.capacity, byte_offset: val.byte_offset, raw: None, finalizer_notify: val.finalizer_notify, @@ -868,6 +878,7 @@ macro_rules! impl_typed_array { val.finalizer_notify = ptr::null_mut::(); val.data = ptr::null_mut(); val.length = 0; + val.capacity = 0; copied_val.raw = Some((ref_, ptr::null_mut())); } Ok(napi_val) @@ -908,17 +919,22 @@ macro_rules! impl_from_slice { } } let len = data.len(); + let cap = data.capacity(); + let finalize_hint = Box::into_raw(Box::new((len, cap))); let mut status = unsafe { sys::napi_create_external_arraybuffer( env.0, inner_ptr.cast(), data.len(), Some(finalize_slice::<$rust_type>), - Box::into_raw(Box::new(len)).cast(), + finalize_hint.cast(), &mut buf, ) }; if status == napi_sys::Status::napi_no_external_buffers_allowed { + unsafe { + let _ = Box::from_raw(finalize_hint); + } let mut underlying_data = ptr::null_mut(); status = unsafe { sys::napi_create_arraybuffer( @@ -1423,8 +1439,12 @@ unsafe extern "C" fn finalize_slice( finalize_data: *mut c_void, finalize_hint: *mut c_void, ) { - let length = unsafe { *Box::from_raw(finalize_hint as *mut usize) }; - unsafe { Vec::from_raw_parts(finalize_data as *mut Data, length, length) }; + let (length, capacity): (usize, usize) = + *unsafe { Box::from_raw(finalize_hint as *mut (usize, usize)) }; + if finalize_data.is_null() { + return; + } + unsafe { Vec::from_raw_parts(finalize_data as *mut Data, length, capacity) }; } impl_typed_array!(Int8Array, i8, TypedArrayType::Int8); @@ -1477,11 +1497,13 @@ impl Uint8Array { /// Create a new JavaScript `Uint8Array` from a Rust `String` without copying the underlying data. pub fn from_string(mut s: String) -> Self { let len = s.len(); + let cap = s.capacity(); let ret = Self { data: s.as_mut_ptr(), length: len, + capacity: cap, finalizer_notify: Box::into_raw(Box::new(move |data, _| { - drop(unsafe { String::from_raw_parts(data, len, len) }); + drop(unsafe { String::from_raw_parts(data, len, cap) }); })), byte_offset: 0, raw: None, @@ -1614,17 +1636,22 @@ impl<'env> Uint8ClampedSlice<'env> { } } let len = data.len(); + let cap = data.capacity(); + let finalize_hint = Box::into_raw(Box::new((len, cap))); let mut status = unsafe { sys::napi_create_external_arraybuffer( env.0, inner_ptr.cast(), data.len(), Some(finalize_slice::), - Box::into_raw(Box::new(len)).cast(), + finalize_hint.cast(), &mut buf, ) }; if status == napi_sys::Status::napi_no_external_buffers_allowed { + unsafe { + let _ = Box::from_raw(finalize_hint); + } let mut underlying_data = ptr::null_mut(); status = unsafe { sys::napi_create_arraybuffer(env.0, len, &mut underlying_data, &mut buf) }; let underlying_slice: &mut [u8] = diff --git a/crates/napi/src/bindgen_runtime/js_values/buffer.rs b/crates/napi/src/bindgen_runtime/js_values/buffer.rs index bf737d19..2aeb91c9 100644 --- a/crates/napi/src/bindgen_runtime/js_values/buffer.rs +++ b/crates/napi/src/bindgen_runtime/js_values/buffer.rs @@ -50,18 +50,23 @@ impl<'env> BufferSlice<'env> { } } let len = data.len(); + let cap = data.capacity(); + let finalize_hint = Box::into_raw(Box::new((len, cap))); let mut status = unsafe { sys::napi_create_external_buffer( env.0, len, inner_ptr.cast(), Some(drop_buffer_slice), - Box::into_raw(Box::new(len)).cast(), + finalize_hint.cast(), &mut buf, ) }; - status = if status == sys::Status::napi_no_external_buffers_allowed { + if status == sys::Status::napi_no_external_buffers_allowed { unsafe { + let _ = Box::from_raw(finalize_hint); + } + status = unsafe { sys::napi_create_buffer_copy( env.0, len, @@ -69,11 +74,10 @@ impl<'env> BufferSlice<'env> { ptr::null_mut(), &mut buf, ) - } + }; } else { - status - }; - mem::forget(data); + mem::forget(data); + } check_status!(status, "Failed to create buffer slice from data")?; Ok(Self { diff --git a/crates/napi/src/bindgen_runtime/js_values/object.rs b/crates/napi/src/bindgen_runtime/js_values/object.rs index 2c82fae1..b7db8392 100644 --- a/crates/napi/src/bindgen_runtime/js_values/object.rs +++ b/crates/napi/src/bindgen_runtime/js_values/object.rs @@ -474,7 +474,7 @@ pub trait JsObjectValue<'env>: JsValue<'env> { .filter(|data| !data.is_null()) .collect::>(); if !closures.is_empty() { - let len = Box::into_raw(Box::new(closures.len())); + let finalize_hint = Box::into_raw(Box::new((closures.len(), closures.capacity()))); check_status!( unsafe { sys::napi_add_finalizer( @@ -482,7 +482,7 @@ pub trait JsObjectValue<'env>: JsValue<'env> { self.value().value, closures.as_mut_ptr().cast(), Some(finalize_closures), - len.cast(), + finalize_hint.cast(), ptr::null_mut(), ) }, @@ -1058,9 +1058,9 @@ pub(crate) unsafe extern "C" fn finalize_closures( data: *mut c_void, len: *mut c_void, ) { - let length: usize = *unsafe { Box::from_raw(len.cast()) }; + let (length, capacity): (usize, usize) = *unsafe { Box::from_raw(len.cast()) }; let closures: Vec<*mut PropertyClosures> = - unsafe { Vec::from_raw_parts(data.cast(), length, length) }; + unsafe { Vec::from_raw_parts(data.cast(), length, capacity) }; for closure_ptr in closures.into_iter() { if !closure_ptr.is_null() { let closures = unsafe { Box::from_raw(closure_ptr) }; diff --git a/crates/napi/src/bindgen_runtime/mod.rs b/crates/napi/src/bindgen_runtime/mod.rs index 2e8dea75..6f60bbd5 100644 --- a/crates/napi/src/bindgen_runtime/mod.rs +++ b/crates/napi/src/bindgen_runtime/mod.rs @@ -98,7 +98,7 @@ pub unsafe extern "C" fn drop_buffer_slice( finalize_data: *mut c_void, finalize_hint: *mut c_void, ) { - let len = *unsafe { Box::from_raw(finalize_hint.cast()) }; + let (len, cap): (usize, usize) = *unsafe { Box::from_raw(finalize_hint.cast()) }; #[cfg(all(debug_assertions, not(windows)))] { js_values::BUFFER_DATA.with(|buffer_data| { @@ -106,7 +106,10 @@ pub unsafe extern "C" fn drop_buffer_slice( buffer.remove(&(finalize_data as *mut u8)); }); } + if finalize_data.is_null() { + return; + } unsafe { - drop(Vec::from_raw_parts(finalize_data, len, len)); + drop(Vec::from_raw_parts(finalize_data, len, cap)); } } diff --git a/crates/napi/src/env.rs b/crates/napi/src/env.rs index 5c1ba0f1..b9524ced 100644 --- a/crates/napi/src/env.rs +++ b/crates/napi/src/env.rs @@ -289,7 +289,11 @@ impl Env { value: raw_value, value_type: ValueType::Object, }), - mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(data_ptr as *mut _, length, length) }), + mem::ManuallyDrop::new(if length == 0 { + Vec::new() + } else { + unsafe { Vec::from_raw_parts(data_ptr as *mut _, length, length) } + }), )) } @@ -302,7 +306,6 @@ impl Env { let length = data.len(); let mut raw_value = ptr::null_mut(); let data_ptr = data.as_mut_ptr(); - let hint_ptr = Box::into_raw(Box::new((length, data.capacity()))); check_status!(unsafe { if length == 0 { // Rust uses 0x1 as the data pointer for empty buffers, @@ -310,6 +313,7 @@ impl Env { // the same data pointer if it's 0x0. sys::napi_create_buffer(self.0, length, ptr::null_mut(), &mut raw_value) } else { + let hint_ptr = Box::into_raw(Box::new((length, data.capacity()))); let status = sys::napi_create_external_buffer( self.0, length, @@ -463,7 +467,11 @@ impl Env { value: raw_value, value_type: ValueType::Object, }), - mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(copy_data as *mut u8, length, length) }), + mem::ManuallyDrop::new(if length == 0 { + Vec::new() + } else { + unsafe { Vec::from_raw_parts(copy_data as *mut u8, length, length) } + }), )) } @@ -1479,6 +1487,9 @@ unsafe extern "C" fn drop_buffer( ) { let length_ptr = hint as *mut (usize, usize); let (length, cap) = unsafe { *Box::from_raw(length_ptr) }; + if length == 0 || finalize_data.is_null() { + return; + } mem::drop(unsafe { Vec::from_raw_parts(finalize_data as *mut u8, length, cap) }); } diff --git a/crates/napi/src/error.rs b/crates/napi/src/error.rs index a6ab9bdc..f2282c27 100644 --- a/crates/napi/src/error.rs +++ b/crates/napi/src/error.rs @@ -1,10 +1,10 @@ use std::convert::{From, TryFrom}; use std::error; -use std::ffi::CString; +use std::ffi::CStr; use std::fmt; #[cfg(feature = "serde-json")] use std::fmt::Display; -use std::os::raw::{c_char, c_void}; +use std::os::raw::c_void; use std::ptr; #[cfg(feature = "serde-json")] @@ -276,9 +276,10 @@ impl TryFrom for ExtendedErrorInfo { fn try_from(value: sys::napi_extended_error_info) -> Result { Ok(Self { message: unsafe { - CString::from_raw(value.error_message as *mut c_char) - .into_string() + CStr::from_ptr(value.error_message.cast()) + .to_str() .map_err(|e| Error::new(Status::GenericFailure, format!("{e}")))? + .to_owned() }, engine_error_code: value.engine_error_code, engine_reserved: value.engine_reserved, diff --git a/crates/napi/src/js_values/buffer.rs b/crates/napi/src/js_values/buffer.rs index 9bb21e44..67e0eaae 100644 --- a/crates/napi/src/js_values/buffer.rs +++ b/crates/napi/src/js_values/buffer.rs @@ -53,7 +53,9 @@ impl JsValue<'_> for JsBuffer { #[deprecated(since = "3.0.0", note = "Please use Buffer or &[u8] instead")] pub struct JsBufferValue { pub(crate) value: JsBuffer, - data: mem::ManuallyDrop>, + len: usize, + data_ptr: *mut u8, + owned: Option>>, } impl JsBuffer { @@ -64,8 +66,10 @@ impl JsBuffer { sys::napi_get_buffer_info(self.0.env, self.0.value, &mut data, &mut len) })?; Ok(JsBufferValue { - data: mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(data as *mut _, len, len) }), value: self, + len, + data_ptr: data as *mut u8, + owned: None, }) } @@ -76,7 +80,18 @@ impl JsBuffer { impl JsBufferValue { pub fn new(value: JsBuffer, data: mem::ManuallyDrop>) -> Self { - JsBufferValue { value, data } + let len = data.len(); + let data_ptr = if len == 0 { + std::ptr::null_mut() + } else { + data.as_ptr() as *mut u8 + }; + JsBufferValue { + value, + len, + data_ptr, + owned: Some(data), + } } pub fn into_raw(self) -> JsBuffer { @@ -90,13 +105,25 @@ impl JsBufferValue { impl AsRef<[u8]> for JsBufferValue { fn as_ref(&self) -> &[u8] { - self.data.as_slice() + if let Some(ref data) = self.owned { + data.as_slice() + } else if self.len == 0 { + &[] + } else { + unsafe { std::slice::from_raw_parts(self.data_ptr as *const u8, self.len) } + } } } impl AsMut<[u8]> for JsBufferValue { fn as_mut(&mut self) -> &mut [u8] { - self.data.as_mut_slice() + if let Some(ref mut data) = self.owned { + data.as_mut_slice() + } else if self.len == 0 { + &mut [] + } else { + unsafe { std::slice::from_raw_parts_mut(self.data_ptr, self.len) } + } } } @@ -104,12 +131,12 @@ impl Deref for JsBufferValue { type Target = [u8]; fn deref(&self) -> &Self::Target { - self.data.as_slice() + self.as_ref() } } impl DerefMut for JsBufferValue { fn deref_mut(&mut self) -> &mut Self::Target { - self.data.as_mut_slice() + self.as_mut() } } diff --git a/crates/napi/src/js_values/mod.rs b/crates/napi/src/js_values/mod.rs index a4f7de99..1f4eedd8 100644 --- a/crates/napi/src/js_values/mod.rs +++ b/crates/napi/src/js_values/mod.rs @@ -618,18 +618,20 @@ macro_rules! impl_object_methods { .map(|p| p.data) .filter(|data| !data.is_null()) .collect::>(); - let len = Box::into_raw(Box::new(closures.len())); - check_status!(unsafe { - sys::napi_add_finalizer( - self.0.env, - self.0.value, - closures.as_mut_ptr().cast(), - Some(finalize_closures), - len.cast(), - ptr::null_mut(), - ) - })?; - std::mem::forget(closures); + if !closures.is_empty() { + let finalize_hint = Box::into_raw(Box::new((closures.len(), closures.capacity()))); + check_status!(unsafe { + sys::napi_add_finalizer( + self.0.env, + self.0.value, + closures.as_mut_ptr().cast(), + Some(finalize_closures), + finalize_hint.cast(), + ptr::null_mut(), + ) + })?; + std::mem::forget(closures); + } } check_status!(unsafe { sys::napi_define_properties( diff --git a/crates/napi/src/js_values/string/latin1.rs b/crates/napi/src/js_values/string/latin1.rs index 6901f018..65f21577 100644 --- a/crates/napi/src/js_values/string/latin1.rs +++ b/crates/napi/src/js_values/string/latin1.rs @@ -44,8 +44,7 @@ impl<'env> JsStringLatin1<'env> { /// The `copied` parameter serves as feedback to understand whether the external string /// optimization was successful or if V8 fell back to traditional string creation. pub fn from_data(env: &'env Env, data: Vec) -> Result> { - use std::mem; - use std::ptr; + use std::{mem, ptr}; use crate::{check_status, Error, Status, Value, ValueType}; @@ -60,7 +59,8 @@ impl<'env> JsStringLatin1<'env> { let mut copied = false; let data_ptr = data.as_ptr(); let len = data.len(); - let finalize_hint = Box::into_raw(Box::new(len)); + let cap = data.capacity(); + let finalize_hint = Box::into_raw(Box::new((len, cap))); check_status!( unsafe { @@ -291,8 +291,11 @@ extern "C" fn drop_latin1_string( finalize_data: *mut c_void, finalize_hint: *mut c_void, ) { - let size: usize = unsafe { *Box::from_raw(finalize_hint.cast()) }; - let data: Vec = unsafe { Vec::from_raw_parts(finalize_data.cast(), size, size) }; + let (size, capacity): (usize, usize) = unsafe { *Box::from_raw(finalize_hint.cast()) }; + if size == 0 || finalize_data.is_null() { + return; + } + let data: Vec = unsafe { Vec::from_raw_parts(finalize_data.cast(), size, capacity) }; drop(data); } diff --git a/crates/napi/src/js_values/string/mod.rs b/crates/napi/src/js_values/string/mod.rs index 7dd5c626..48ac4900 100644 --- a/crates/napi/src/js_values/string/mod.rs +++ b/crates/napi/src/js_values/string/mod.rs @@ -97,7 +97,7 @@ impl<'env> JsString<'env> { pub fn into_utf8(self) -> Result> { let mut written_char_count = 0; let len = self.utf8_len()? + 1; - let mut result = Vec::with_capacity(len); + let mut result = vec![0; len]; let buf_ptr = result.as_mut_ptr(); check_status!(unsafe { sys::napi_get_value_string_utf8( @@ -113,7 +113,7 @@ impl<'env> JsString<'env> { Ok(JsStringUtf8 { inner: self, - buf: unsafe { Vec::from_raw_parts(buf_ptr.cast(), written_char_count, written_char_count) }, + buf: unsafe { Vec::from_raw_parts(buf_ptr.cast(), written_char_count, len) }, }) } @@ -142,7 +142,7 @@ impl<'env> JsString<'env> { pub fn into_latin1(self) -> Result> { let mut written_char_count = 0usize; let len = self.latin1_len()? + 1; - let mut result = Vec::with_capacity(len); + let mut result = vec![0; len]; let buf_ptr = result.as_mut_ptr(); check_status!(unsafe { sys::napi_get_value_string_latin1( @@ -159,9 +159,7 @@ impl<'env> JsString<'env> { Ok(JsStringLatin1 { inner: self, buf: unsafe { std::slice::from_raw_parts(buf_ptr.cast(), written_char_count) }, - _inner_buf: unsafe { - Vec::from_raw_parts(buf_ptr.cast(), written_char_count, written_char_count) - }, + _inner_buf: unsafe { Vec::from_raw_parts(buf_ptr.cast(), written_char_count, len) }, }) } } diff --git a/crates/napi/src/js_values/string/utf16.rs b/crates/napi/src/js_values/string/utf16.rs index 8b991aac..0f82d585 100644 --- a/crates/napi/src/js_values/string/utf16.rs +++ b/crates/napi/src/js_values/string/utf16.rs @@ -62,7 +62,8 @@ impl<'env> JsStringUtf16<'env> { let mut copied = false; let data_ptr = data.as_ptr(); let len = data.len(); - let finalize_hint = Box::into_raw(Box::new(len)); + let cap = data.capacity(); + let finalize_hint = Box::into_raw(Box::new((len, cap))); check_status!( unsafe { @@ -312,8 +313,11 @@ extern "C" fn drop_utf16_string( finalize_data: *mut c_void, finalize_hint: *mut c_void, ) { - let size: usize = unsafe { *Box::from_raw(finalize_hint.cast()) }; - let data: Vec = unsafe { Vec::from_raw_parts(finalize_data.cast(), size, size) }; + let (size, capacity): (usize, usize) = unsafe { *Box::from_raw(finalize_hint.cast()) }; + if size == 0 || finalize_data.is_null() { + return; + } + let data: Vec = unsafe { Vec::from_raw_parts(finalize_data.cast(), size, capacity) }; drop(data); } diff --git a/examples/napi/Cargo.toml b/examples/napi/Cargo.toml index cf023f66..7e4a876c 100644 --- a/examples/napi/Cargo.toml +++ b/examples/napi/Cargo.toml @@ -10,6 +10,7 @@ crate-type = ["cdylib"] [features] snmalloc = ["snmalloc-rs"] +mimalloc = ["mimalloc-safe"] dyn-symbols = ["napi/dyn-symbols"] error_try_builds = [] noop = ["napi/noop"] @@ -60,6 +61,11 @@ version = "0.3" features = ["build_cc", "local_dynamic_tls"] optional = true +[dependencies.mimalloc-safe] +version = "0.1" +optional = true +features = ["skip_collect_on_exit"] + [build-dependencies] napi-build = { path = "../../crates/build" } diff --git a/examples/napi/__tests__/worker-thread.spec.ts b/examples/napi/__tests__/worker-thread.spec.ts index 856e2c99..f2943a31 100644 --- a/examples/napi/__tests__/worker-thread.spec.ts +++ b/examples/napi/__tests__/worker-thread.spec.ts @@ -1,6 +1,7 @@ import { join } from 'node:path' import { fileURLToPath } from 'node:url' import { Worker } from 'node:worker_threads' +import { setTimeout } from 'node:timers/promises' import test from 'ava' @@ -21,10 +22,14 @@ const concurrency = : 1 test.after(() => { - shutdownRuntime() + if (process.platform !== 'win32') { + shutdownRuntime() + } }) -test('should be able to require in worker thread', async (t) => { +const condTest = process.platform !== 'win32' ? test : test.skip + +condTest('should be able to require in worker thread', async (t) => { await Promise.all( Array.from({ length: concurrency }).map(() => { const w = new Worker(join(__dirname, 'worker.js'), { @@ -40,6 +45,7 @@ test('should be able to require in worker thread', async (t) => { reject(err) }) }) + .then(() => setTimeout(100)) .then(() => w.terminate()) .then(() => { t.pass() @@ -48,7 +54,7 @@ test('should be able to require in worker thread', async (t) => { ) }) -test('custom GC works on worker_threads', async (t) => { +condTest('custom GC works on worker_threads', async (t) => { await Promise.all( Array.from({ length: concurrency }).map(() => Promise.all([ @@ -83,7 +89,8 @@ test('custom GC works on worker_threads', async (t) => { w.on('error', (err) => { reject(err) }) - }).then((w) => { + }).then(async (w) => { + await setTimeout(100) return w.terminate() }), ]), @@ -91,26 +98,30 @@ test('custom GC works on worker_threads', async (t) => { ) }) -test('should be able to new Class in worker thread concurrently', async (t) => { - await Promise.all( - Array.from({ length: concurrency }).map(() => { - const w = new Worker(join(__dirname, 'worker.js'), { - env: process.env, - }) - return new Promise((resolve, reject) => { - w.postMessage({ type: 'constructor' }) - w.on('message', (msg) => { - t.is(msg, 'Ellie') - resolve() +condTest( + 'should be able to new Class in worker thread concurrently', + async (t) => { + await Promise.all( + Array.from({ length: concurrency }).map(() => { + const w = new Worker(join(__dirname, 'worker.js'), { + env: process.env, }) - w.on('error', (err) => { - reject(err) + return new Promise((resolve, reject) => { + w.postMessage({ type: 'constructor' }) + w.on('message', (msg) => { + t.is(msg, 'Ellie') + resolve() + }) + w.on('error', (err) => { + reject(err) + }) }) - }) - .then(() => w.terminate()) - .then(() => { - t.pass() - }) - }), - ) -}) + .then(() => setTimeout(100)) + .then(() => w.terminate()) + .then(() => { + t.pass() + }) + }), + ) + }, +) diff --git a/examples/napi/package.json b/examples/napi/package.json index 55b66b30..d63ed02c 100644 --- a/examples/napi/package.json +++ b/examples/napi/package.json @@ -60,6 +60,7 @@ "files": [ "__tests__/**/*.spec.{ts,cts,js,cjs,mjs}" ], + "workerThreads": false, "cache": false, "timeout": "10m" }, diff --git a/examples/napi/src/lib.rs b/examples/napi/src/lib.rs index 3419ae76..27eed230 100644 --- a/examples/napi/src/lib.rs +++ b/examples/napi/src/lib.rs @@ -8,7 +8,7 @@ #[cfg(not(target_family = "wasm"))] use napi::bindgen_prelude::create_custom_tokio_runtime; use napi::bindgen_prelude::{JsObjectValue, Object, Result, Symbol}; -// pub use napi_shared::*; +pub use napi_shared::*; #[macro_use] extern crate napi_derive; @@ -19,6 +19,10 @@ extern crate serde_derive; #[global_allocator] static ALLOC: snmalloc_rs::SnMalloc = snmalloc_rs::SnMalloc; +#[cfg(feature = "mimalloc")] +#[global_allocator] +static ALLOC: mimalloc_safe::MiMalloc = mimalloc_safe::MiMalloc; + #[cfg(not(target_family = "wasm"))] #[napi_derive::module_init] fn init() {