From d14fdca2424f210f8faf061d5fea795b52835204 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sat, 15 Apr 2023 15:37:01 +0800 Subject: [PATCH] fix(napi): thread safe issue while creating class instance (#1561) --- .../bindgen_runtime/js_values/value_ref.rs | 21 ++++++-------- crates/napi/src/bindgen_runtime/mod.rs | 2 +- examples/napi/__tests__/worker-thread.spec.ts | 29 +++++++++++++++---- examples/napi/__tests__/worker.js | 7 +++++ package.json | 2 +- 5 files changed, 42 insertions(+), 19 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 0c911680..f2e6c5a6 100644 --- a/crates/napi/src/bindgen_runtime/js_values/value_ref.rs +++ b/crates/napi/src/bindgen_runtime/js_values/value_ref.rs @@ -1,14 +1,10 @@ -use std::cell::Cell; +use std::cell::{Cell, RefCell}; +use std::collections::HashMap; use std::ffi::c_void; use std::ops::{Deref, DerefMut}; use std::rc::{Rc, Weak}; -use once_cell::sync::Lazy; - -use crate::{ - bindgen_runtime::{PersistedPerInstanceHashMap, ToNapiValue}, - check_status, Env, Error, Result, Status, -}; +use crate::{bindgen_runtime::ToNapiValue, check_status, Env, Error, Result, Status}; type RefInformation = ( *mut c_void, @@ -16,8 +12,9 @@ type RefInformation = ( *const Cell<*mut dyn FnOnce()>, ); -pub(crate) static REFERENCE_MAP: Lazy> = - Lazy::new(Default::default); +thread_local! { + pub(crate) static REFERENCE_MAP: RefCell> = RefCell::new(HashMap::default()); +} /// ### Experimental feature /// @@ -61,8 +58,8 @@ 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) { + REFERENCE_MAP.with(|map| { + if let Some((_, previous_ref, previous_rc)) = map.borrow_mut().insert(t, value) { unsafe { Rc::from_raw(previous_rc) }; unsafe { crate::sys::napi_delete_reference(env, previous_ref) }; } @@ -72,7 +69,7 @@ impl Reference { #[doc(hidden)] pub unsafe fn from_value_ptr(t: *mut c_void, env: crate::sys::napi_env) -> Result { if let Some((wrapped_value, napi_ref, finalize_callbacks_ptr)) = - REFERENCE_MAP.borrow_mut(|map| map.get(&t).cloned()) + REFERENCE_MAP.with(|map| map.borrow().get(&t).cloned()) { check_status!( unsafe { crate::sys::napi_reference_ref(env, napi_ref, &mut 0) }, diff --git a/crates/napi/src/bindgen_runtime/mod.rs b/crates/napi/src/bindgen_runtime/mod.rs index 2019727b..26fd11ef 100644 --- a/crates/napi/src/bindgen_runtime/mod.rs +++ b/crates/napi/src/bindgen_runtime/mod.rs @@ -40,7 +40,7 @@ pub unsafe extern "C" fn raw_finalize_unchecked( unsafe { e.throw_into(env) }; } if let Some((_, ref_val, finalize_callbacks_ptr)) = - REFERENCE_MAP.borrow_mut(|reference_map| reference_map.remove(&finalize_data)) + REFERENCE_MAP.with(|reference_map| reference_map.borrow_mut().remove(&finalize_data)) { let finalize_callbacks_rc = unsafe { Rc::from_raw(finalize_callbacks_ptr) }; diff --git a/examples/napi/__tests__/worker-thread.spec.ts b/examples/napi/__tests__/worker-thread.spec.ts index 11778421..1276eb5e 100644 --- a/examples/napi/__tests__/worker-thread.spec.ts +++ b/examples/napi/__tests__/worker-thread.spec.ts @@ -5,10 +5,7 @@ import test from 'ava' import { Animal, Kind, DEFAULT_COST } from '../index' -const t = - process.arch === 'arm64' && process.platform === 'linux' ? test.skip : test - -t('should be able to require in worker thread', async (t) => { +test('should be able to require in worker thread', async (t) => { await Promise.all( Array.from({ length: 100 }).map(() => { const w = new Worker(join(__dirname, 'worker.js')) @@ -30,7 +27,7 @@ t('should be able to require in worker thread', async (t) => { ) }) -t('custom GC works on worker_threads', async (t) => { +test('custom GC works on worker_threads', async (t) => { await Promise.all( Array.from({ length: 50 }).map(() => Promise.all([ @@ -68,3 +65,25 @@ t('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: 100 }).map(() => { + const w = new Worker(join(__dirname, 'worker.js')) + 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() + }) + }), + ) +}) diff --git a/examples/napi/__tests__/worker.js b/examples/napi/__tests__/worker.js index 392041a5..6021d7bd 100644 --- a/examples/napi/__tests__/worker.js +++ b/examples/napi/__tests__/worker.js @@ -35,6 +35,13 @@ parentPort.on('message', ({ type }) => { throw e }) + break + case 'constructor': + let ellie + for (let i = 0; i < 10000; i++) { + ellie = new native.Animal(native.Kind.Cat, 'Ellie') + } + parentPort.postMessage(ellie.name) break default: throw new TypeError(`Unknown message type: ${type}`) diff --git a/package.json b/package.json index f09d6f7e..d3c1df14 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "build": "lerna run build --scope '@napi-rs/*'", "build:bench": "yarn workspace bench build", "build:memory": "yarn workspace memory-testing build", - "build:test": "lerna run build --scope '@examples/*'", + "build:test": "lerna run build --stream --concurrency 1 --scope '@examples/*'", "format": "run-p format:prettier format:rs format:toml", "format:prettier": "prettier . -w", "format:rs": "cargo fmt",