fix(napi): race issues with Node.js worker_thread (#1081)

Co-authored-by: Jan Piotrowski <piotrowski+github@gmail.com>

.cirrus.yml

@@ -1,5 +1,5 @@
 freebsd_instance:
-  image: freebsd-12-2-release-amd64
+  image: freebsd-13-0-release-amd64
 
 task:
   name: FreeBSD
@@ -8,7 +8,8 @@ task:
     RUSTUP_HOME: /usr/local/rustup
     CARGO_HOME: /usr/local/cargo
     PATH: /usr/local/cargo/bin:$PATH
-    RUSTUP_IO_THREADS: 1
+    RUSTUP_IO_THREADS: '1'
+    CI: '1'
   setup_script:
     - pkg update
     - pkg install -y -f curl node16 libnghttp2
@@ -28,4 +29,4 @@ task:
     - yarn build
     - cargo test -p napi-sys --lib -- --nocapture
     - yarn build:test
-    - yarn test
+    - yarn test --verbose

.github/workflows/android-armv7.yml

@@ -17,7 +17,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           cache: 'yarn'

.github/workflows/android.yml

@@ -17,7 +17,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           cache: 'yarn'

.github/workflows/asan.yml

@@ -24,7 +24,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: ${{ matrix.node }}
           check-latest: true

.github/workflows/bench.yaml

@@ -19,7 +19,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/lint.yaml

@@ -15,7 +15,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/linux-aarch64-musl.yaml

@@ -20,7 +20,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/linux-aarch64.yaml

@@ -21,7 +21,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/linux-armv7.yaml

@@ -21,7 +21,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/linux-musl.yaml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/memory-test.yml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

.github/workflows/test.yaml

@@ -24,7 +24,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: ${{ matrix.node }}
           check-latest: true
@@ -46,16 +46,16 @@ jobs:
         uses: actions/cache@v2
         with:
           path: ~/.cargo/registry
-          key: stable-${{ matrix.os }}-node@${{ matrix.node }}-cargo-registry-trimmed
+          key: stable-${{ matrix.os }}-node@${{ matrix.node }}-cargo-registry
 
       - name: Cache cargo index
         uses: actions/cache@v2
         with:
           path: ~/.cargo/git
-          key: stable-${{ matrix.os }}gnu-node@${{ matrix.node }}-cargo-index-trimmed
+          key: stable-${{ matrix.os }}-node@${{ matrix.node }}-cargo-index
 
       - name: 'Install dependencies'
-        run: yarn install --immutable --network-timeout 300000
+        run: yarn install --mode=skip-build --immutable --network-timeout 300000
 
       - name: 'Build TypeScript'
         run: yarn build
@@ -69,7 +69,7 @@ jobs:
       - name: Unit tests
         run: |
           yarn build:test
-          yarn test
+          yarn test --verbose
         env:
           RUST_BACKTRACE: 1
 

.github/workflows/windows-arm.yml

@@ -18,14 +18,14 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true
           cache: 'yarn'
 
       - name: 'Install dependencies'
-        run: yarn install --immutable --network-timeout 300000
+        run: yarn install --mode=skip-build --immutable --network-timeout 300000
 
       - name: 'Build TypeScript'
         run: yarn build

.github/workflows/windows-i686.yml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true
@@ -26,7 +26,7 @@ jobs:
           cache: 'yarn'
 
       - name: 'Install dependencies'
-        run: yarn install --immutable --network-timeout 300000
+        run: yarn install --mode=skip-build --immutable --network-timeout 300000
 
       - name: 'Build TypeScript'
         run: yarn build
@@ -50,13 +50,13 @@ jobs:
         uses: actions/cache@v2
         with:
           path: ~/.cargo/registry
-          key: stable-windows-i686-node@lts-cargo-registry-trimmed-${{ hashFiles('**/Cargo.lock') }}
+          key: stable-windows-i686-node@lts-cargo-registry
 
       - name: Cache cargo index
         uses: actions/cache@v2
         with:
           path: ~/.cargo/git
-          key: stable-windows-i686-node@lts-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }}
+          key: stable-windows-i686-node@lts-cargo-index
 
       - name: Check build
         uses: actions-rs/cargo@v1
@@ -66,9 +66,9 @@ jobs:
 
       - name: Build Tests
         run: |
-          yarn --cwd ./examples/napi-compat-mode build-i686
+          yarn --cwd ./examples/napi-compat-mode build-i686 --release
-          yarn --cwd ./examples/napi build-i686
+          yarn --cwd ./examples/napi build-i686 --release
-          yarn test
+          yarn test --verbose
         env:
           RUST_BACKTRACE: 1
 

.github/workflows/zig.yaml

@@ -27,7 +27,7 @@ jobs:
       - run: docker run --rm --privileged multiarch/qemu-user-static:register --reset
       - uses: actions/checkout@v2
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           # Testing for compatibility with node v12.x
           node-version: 12
@@ -93,7 +93,7 @@ jobs:
         if: matrix.settings.host == 'ubuntu-latest'
       - uses: actions/checkout@v2
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

ava.config.mjs

@@ -1,3 +1,5 @@
+import { cpus } from 'os'
+
 const configuration = {
   extensions: ['ts', 'tsx'],
   files: ['cli/**/*.spec.ts', 'examples/**/__test__/**/*.spec.ts'],
@@ -6,7 +8,10 @@ const configuration = {
     TS_NODE_PROJECT: './examples/tsconfig.json',
   },
   timeout: '1m',
-  workerThreads: false,
+  workerThreads: true,
+  concurrency: process.env.CI ? 2 : cpus().length,
+  failFast: false,
+  verbose: !!process.env.CI,
 }
 
 if (parseInt(process.versions.napi, 10) < 4) {

cli/src/new/ci-template.ts

@@ -123,7 +123,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true
@@ -255,7 +255,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: \${{ matrix.node }}
           check-latest: true
@@ -297,7 +297,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: \${{ matrix.node }}
           check-latest: true
@@ -339,7 +339,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: \${{ matrix.node }}
           check-latest: true
@@ -512,7 +512,7 @@ jobs:
       - uses: actions/checkout@v2
 
       - name: Setup node
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v3
         with:
           node-version: 16
           check-latest: true

crates/backend/src/codegen/fn.rs

@@ -49,7 +49,7 @@ impl TryToTokens for NapiFn {
       quote! {
         // constructor function is called from class `factory`
         // so we should skip the original `constructor` logic
-        if napi::bindgen_prelude::___CALL_FROM_FACTORY.load(std::sync::atomic::Ordering::Relaxed) {
+        if napi::bindgen_prelude::___CALL_FROM_FACTORY.with(|inner| inner.load(std::sync::atomic::Ordering::Relaxed)) {
           return std::ptr::null_mut();
         }
         napi::bindgen_prelude::CallbackInfo::<#args_len>::new(env, cb, None).and_then(|mut cb| {
@@ -319,7 +319,7 @@ impl NapiFn {
             "Failed to register function `{}`",
             #name_str,
           )?;
-          napi::bindgen_prelude::register_js_function(#js_name, env, #cb_name, Some(#intermediate_ident));
+          napi::bindgen_prelude::register_js_function(#js_name, #cb_name, Some(#intermediate_ident));
           Ok(fn_ptr)
         }
 

crates/backend/src/codegen/struct.rs

@@ -232,7 +232,7 @@ impl NapiStruct {
             )?;
 
             let mut result = std::ptr::null_mut();
-            napi::bindgen_prelude::___CALL_FROM_FACTORY.store(true, std::sync::atomic::Ordering::Relaxed);
+            napi::bindgen_prelude::___CALL_FROM_FACTORY.with(|inner| inner.store(true, std::sync::atomic::Ordering::Relaxed));
             napi::check_status!(
               napi::sys::napi_new_instance(env, ctor, 0, std::ptr::null_mut(), &mut result),
               "Failed to construct class `{}`",
@@ -250,7 +250,7 @@ impl NapiStruct {
               "Failed to wrap native object of class `{}`",
               #js_name_str
             )?;
-            napi::bindgen_prelude::___CALL_FROM_FACTORY.store(false, std::sync::atomic::Ordering::Relaxed);
+            napi::bindgen_prelude::___CALL_FROM_FACTORY.with(|inner| inner.store(false, std::sync::atomic::Ordering::Relaxed));
             Ok(result)
           } else {
             Err(napi::bindgen_prelude::Error::new(

crates/napi/src/bindgen_runtime/callback_info.rs

@@ -4,11 +4,11 @@ use std::sync::atomic::{AtomicBool, Ordering};
 
 use crate::{bindgen_prelude::*, check_status, sys, Result};
 
-#[doc(hidden)]
+thread_local! {
-/// Determined is `constructor` called from Class `factory`
+  #[doc(hidden)]
-/// Ugly but works
+  /// Determined is `constructor` called from Class `factory`
-/// We can even be more ugly without `atomic`
+  pub static ___CALL_FROM_FACTORY: AtomicBool = AtomicBool::new(false);
-pub static ___CALL_FROM_FACTORY: AtomicBool = AtomicBool::new(false);
+}
 
 pub struct CallbackInfo<const N: usize> {
   env: sys::napi_env,
@@ -90,9 +90,9 @@ impl<const N: usize> CallbackInfo<N> {
     let this = self.this();
     let mut instance = ptr::null_mut();
     unsafe {
-      ___CALL_FROM_FACTORY.store(true, Ordering::Relaxed);
+      ___CALL_FROM_FACTORY.with(|inner| inner.store(true, Ordering::Relaxed));
       let status = sys::napi_new_instance(self.env, this, 0, ptr::null_mut(), &mut instance);
-      ___CALL_FROM_FACTORY.store(false, Ordering::Relaxed);
+      ___CALL_FROM_FACTORY.with(|inner| inner.store(false, Ordering::Relaxed));
       // Error thrown in `constructor`
       if status == sys::Status::napi_pending_exception {
         let mut exception = ptr::null_mut();

crates/napi/src/bindgen_runtime/js_values/number.rs

@@ -1,4 +1,5 @@
 use super::{check_status, sys, Result};
+use crate::type_of;
 
 macro_rules! impl_number_conversions {
 	( $( ($name:literal, $t:ty, $get:ident, $create:ident) ,)* ) => {
@@ -43,8 +44,9 @@ macro_rules! impl_number_conversions {
 
           check_status!(
             unsafe { sys::$get(env, napi_val, &mut ret) },
-						"Failed to convert napi value into rust type `{}`",
+            "Failed to convert napi value {:?} into rust type `{}`",
-            $name
+            type_of!(env, napi_val),
+            $name,
           )?;
 
             Ok(ret)

crates/napi/src/bindgen_runtime/module_register.rs

@@ -1,14 +1,18 @@
 use std::cell::RefCell;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
+#[cfg(all(feature = "tokio_rt", feature = "napi4"))]
 use std::ffi::c_void;
-use std::ffi::CStr;
+use std::ffi::{CStr, CString};
+use std::mem;
 use std::ptr;
-use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering};
+use std::sync::atomic::AtomicBool;
+use std::sync::{atomic::Ordering, Mutex};
 
 use lazy_static::lazy_static;
 
 use crate::{
-  check_status, check_status_or_throw, sys, JsError, JsFunction, Property, Result, Value, ValueType,
+  check_status, check_status_or_throw, sys, Env, JsError, JsFunction, Property, Result, Value,
+  ValueType,
 };
 
 pub type ExportRegisterCallback = unsafe fn(sys::napi_env) -> Result<sys::napi_value>;
@@ -16,64 +20,61 @@ pub type ModuleExportsCallback =
   unsafe fn(env: sys::napi_env, exports: sys::napi_value) -> Result<()>;
 
 struct PersistedSingleThreadVec<T> {
-  inner: AtomicPtr<T>,
+  inner: Mutex<Vec<T>>,
-  length: AtomicUsize,
 }
 
 impl<T> Default for PersistedSingleThreadVec<T> {
   fn default() -> Self {
-    let mut vec: Vec<T> = Vec::with_capacity(1);
+    PersistedSingleThreadVec {
-    let ret = PersistedSingleThreadVec {
+      inner: Mutex::new(Vec::new()),
-      inner: AtomicPtr::new(vec.as_mut_ptr()),
+    }
-      length: AtomicUsize::new(0),
-    };
-    std::mem::forget(vec);
-    ret
   }
 }
 
 impl<T> PersistedSingleThreadVec<T> {
   #[allow(clippy::mut_from_ref)]
-  fn borrow_mut(&self) -> &mut [T] {
+  fn borrow_mut<F>(&self, f: F)
-    let length = self.length.load(Ordering::Relaxed);
+  where
-    if length == 0 {
+    F: FnOnce(&mut [T]),
-      return &mut [];
+  {
-    }
+    let mut locked = self
-    let inner = self.inner.load(Ordering::Relaxed);
+      .inner
-    unsafe { std::slice::from_raw_parts_mut(inner, length) }
+      .lock()
+      .expect("Acquire persisted thread vec lock failed");
+    f(&mut *locked);
   }
 
   fn push(&self, item: T) {
-    let length = self.length.load(Ordering::Relaxed);
+    let mut locked = self
-    let inner = self.inner.load(Ordering::Relaxed);
+      .inner
-    let mut temp = unsafe { Vec::from_raw_parts(inner, length, length) };
+      .lock()
-    temp.push(item);
+      .expect("Acquire persisted thread vec lock failed");
-    // Inner Vec has been reallocated, so we need to update the pointer
+    locked.push(item);
-    if temp.as_mut_ptr() != inner {
-      self.inner.store(temp.as_mut_ptr(), Ordering::Relaxed);
-    }
-    std::mem::forget(temp);
-
-    self.length.fetch_add(1, Ordering::Relaxed);
   }
 }
 
 unsafe impl<T> Send for PersistedSingleThreadVec<T> {}
 unsafe impl<T> Sync for PersistedSingleThreadVec<T> {}
 
-struct PersistedSingleThreadHashMap<K, V>(*mut HashMap<K, V>);
+struct PersistedSingleThreadHashMap<K, V>(Mutex<HashMap<K, V>>);
 
 impl<K, V> PersistedSingleThreadHashMap<K, V> {
   #[allow(clippy::mut_from_ref)]
-  fn borrow_mut(&self) -> &mut HashMap<K, V> {
+  fn borrow_mut<F, R>(&self, f: F) -> R
-    unsafe { Box::leak(Box::from_raw(self.0)) }
+  where
+    F: FnOnce(&mut HashMap<K, V>) -> R,
+  {
+    let mut lock = self
+      .0
+      .lock()
+      .expect("Acquire persisted thread hash map lock failed");
+    f(&mut *lock)
   }
 }
 
 impl<K, V> Default for PersistedSingleThreadHashMap<K, V> {
   fn default() -> Self {
-    let map = Default::default();
+    PersistedSingleThreadHashMap(Mutex::new(Default::default()))
-    PersistedSingleThreadHashMap(Box::into_raw(Box::new(map)))
   }
 }
 
@@ -85,18 +86,21 @@ type ModuleClassProperty = PersistedSingleThreadHashMap<
   HashMap<Option<&'static str>, (&'static str, Vec<Property>)>,
 >;
 
-type FnRegisterMap = PersistedSingleThreadHashMap<
-  ExportRegisterCallback,
-  (sys::napi_env, sys::napi_callback, &'static str),
->;
-
 unsafe impl<K, V> Send for PersistedSingleThreadHashMap<K, V> {}
 unsafe impl<K, V> Sync for PersistedSingleThreadHashMap<K, V> {}
 
 lazy_static! {
   static ref MODULE_REGISTER_CALLBACK: ModuleRegisterCallback = Default::default();
   static ref MODULE_CLASS_PROPERTIES: ModuleClassProperty = Default::default();
-  static ref FN_REGISTER_MAP: FnRegisterMap = Default::default();
+  static ref MODULE_REGISTER_LOCK: Mutex<()> = Mutex::new(());
+  static ref REGISTERED: AtomicBool = AtomicBool::new(false);
+}
+
+#[inline]
+fn wait_first_thread_registered() {
+  while !REGISTERED.load(Ordering::SeqCst) {
+    std::hint::spin_loop();
+  }
 }
 
 #[cfg(feature = "compat-mode")]
@@ -107,13 +111,15 @@ lazy_static! {
 
 thread_local! {
   static REGISTERED_CLASSES: RefCell<HashMap<
-    /* export name */ &'static str,
+    /* export name */ String,
     /* constructor */ sys::napi_ref,
   >> = Default::default();
+  static FN_REGISTER_MAP: RefCell<HashMap<ExportRegisterCallback, (sys::napi_callback, String)>> = Default::default();
 }
 
 #[doc(hidden)]
 pub fn get_class_constructor(js_name: &'static str) -> Option<sys::napi_ref> {
+  wait_first_thread_registered();
   REGISTERED_CLASSES.with(|registered_classes| {
     let classes = registered_classes.borrow();
     classes.get(js_name).copied()
@@ -139,11 +145,12 @@ pub fn register_module_export(
 #[doc(hidden)]
 pub fn register_js_function(
   name: &'static str,
-  env: sys::napi_env,
   cb: ExportRegisterCallback,
   c_fn: sys::napi_callback,
 ) {
-  FN_REGISTER_MAP.borrow_mut().insert(cb, (env, c_fn, name));
+  FN_REGISTER_MAP.with(|inner| {
+    inner.borrow_mut().insert(cb, (c_fn, name.to_owned()));
+  });
 }
 
 #[doc(hidden)]
@@ -153,12 +160,12 @@ pub fn register_class(
   js_name: &'static str,
   props: Vec<Property>,
 ) {
-  let map = MODULE_CLASS_PROPERTIES.borrow_mut();
+  MODULE_CLASS_PROPERTIES.borrow_mut(|inner| {
-  let val = map.entry(rust_name).or_default();
+    let val = inner.entry(rust_name).or_default();
     let val = val.entry(js_mod).or_default();
-
     val.0 = js_name;
     val.1.extend(props.into_iter());
+  });
 }
 
 #[inline]
@@ -179,17 +186,19 @@ pub fn register_class(
 /// returnSomeFn()(); // 1
 /// ```
 ///
-pub fn get_js_function(raw_fn: ExportRegisterCallback) -> Result<JsFunction> {
+pub fn get_js_function(env: &Env, raw_fn: ExportRegisterCallback) -> Result<JsFunction> {
-  FN_REGISTER_MAP
+  wait_first_thread_registered();
-    .borrow_mut()
+  FN_REGISTER_MAP.with(|inner| {
+    inner
+      .borrow()
       .get(&raw_fn)
-    .and_then(|(env, cb, name)| {
+      .and_then(|(cb, name)| {
         let mut function = ptr::null_mut();
         let name_len = name.len() - 1;
         let fn_name = unsafe { CStr::from_bytes_with_nul_unchecked(name.as_bytes()) };
         check_status!(unsafe {
           sys::napi_create_function(
-          *env,
+            env.0,
             fn_name.as_ptr(),
             name_len,
             *cb,
@@ -199,7 +208,7 @@ pub fn get_js_function(raw_fn: ExportRegisterCallback) -> Result<JsFunction> {
         })
         .ok()?;
         Some(JsFunction(Value {
-        env: *env,
+          env: env.0,
           value: function,
           value_type: ValueType::Function,
         }))
@@ -210,6 +219,7 @@ pub fn get_js_function(raw_fn: ExportRegisterCallback) -> Result<JsFunction> {
           "JavaScript function does not exist".to_owned(),
         )
       })
+  })
 }
 
 /// Get `C Callback` from defined Rust `fn`
@@ -232,16 +242,19 @@ pub fn get_js_function(raw_fn: ExportRegisterCallback) -> Result<JsFunction> {
 /// ```
 ///
 pub fn get_c_callback(raw_fn: ExportRegisterCallback) -> Result<crate::Callback> {
-  FN_REGISTER_MAP
+  wait_first_thread_registered();
-    .borrow_mut()
+  FN_REGISTER_MAP.with(|inner| {
+    inner
+      .borrow()
       .get(&raw_fn)
-    .and_then(|(_env, cb, _name)| *cb)
+      .and_then(|(cb, _name)| *cb)
       .ok_or_else(|| {
         crate::Error::new(
           crate::Status::InvalidArg,
           "JavaScript function does not exist".to_owned(),
         )
       })
+  })
 }
 
 #[no_mangle]
@@ -249,9 +262,12 @@ unsafe extern "C" fn napi_register_module_v1(
   env: sys::napi_env,
   exports: sys::napi_value,
 ) -> sys::napi_value {
-  let mut exports_objects: HashMap<Option<&'static str>, sys::napi_value> = HashMap::default();
+  let lock = MODULE_REGISTER_LOCK
-  MODULE_REGISTER_CALLBACK
+    .lock()
-    .borrow_mut()
+    .expect("Failed to acquire module register lock");
+  let mut exports_objects: HashSet<String> = HashSet::default();
+  MODULE_REGISTER_CALLBACK.borrow_mut(|inner| {
+    inner
       .iter_mut()
       .fold(
         HashMap::<Option<&'static str>, Vec<(&'static str, ExportRegisterCallback)>>::new(),
@@ -268,8 +284,22 @@ unsafe extern "C" fn napi_register_module_v1(
       .for_each(|(js_mod, items)| {
         let mut exports_js_mod = ptr::null_mut();
         if let Some(js_mod_str) = js_mod {
-        if let Some(exports_object) = exports_objects.get(js_mod) {
+          let mod_name_c_str =
-          exports_js_mod = *exports_object;
+            unsafe { CStr::from_bytes_with_nul_unchecked(js_mod_str.as_bytes()) };
+          if exports_objects.contains(*js_mod_str) {
+            check_status_or_throw!(
+              env,
+              unsafe {
+                sys::napi_get_named_property(
+                  env,
+                  exports,
+                  mod_name_c_str.as_ptr(),
+                  &mut exports_js_mod,
+                )
+              },
+              "Get mod {} from exports failed",
+              js_mod_str,
+            );
           } else {
             check_status_or_throw!(
               env,
@@ -280,22 +310,17 @@ unsafe extern "C" fn napi_register_module_v1(
             check_status_or_throw!(
               env,
               unsafe {
-              sys::napi_set_named_property(
+                sys::napi_set_named_property(env, exports, mod_name_c_str.as_ptr(), exports_js_mod)
-                env,
-                exports,
-                js_mod_str.as_ptr() as *const _,
-                exports_js_mod,
-              )
               },
               "Set exports Object [{}] into exports object failed",
               js_mod_str
             );
-          exports_objects.insert(*js_mod, exports_js_mod);
+            exports_objects.insert(js_mod_str.to_string());
           }
         }
         for (name, callback) in items {
-        let js_name = unsafe { CStr::from_bytes_with_nul_unchecked(name.as_bytes()) };
           unsafe {
+            let js_name = CStr::from_bytes_with_nul_unchecked(name.as_bytes());
             if let Err(e) = callback(env).and_then(|v| {
               let exported_object = if exports_js_mod.is_null() {
                 exports
@@ -312,18 +337,28 @@ unsafe extern "C" fn napi_register_module_v1(
             }
           }
         }
+      })
   });
 
-  MODULE_CLASS_PROPERTIES
+  MODULE_CLASS_PROPERTIES.borrow_mut(|inner| {
-    .borrow_mut()
+    inner.iter().for_each(|(rust_name, js_mods)| {
-    .iter()
-    .for_each(|(rust_name, js_mods)| {
       for (js_mod, (js_name, props)) in js_mods {
         let mut exports_js_mod = ptr::null_mut();
         unsafe {
           if let Some(js_mod_str) = js_mod {
-            if let Some(exports_object) = exports_objects.get(js_mod) {
+            let mod_name_c_str = CStr::from_bytes_with_nul_unchecked(js_mod_str.as_bytes());
-              exports_js_mod = *exports_object;
+            if exports_objects.contains(*js_mod_str) {
+              check_status_or_throw!(
+                env,
+                sys::napi_get_named_property(
+                  env,
+                  exports,
+                  mod_name_c_str.as_ptr(),
+                  &mut exports_js_mod,
+                ),
+                "Get mod {} from exports failed",
+                js_mod_str,
+              );
             } else {
               check_status_or_throw!(
                 env,
@@ -333,16 +368,11 @@ unsafe extern "C" fn napi_register_module_v1(
               );
               check_status_or_throw!(
                 env,
-                sys::napi_set_named_property(
+                sys::napi_set_named_property(env, exports, mod_name_c_str.as_ptr(), exports_js_mod),
-                  env,
-                  exports,
-                  js_mod_str.as_ptr() as *const _,
-                  exports_js_mod
-                ),
                 "Set exports Object [{}] into exports object failed",
                 js_mod_str
               );
-              exports_objects.insert(*js_mod, exports_js_mod);
+              exports_objects.insert(js_mod_str.to_string());
             }
           }
           let (ctor, props): (Vec<_>, Vec<_>) = props.iter().partition(|prop| prop.is_ctor);
@@ -354,7 +384,7 @@ unsafe extern "C" fn napi_register_module_v1(
           let ctor = ctor.get(0).map(|c| c.raw().method.unwrap()).unwrap_or(noop);
           let raw_props: Vec<_> = props.iter().map(|prop| prop.raw()).collect();
 
-          let js_class_name = CStr::from_bytes_with_nul_unchecked(js_name.as_bytes());
+          let js_class_name = CString::from_vec_with_nul_unchecked(js_name.as_bytes().to_vec());
           let mut class_ptr = ptr::null_mut();
 
           check_status_or_throw!(
@@ -362,7 +392,7 @@ unsafe extern "C" fn napi_register_module_v1(
             sys::napi_define_class(
               env,
               js_class_name.as_ptr(),
-              js_name.len(),
+              js_name.len() - 1,
               Some(ctor),
               ptr::null_mut(),
               raw_props.len(),
@@ -379,7 +409,7 @@ unsafe extern "C" fn napi_register_module_v1(
 
           REGISTERED_CLASSES.with(|registered_classes| {
             let mut registered_class = registered_classes.borrow_mut();
-            registered_class.insert(js_name, ctor_ref);
+            registered_class.insert(js_name.to_string(), ctor_ref);
           });
 
           check_status_or_throw!(
@@ -400,22 +430,22 @@ unsafe extern "C" fn napi_register_module_v1(
           );
         }
       }
+    })
   });
 
   #[cfg(feature = "compat-mode")]
-  MODULE_EXPORTS
+  MODULE_EXPORTS.borrow_mut(|inner| {
-    .borrow_mut()
+    inner.iter().for_each(|callback| unsafe {
-    .iter()
-    .for_each(|callback| unsafe {
       if let Err(e) = callback(env, exports) {
         JsError::from(e).throw_into(env);
       }
+    })
   });
 
   #[cfg(all(feature = "tokio_rt", feature = "napi4"))]
   {
     let _ = crate::tokio_runtime::RT.clone();
-    crate::tokio_runtime::TOKIO_RT_REF_COUNT.fetch_add(1, Ordering::Relaxed);
+    crate::tokio_runtime::TOKIO_RT_REF_COUNT.fetch_add(1, Ordering::SeqCst);
     assert_eq!(
       unsafe {
         sys::napi_add_env_cleanup_hook(env, Some(crate::shutdown_tokio_rt), env as *mut c_void)
@@ -423,7 +453,8 @@ unsafe extern "C" fn napi_register_module_v1(
       sys::Status::napi_ok
     );
   }
-
+  mem::drop(lock);
+  REGISTERED.store(true, Ordering::SeqCst);
   exports
 }
 
@@ -431,7 +462,7 @@ pub(crate) unsafe extern "C" fn noop(
   env: sys::napi_env,
   _info: sys::napi_callback_info,
 ) -> sys::napi_value {
-  if !crate::bindgen_runtime::___CALL_FROM_FACTORY.load(std::sync::atomic::Ordering::Relaxed) {
+  if !crate::bindgen_runtime::___CALL_FROM_FACTORY.with(|inner| inner.load(Ordering::Relaxed)) {
     unsafe {
       sys::napi_throw_error(
         env,

crates/napi/src/js_values/de.rs

@@ -20,7 +20,7 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> {
   where
     V: Visitor<'x>,
   {
-    let js_value_type = unsafe { type_of!(self.0.env, self.0.value) }?;
+    let js_value_type = type_of!(self.0.env, self.0.value)?;
     match js_value_type {
       ValueType::Null | ValueType::Undefined => visitor.visit_unit(),
       ValueType::Boolean => {
@@ -89,7 +89,7 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> {
   where
     V: Visitor<'x>,
   {
-    match unsafe { type_of!(self.0.env, self.0.value) }? {
+    match type_of!(self.0.env, self.0.value)? {
       ValueType::Undefined | ValueType::Null => visitor.visit_none(),
       _ => visitor.visit_some(self),
     }
@@ -104,7 +104,7 @@ impl<'x, 'de, 'env> serde::de::Deserializer<'x> for &'de mut De<'env> {
   where
     V: Visitor<'x>,
   {
-    let js_value_type = unsafe { type_of!(self.0.env, self.0.value)? };
+    let js_value_type = type_of!(self.0.env, self.0.value)?;
     match js_value_type {
       ValueType::String => visitor.visit_enum(JsEnumAccess::new(
         unsafe { JsString::from_raw_unchecked(self.0.env, self.0.value) }

crates/napi/src/js_values/mod.rs

@@ -669,7 +669,7 @@ impl<'env> NapiRaw for &'env JsUnknown {
 
 impl JsUnknown {
   pub fn get_type(&self) -> Result<ValueType> {
-    unsafe { type_of!(self.0.env, self.0.value) }
+    type_of!(self.0.env, self.0.value)
   }
 
   /// # Safety

crates/napi/src/tokio_runtime.rs

@@ -36,7 +36,7 @@ pub(crate) static TOKIO_RT_REF_COUNT: AtomicUsize = AtomicUsize::new(0);
 #[doc(hidden)]
 #[inline(never)]
 pub unsafe extern "C" fn shutdown_tokio_rt(arg: *mut c_void) {
-  if TOKIO_RT_REF_COUNT.fetch_sub(1, Ordering::Relaxed) == 0 {
+  if TOKIO_RT_REF_COUNT.fetch_sub(1, Ordering::SeqCst) == 0 {
     let sender = &RT.1;
     if let Err(e) = sender.clone().try_send(()) {
       match e {

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

@@ -3,18 +3,25 @@ import { Worker } from 'worker_threads'
 
 import test from 'ava'
 
-import { DEFAULT_COST, Animal, Kind } from '../index'
+import { Animal, Kind, DEFAULT_COST } from '../index'
 
-test('should be able to require in worker thread', (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'))
-  return new Promise<void>((resolve) => {
+      return new Promise<void>((resolve, reject) => {
         w.on('message', (msg) => {
           t.is(msg, Animal.withKind(Kind.Cat).whoami() + DEFAULT_COST)
           resolve()
         })
+        w.on('error', (err) => {
+          reject(err)
+        })
       })
         .then(() => w.terminate())
         .then(() => {
           t.pass()
         })
+    }),
+  )
 })

examples/napi/src/callback.rs

@@ -43,6 +43,6 @@ fn read_file_content() -> Result<String> {
 }
 
 #[napi]
-fn return_js_function() -> Result<JsFunction> {
+fn return_js_function(env: Env) -> Result<JsFunction> {
-  get_js_function(read_file_js_function)
+  get_js_function(&env, read_file_js_function)
 }

package.json

@@ -33,7 +33,7 @@
     "format:toml": "taplo format",
     "lint": "eslint -c .eslintrc.yml .",
     "prepublishOnly": "npm run build && pinst --disable",
-    "test": "ava",
+    "test": "ava \"./examples/napi/**/*.ts\" && ava --no-worker-threads \"./examples/napi-compat-mode/**/*.ts\" && ava \"./cli/**/*.ts\"",
     "test:memory": "node memory-testing/index.mjs",
     "postinstall": "husky install",
     "postpublish": "pinst --enable"
@@ -60,7 +60,7 @@
       "taplo format"
     ],
     "*.rs": [
-      "cargo fmt"
+      "cargo fmt --"
     ]
   },
   "husky": {