napi: read the VM exception slot raw at a gated call's entry, never service a trap there

src/jsc/bindings/napi.cpp

@@ -85,11 +85,12 @@
 using namespace JSC;
 using namespace Zig;
 
-// Node: RETURN_STATUS_IF_FALSE(env, env->can_call_into_js(), ...)
-#define NAPI_RETURN_IF_CANNOT_CALL_INTO_JS(_env)                                \
-    do {                                                                        \
-        if (!(_env)->canCallIntoJS()) [[unlikely]]                              \
-            return napi_set_last_error(_env, (_env)->cannotCallIntoJSStatus()); \
+// Node's NAPI_PREAMBLE entry check: NapiEnv::entryStatus.
+#define NAPI_RETURN_ENTRY_STATUS_WITH_SCOPE(_env, _scope)                  \
+    do {                                                                   \
+        if (napi_status napi_entry_status__ = (_env)->entryStatus(_scope); \
+            napi_entry_status__ != napi_ok) [[unlikely]]                   \
+            return napi_set_last_error(_env, napi_entry_status__);         \
     } while (0)
 
 // Every NAPI function should use this at the start. It does the following:
@@ -107,8 +108,7 @@ using namespace Zig;
     /* PREAMBLE then sees as an unchecked exception. If you need to throw */    \
     /* or clear exceptions, make your own scope. */                             \
     auto napi_preamble_throw_scope__ = DECLARE_TOP_EXCEPTION_SCOPE(_env->vm()); \
-    NAPI_RETURN_IF_EXCEPTION(_env);                                             \
-    NAPI_RETURN_IF_CANNOT_CALL_INTO_JS(_env);
+    NAPI_RETURN_ENTRY_STATUS_WITH_SCOPE(_env, napi_preamble_throw_scope__);
 
 // Only use this for functions that need their own throw or catch scope. Functions that call into
 // JS code that might throw should use NAPI_RETURN_IF_EXCEPTION.
@@ -118,17 +118,18 @@ using namespace Zig;
         NAPI_CHECK_ARG(_env, _env);        \
     } while (0)
 
-// NAPI_PREAMBLE_NO_THROW_SCOPE plus the can_call_into_js gate, for a function Node opens with NAPI_PREAMBLE.
-#define NAPI_PREAMBLE_NO_THROW_SCOPE_GATED(_env)  \
-    do {                                          \
-        NAPI_PREAMBLE_NO_THROW_SCOPE(_env);       \
-        NAPI_RETURN_IF_CANNOT_CALL_INTO_JS(_env); \
+// NAPI_PREAMBLE_NO_THROW_SCOPE plus the entry check, for a function Node opens with NAPI_PREAMBLE.
+#define NAPI_PREAMBLE_NO_THROW_SCOPE_GATED(_env)                                      \
+    do {                                                                              \
+        NAPI_PREAMBLE_NO_THROW_SCOPE(_env);                                           \
+        auto napi_preamble_entry_scope__ = DECLARE_TOP_EXCEPTION_SCOPE((_env)->vm()); \
+        NAPI_RETURN_ENTRY_STATUS_WITH_SCOPE(_env, napi_preamble_entry_scope__);       \
     } while (0)
 
 // What the value constructors/accessors Node gates with CHECK_ENV only run under (NAPI_PREAMBLE_NO_PENDING_CHECK
 // here, `ungated!` in napi_body.rs): callable with an exception pending (stashed for the call; only then, since the
 // restore is unconditional) and never where a worker.terminate() / node:vm timeout is delivered (DeferTraps: the
-// next exception check after the call delivers it, as in Node). No JS or addon code may run under it.
+// next JSC exception check after the call delivers it, as in Node). No JS or addon code may run under it.
 struct NapiUngatedScope {
     explicit NapiUngatedScope(JSC::VM& vm)
         : deferTraps(vm)
@@ -1071,11 +1072,6 @@ static napi_status throwErrorWithCStrings(napi_env env, const char* code_utf8, c
     auto* globalObject = toJS(env);
     auto& vm = JSC::getVM(globalObject);
 
-    auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
-    if (scope.exception() || env->hasPendingException()) {
-        return napi_set_last_error(env, napi_pending_exception);
-    }
-
     if (!msg_utf8) {
         return napi_set_last_error(env, napi_invalid_arg);
     }
@@ -1370,7 +1366,9 @@ extern "C" napi_status napi_is_exception_pending(napi_env env, bool* result)
             auto& vm = JSC::getVM(globalObject);
             // Use a catch scope instead of throw scope for safety during cleanup
             auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
-            *result = scope.exception() != nullptr;
+            // A pending termination is not an exception the addon can read (NapiEnv::entryStatus).
+            auto* exception = scope.exception();
+            *result = exception && !vm.isTerminationException(exception);
         }
     }
 
@@ -1388,9 +1386,12 @@ extern "C" napi_status napi_get_and_clear_last_exception(napi_env env,
     }
 
     auto globalObject = toJS(env);
-    auto scope = DECLARE_TOP_EXCEPTION_SCOPE(JSC::getVM(globalObject));
-    if (scope.exception()) [[unlikely]] {
-        *result = toNapi(JSValue(scope.exception()->value()), globalObject);
+    auto& vm = JSC::getVM(globalObject);
+    auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
+    auto* exception = scope.exception();
+    // A pending termination is not handed out (NapiEnv::entryStatus); tryClearException leaves it too.
+    if (exception && !vm.isTerminationException(exception)) [[unlikely]] {
+        *result = toNapi(JSValue(exception->value()), globalObject);
     } else if (std::optional<JSValue> pending = env->pendingException()) {
         *result = toNapi(pending.value(), globalObject);
         env->clearPendingException();
@@ -1597,7 +1598,6 @@ extern "C" JS_EXPORT napi_status node_api_create_buffer_from_arraybuffer(napi_en
     NAPI_PREAMBLE_NO_THROW_SCOPE_GATED(env);
     auto* globalObject = toJS(env);
     auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject));
-    NAPI_RETURN_IF_EXCEPTION_WITH_SCOPE(env, scope);
     NAPI_CHECK_ARG(env, arraybuffer);
     NAPI_CHECK_ARG(env, result);
 
@@ -1774,7 +1774,6 @@ extern "C" JS_EXPORT napi_status node_api_create_external_sharedarraybuffer(napi
     NAPI_PREAMBLE(env);
     NAPI_CHECK_ENV_NOT_IN_GC(env);
     NAPI_CHECK_ARG(env, result);
-    NAPI_RETURN_EARLY_IF_FALSE(env, !env->hasPendingException(), napi_pending_exception);
 
     Zig::GlobalObject* globalObject = toJS(env);
     JSC::VM& vm = JSC::getVM(globalObject);
@@ -1879,7 +1878,6 @@ extern "C" napi_status napi_create_dataview(napi_env env, size_t length,
     NAPI_PREAMBLE_NO_THROW_SCOPE_GATED(env);
     Zig::GlobalObject* globalObject = toJS(env);
     auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject));
-    NAPI_RETURN_IF_EXCEPTION_WITH_SCOPE(env, scope);
     NAPI_CHECK_ARG(env, arraybuffer);
     NAPI_CHECK_ARG(env, result);
     JSValue arraybufferValue = toJS(arraybuffer);
@@ -2298,7 +2296,6 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length,
     NAPI_PREAMBLE_NO_THROW_SCOPE_GATED(env);
     Zig::GlobalObject* globalObject = toJS(env);
     auto scope = DECLARE_THROW_SCOPE(env->vm());
-    NAPI_RETURN_IF_EXCEPTION_WITH_SCOPE(env, scope);
     NAPI_CHECK_ARG(env, result);
 
     auto* subclassStructure = globalObject->JSBufferSubclassStructure();
@@ -2336,7 +2333,6 @@ extern "C" napi_status napi_create_buffer_copy(napi_env env, size_t length,
     NAPI_PREAMBLE_NO_THROW_SCOPE_GATED(env);
     Zig::GlobalObject* globalObject = toJS(env);
     auto scope = DECLARE_THROW_SCOPE(env->vm());
-    NAPI_RETURN_IF_EXCEPTION_WITH_SCOPE(env, scope);
     NAPI_CHECK_ARG(env, result);
 
     auto* subclassStructure = globalObject->JSBufferSubclassStructure();
@@ -3050,7 +3046,6 @@ extern "C" napi_status napi_run_script(napi_env env, napi_value script,
     Zig::GlobalObject* globalObject = toJS(env);
     auto& vm = JSC::getVM(globalObject);
     auto throwScope = DECLARE_THROW_SCOPE(vm);
-    NAPI_RETURN_IF_EXCEPTION_WITH_SCOPE(env, throwScope);
     NAPI_CHECK_ARG(env, script);
     NAPI_CHECK_ARG(env, result);
     JSValue scriptValue = toJS(script);
@@ -3122,7 +3117,6 @@ extern "C" napi_status napi_create_bigint_words(napi_env env,
     Zig::GlobalObject* globalObject = toJS(env);
     auto& vm = env->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
-    NAPI_RETURN_IF_EXCEPTION_WITH_SCOPE(env, scope);
     NAPI_CHECK_ARG(env, result);
     NAPI_CHECK_ARG(env, words);
     NAPI_RETURN_EARLY_IF_FALSE(env, word_count <= INT_MAX, napi_invalid_arg);
@@ -3420,26 +3414,18 @@ extern "C" bool NapiEnv__setCompletingForStoppedContext(napi_env env, bool value
     return std::exchange(env->m_isCompletingForStoppedContext, value);
 }
 
-extern "C" bool NapiEnv__hasPendingException(napi_env env)
+// napi_ok, or the status NAPI_PREAMBLE returns at entry (not yet set as the last error).
+extern "C" napi_status NapiEnv__entryStatus(napi_env env)
 {
-    if (env->hasPendingException()) {
-        return true;
-    }
     auto scope = DECLARE_TOP_EXCEPTION_SCOPE(env->vm());
-    return scope.exception() != nullptr;
+    return env->entryStatus(scope);
 }
 
 extern "C" uint32_t Bun__napi_get_version(napi_env env)
 {
     return env->napiModule().nm_version;
 }
 
-// napi_ok, or the status NAPI_RETURN_IF_CANNOT_CALL_INTO_JS returns (not yet set as the last error).
-extern "C" napi_status NapiEnv__checkCanCallIntoJS(napi_env env)
-{
-    return env->canCallIntoJS() ? napi_ok : env->cannotCallIntoJSStatus();
-}
-
 extern "C" JSGlobalObject* NapiEnv__globalObject(napi_env env)
 {
     return env->globalObject();

src/jsc/bindings/napi.h

@@ -518,6 +518,24 @@ struct NapiEnv : public WTF::RefCounted<NapiEnv> {
         return m_napiModule.nm_version >= 10 ? napi_cannot_run_js : napi_pending_exception;
     }
 
+    // Node's NAPI_PREAMBLE entry check, in its order: napi_pending_exception for an exception the addon can
+    // read (on the env or on the VM), then the can_call_into_js() gate. The VM's slot is read raw, not through
+    // a JSC exception check: that would service the traps and deliver a worker.terminate() with only addon
+    // frames on the stack, where V8 never does. A termination already pending answers as the closed gate does.
+    inline napi_status entryStatus(JSC::TopExceptionScope& scope) const
+    {
+        if (m_pendingException) [[unlikely]] {
+            return napi_pending_exception;
+        }
+        if (auto* exception = scope.exception()) [[unlikely]] {
+            return m_vm.isTerminationException(exception) ? cannotCallIntoJSStatus() : napi_pending_exception;
+        }
+        if (!canCallIntoJS()) [[unlikely]] {
+            return cannotCallIntoJSStatus();
+        }
+        return napi_ok;
+    }
+
     // Almost all NAPI functions should set error_code to the status they're returning right before
     // they return it
     napi_extended_error_info m_lastNapiErrorInfo = {

src/runtime/napi/napi_body.rs

@@ -105,11 +105,10 @@ bun_opaque::opaque_ffi! {
 unsafe extern "C" {
     fn NapiEnv__globalObject(env: *mut NapiEnv) -> *mut JSGlobalObject;
     fn NapiEnv__getAndClearPendingException(env: *mut NapiEnv, out: *mut JSValue) -> bool;
-    fn NapiEnv__hasPendingException(env: *mut NapiEnv) -> bool;
     /// Returns the previous value.
     fn NapiEnv__setCompletingForStoppedContext(env: *mut NapiEnv, value: bool) -> bool;
-    /// `NapiStatus::ok`, or the status Node's `NAPI_PREAMBLE` returns when `can_call_into_js()` is false.
-    fn NapiEnv__checkCanCallIntoJS(env: *mut NapiEnv) -> napi_status;
+    /// `NapiStatus::ok`, or the status Node's `NAPI_PREAMBLE` returns at entry (`NapiEnv::entryStatus`).
+    fn NapiEnv__entryStatus(env: *mut NapiEnv) -> napi_status;
     fn NapiEnv__deref(env: *mut NapiEnv);
     fn NapiEnv__ref(env: *mut NapiEnv);
     /// The reference to its VM's handle the env holds (`BunVmHandleRef`).
@@ -187,17 +186,11 @@ impl NapiEnv {
         unsafe { NapiEnv__setCompletingForStoppedContext(self.as_mut_ptr(), previous) };
     }
 
-    /// Checks both `env->m_pendingException` (set by `napi_throw*`) and the JSC
-    /// VM exception slot. This is the gate Node.js's `NAPI_PREAMBLE` enforces.
-    pub(crate) fn has_pending_exception(&self) -> bool {
+    /// Node's `NAPI_PREAMBLE` entry check (napi.h `NapiEnv::entryStatus`). `Err` is the status Node
+    /// returns, set as the last error.
+    pub(crate) fn entry_status(&self) -> Result<(), napi_status> {
         // SAFETY: env is non-null; C++ side is read-only here.
-        unsafe { NapiEnv__hasPendingException(self.as_mut_ptr()) }
-    }
-
-    /// Node's `can_call_into_js()` gate. `Err` is the status Node returns, set as the last error.
-    pub(crate) fn check_can_call_into_js(&self) -> Result<(), napi_status> {
-        // SAFETY: env is non-null; C++ side is read-only here.
-        let status = unsafe { NapiEnv__checkCanCallIntoJS(self.as_mut_ptr()) };
+        let status = unsafe { NapiEnv__entryStatus(self.as_mut_ptr()) };
         if status == NapiStatus::ok as napi_status {
             return Ok(());
         }
@@ -535,15 +528,11 @@ macro_rules! get_env {
     };
 }
 
-/// Node's `NAPI_PREAMBLE`: `get_env!`, then `napi_pending_exception` if a JS
-/// exception is pending, then the `can_call_into_js()` gate.
+/// Node's `NAPI_PREAMBLE`: `get_env!`, then [`NapiEnv::entry_status`].
 macro_rules! preamble {
     ($env:expr) => {{
         let env = get_env!($env);
-        if env.has_pending_exception() {
-            return env.pending_exception();
-        }
-        if let Err(status) = env.check_can_call_into_js() {
+        if let Err(status) = env.entry_status() {
             return status;
         }
         env
@@ -1705,9 +1694,6 @@ fn conclude_deferred(
     deferred: napi_deferred,
     settle: impl FnOnce(&mut jsc::JSPromise, &JSGlobalObject) -> JsResult<()>,
 ) -> napi_status {
-    if env.has_pending_exception() {
-        return env.pending_exception();
-    }
     // SAFETY: deferred was created by heap::alloc in napi_create_promise.
     let context = unsafe { (*deferred).context };
     if !env.to_js().bun_vm().is_context_live(context) {
@@ -1718,7 +1704,7 @@ fn conclude_deferred(
     }
     let mut status = env.ok();
     env.complete_in_context(context, || {
-        if let Err(refused) = env.check_can_call_into_js() {
+        if let Err(refused) = env.entry_status() {
             status = refused;
             return;
         }

test/napi/napi-app/binding.gyp

@@ -373,5 +373,29 @@
                 "NAPI_VERSION=8",
             ],
         },
+        {
+            "target_name": "terminate_probe_addon_v9",
+            "sources": ["terminate_probe_addon.cpp"],
+            "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
+            "libraries": [],
+            "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],
+            "defines": [
+                "NAPI_DISABLE_CPP_EXCEPTIONS",
+                "NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS",
+                "NAPI_VERSION=9",
+            ],
+        },
+        {
+            "target_name": "terminate_probe_addon_v10",
+            "sources": ["terminate_probe_addon.cpp"],
+            "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
+            "libraries": [],
+            "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],
+            "defines": [
+                "NAPI_DISABLE_CPP_EXCEPTIONS",
+                "NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS",
+                "NAPI_VERSION=10",
+            ],
+        },
     ]
 }

test/napi/napi-app/module.js

@@ -405,7 +405,6 @@ nativeTests.test_get_all_property_names_throwing_proxy_traps = () => {
       napi_key_keep_numbers,
     ),
   );
-
 };
 
 nativeTests.test_get_all_property_names_get_prototype_throws_in_descriptor_walk = () => {
@@ -1950,6 +1949,38 @@ nativeTests.test_ungated_calls_worker_terminate = async () => {
   }
 };
 
+// A worker.terminate() that lands while the addon is inside a call Node gates
+// behind can_call_into_js(): its module Init, or a method entered from JS.
+// terminate_probe_addon.cpp records what the refused call, the exception
+// readers and napi_throw say, then makes one node-addon-api call refused (that
+// escalates to napi_fatal_error unless those agree the env is stopping). The
+// parent arms terminate() once the addon has created the signal file. `dir` is
+// the test's temporary directory; the probe file is left there for the test to
+// read if this process does not get to print it.
+nativeTests.test_terminate_during_gated_calls = async (_gc, version, frame, dir) => {
+  const { Worker } = require("node:worker_threads");
+  const fs = require("node:fs");
+  const path = require("node:path");
+  dir = path.join(dir, process.versions.bun ? "bun" : "node");
+  fs.mkdirSync(dir, { recursive: true });
+  const signal = path.join(dir, "signal");
+  const probe = path.join(dir, "probe");
+  const worker = new Worker(path.join(__dirname, "terminate-probe-worker.js"), {
+    workerData: { version, frame, signal, probe },
+  });
+  const failed = new Promise((_, reject) => {
+    worker.once("error", reject);
+    worker.once("exit", code => reject(new Error(`worker exited with ${code} before the signal`)));
+  });
+  const signalSeen = (async () => {
+    while (!fs.existsSync(signal)) await new Promise(resolve => setImmediate(resolve));
+  })();
+  await Promise.race([signalSeen, failed]);
+  worker.removeAllListeners("exit");
+  console.log("terminate() resolved with", await Promise.race([worker.terminate(), failed]));
+  console.log(fs.readFileSync(probe, "utf8"));
+};
+
 // See ungated_calls_through_timeout in standalone_tests.cpp: 600ms of ungated
 // calls under a 150ms timeout. The timeout is wall-clock from the start of the
 // run, so it must comfortably cover reaching the addon on the slowest lane.

test/napi/napi-app/terminate-probe-worker.js

@@ -0,0 +1,16 @@
+// Loads terminate_probe_addon_v{9,10}.node while the parent terminates this worker
+// (test_terminate_during_gated_calls in module.js). `frame` says where the addon spins
+// through gated calls: its module Init, a method entered from here, or script the
+// method enters.
+const { workerData } = require("node:worker_threads");
+const { version, frame, signal, probe } = workerData;
+globalThis.__terminateProbe = { signal, probe, inInit: frame === "init" };
+const addon = require(`./build/Debug/terminate_probe_addon_v${version}.node`);
+if (frame === "method") addon.spin(signal, probe);
+if (frame === "script") {
+  const { writeFileSync } = require("node:fs");
+  addon.spinScript(probe, () => {
+    writeFileSync(signal, "");
+    for (;;) {}
+  });
+}