fix(napi): thread safe issue while creating class instance (#1561)

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::ToNapiValue, check_status, Env, Error, Result, Status};
-
-use crate::{
-  bindgen_runtime::{PersistedPerInstanceHashMap, 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<PersistedPerInstanceHashMap<*mut c_void, RefInformation>> =
+thread_local! {
-  Lazy::new(Default::default);
+  pub(crate) static REFERENCE_MAP: RefCell<HashMap<*mut c_void, RefInformation>> = RefCell::new(HashMap::default());
+}
 
 /// ### Experimental feature
 ///
@@ -61,8 +58,8 @@ impl<T: 'static> Reference<T> {
   #[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| {
+    REFERENCE_MAP.with(|map| {
-      if let Some((_, previous_ref, previous_rc)) = map.insert(t, value) {
+      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<T: 'static> Reference<T> {
   #[doc(hidden)]
   pub unsafe fn from_value_ptr(t: *mut c_void, env: crate::sys::napi_env) -> Result<Self> {
     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) },

crates/napi/src/bindgen_runtime/mod.rs

@@ -40,7 +40,7 @@ pub unsafe extern "C" fn raw_finalize_unchecked<T: ObjectFinalize>(
     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) };
 

examples/napi/__tests__/worker-thread.spec.ts

@@ -5,10 +5,7 @@ import test from 'ava'
 
 import { Animal, Kind, DEFAULT_COST } from '../index'
 
-const t =
+test('should be able to require in worker thread', async (t) => {
-  process.arch === 'arm64' && process.platform === 'linux' ? test.skip : test
-
-t('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<void>((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()
+        })
+    }),
+  )
+})

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}`)

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",