fix: Fix race condition on shutdown

crates/neon/src/sys/no_panic.rs

@@ -50,8 +50,8 @@ impl FailureBoundary {
     where
         F: FnOnce(Option<Env>) -> Local,
     {
-        // Event loop has terminated if `null`
-        let env = if env.is_null() { None } else { Some(env) };
+        // Make `env = None` if unable to call into JS
+        let env = can_call_into_js(env).then(|| env);
 
         // Run the user supplied callback, catching panics
         // This is unwind safe because control is never yielded back to the caller
@@ -121,6 +121,12 @@ impl FailureBoundary {
     }
 }
 
+// HACK: Force `NAPI_PREAMBLE` to run without executing any JavaScript to tell if it's
+// possible to call into JS.
+fn can_call_into_js(env: Env) -> bool {
+    !env.is_null() && unsafe { napi::throw(env, ptr::null_mut()) == napi::Status::InvalidArg }
+}
+
 // We cannot use `napi_fatal_exception` because of this bug; instead, cause an
 // unhandled rejection which has similar behavior on recent versions of Node.
 // https://github.com/nodejs/node/issues/33771

test/napi/lib/workers.js

@@ -10,6 +10,9 @@ const addon = require("..");
 
 // Receive a message, try that method and return the error message
 if (!isMainThread) {
+  // Attempt to reproduce shutdown race condition bug
+  addon.reject_after(new Error("Oh, no!"), 200).catch(() => {});
+
   addon.get_or_init_thread_id(threadId);
   parentPort.once("message", (message) => {
     try {
@@ -187,4 +190,11 @@ describe("Instance-local storage", () => {
 
     worker.postMessage("get_thread_id");
   });
+
+  it("should be able to exit a worker without a crash", (cb) => {
+    const worker = new Worker(__filename);
+
+    setTimeout(() => worker.terminate(), 50);
+    setTimeout(cb, 100);
+  });
 });

test/napi/src/js/workers.rs

@@ -1,4 +1,4 @@
-use std::sync::Mutex;
+use std::{convert::TryFrom, sync::Mutex, thread, time::Duration};
 
 use once_cell::sync::{Lazy, OnceCell};
 
@@ -90,3 +90,21 @@ pub fn unstash_global_object(mut cx: FunctionContext) -> JsResult<JsValue> {
         None => cx.null().upcast(),
     })
 }
+
+pub fn reject_after(mut cx: FunctionContext) -> JsResult<JsPromise> {
+    let err = cx.argument::<JsObject>(0)?.root(&mut cx);
+    let ms = cx.argument::<JsNumber>(1)?.value(&mut cx) as i64;
+    let ms = u64::try_from(ms)
+        .or_else(|err| cx.throw_error(err.to_string()))
+        .map(Duration::from_millis)?;
+
+    let promise =
+        cx.task(move || thread::sleep(ms))
+            .promise(move |mut cx, _| -> JsResult<JsValue> {
+                let err = err.into_inner(&mut cx);
+
+                cx.throw(err)
+            });
+
+    Ok(promise)
+}

test/napi/src/lib.rs

@@ -345,6 +345,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
     cx.export_function("get_reentrant_value", js::workers::get_reentrant_value)?;
     cx.export_function("stash_global_object", js::workers::stash_global_object)?;
     cx.export_function("unstash_global_object", js::workers::unstash_global_object)?;
+    cx.export_function("reject_after", js::workers::reject_after)?;
 
     Ok(())
 }