diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index e7f5eebd156c..6b90a8f89d58 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -17,6 +17,7 @@ #include "helpers.h" #include +#include #include #include #include @@ -106,15 +107,37 @@ using namespace Zig; NAPI_CHECK_ARG(_env, _env); \ } while (0) -// Like NAPI_PREAMBLE but for pure value constructors/accessors, which Node lets an addon call while -// an exception is pending (CHECK_ENV_NOT_IN_GC only) — node-addon-api relies on that to build the -// Error it wraps a failed call in. Any exception already on the VM (a napi_throw*, or a termination -// request that materialised in an earlier call while a worker is being stopped) is stashed for the -// duration and restored on return; the throw scope still catches what the body itself raises. -#define NAPI_PREAMBLE_NO_PENDING_CHECK(_env) \ - NAPI_LOG_CURRENT_FUNCTION; \ - NAPI_CHECK_ARG(_env, _env); \ - JSC::SuspendExceptionScope napi_preamble_suspended_exception__ { _env->vm() }; \ +// 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. +struct NapiUngatedScope { + explicit NapiUngatedScope(JSC::VM& vm) + : deferTraps(vm) + { + if (vm.exceptionForInspection()) [[unlikely]] + suspended.emplace(vm); + } + std::optional suspended; + JSC::DeferTraps deferTraps; +}; + +// Constructed in place in storage owned by the Rust caller (napi_body.rs `UngatedScope`). +static_assert(sizeof(NapiUngatedScope) <= 80 && alignof(NapiUngatedScope) <= 8, "napi_body.rs UngatedScope storage is 80 bytes, 8-aligned"); +extern "C" void NapiUngatedScope__construct(void* storage, napi_env env) +{ + ASSERT(reinterpret_cast(storage) % alignof(NapiUngatedScope) == 0); + new (storage) NapiUngatedScope(env->vm()); +} +extern "C" void NapiUngatedScope__destruct(void* storage) +{ + static_cast(storage)->~NapiUngatedScope(); +} + +#define NAPI_PREAMBLE_NO_PENDING_CHECK(_env) \ + NAPI_LOG_CURRENT_FUNCTION; \ + NAPI_CHECK_ARG(_env, _env); \ + NapiUngatedScope napi_preamble_ungated__ { _env->vm() }; \ auto napi_preamble_throw_scope__ = DECLARE_TOP_EXCEPTION_SCOPE(_env->vm()); // Return an error code if arg is null. Only use for input validation. @@ -1444,23 +1467,17 @@ extern "C" napi_status napi_create_type_error(napi_env env, napi_value code, return createErrorWithNapiValues(env, code, msg, JSC::ErrorType::TypeError, result); } -extern "C" JS_EXPORT napi_status -node_api_create_external_string_latin1(napi_env env, - char* str, - size_t length, - napi_finalize finalize_callback, - void* finalize_hint, - napi_value* result, - bool* copied) +// On `disposeNow` the caller runs the addon's finalizer, which must not run under this function's preamble. +template +static napi_status createExternalString(napi_env env, Char* str, size_t length, napi_finalize finalize_callback, void* finalize_hint, napi_value* result, bool* copied, bool& disposeNow) { - // https://nodejs.org/api/n-api.html#node_api_create_external_string_latin1 NAPI_PREAMBLE_NO_PENDING_CHECK(env); // Node's CHECK_NEW_STRING_ARGS: str may be null when length is 0. NAPI_RETURN_EARLY_IF_FALSE(env, length == 0 || str != nullptr, napi_invalid_arg); NAPI_CHECK_ARG(env, result); NAPI_RETURN_EARLY_IF_FALSE(env, length == NAPI_AUTO_LENGTH || length <= INT_MAX, napi_invalid_arg); - length = length == NAPI_AUTO_LENGTH ? strlen(str) : length; + length = length == NAPI_AUTO_LENGTH ? std::char_traits::length(str) : length; Zig::GlobalObject* globalObject = toJS(env); if (copied) { @@ -1471,14 +1488,12 @@ node_api_create_external_string_latin1(napi_env env, // returning the empty string and disposing the caller's buffer immediately. if (length == 0) { *result = toNapi(JSC::jsEmptyString(JSC::getVM(globalObject)), globalObject); - env->doFinalizer(finalize_callback, str, finalize_hint); - // Ownership transferred; return ok even if doFinalizer promoted a - // pre-existing napi_throw to the VM, so the caller doesn't double-free. - return napi_set_last_error(env, napi_ok); + disposeNow = true; + NAPI_RETURN_SUCCESS(env); } - Ref impl = WTF::ExternalStringImpl::create({ reinterpret_cast(str), static_cast(length) }, finalize_hint, [finalize_callback, env](void* hint, void* str, unsigned length) { - NAPI_LOG("latin1 string finalizer"); + Ref impl = WTF::ExternalStringImpl::create({ reinterpret_cast(str), static_cast(length) }, finalize_hint, [finalize_callback, env](void* hint, void* str, unsigned) { + NAPI_LOG("external string finalizer"); env->doFinalizer(finalize_callback, str, hint); }); @@ -1490,6 +1505,26 @@ node_api_create_external_string_latin1(napi_env env, NAPI_RETURN_SUCCESS(env); } +extern "C" JS_EXPORT napi_status +node_api_create_external_string_latin1(napi_env env, + char* str, + size_t length, + napi_finalize finalize_callback, + void* finalize_hint, + napi_value* result, + bool* copied) +{ + // https://nodejs.org/api/n-api.html#node_api_create_external_string_latin1 + bool disposeNow = false; + napi_status status = createExternalString(env, str, length, finalize_callback, finalize_hint, result, copied, disposeNow); + if (disposeNow) { + env->doFinalizer(finalize_callback, str, finalize_hint); + // napi_ok even if the finalizer threw: the buffer is consumed either way. + return napi_set_last_error(env, napi_ok); + } + return status; +} + extern "C" JS_EXPORT napi_status node_api_create_external_string_utf16(napi_env env, char16_t* str, @@ -1500,40 +1535,13 @@ node_api_create_external_string_utf16(napi_env env, bool* copied) { // https://nodejs.org/api/n-api.html#node_api_create_external_string_utf16 - NAPI_PREAMBLE_NO_PENDING_CHECK(env); - // Node's CHECK_NEW_STRING_ARGS: str may be null when length is 0. - NAPI_RETURN_EARLY_IF_FALSE(env, length == 0 || str != nullptr, napi_invalid_arg); - NAPI_CHECK_ARG(env, result); - NAPI_RETURN_EARLY_IF_FALSE(env, length == NAPI_AUTO_LENGTH || length <= INT_MAX, napi_invalid_arg); - - length = length == NAPI_AUTO_LENGTH ? std::char_traits::length(str) : length; - Zig::GlobalObject* globalObject = toJS(env); - - if (copied) { - *copied = false; - } - - // WTF::ExternalStringImpl does not allow zero-length strings; match Node.js/V8 by - // returning the empty string and disposing the caller's buffer immediately. - if (length == 0) { - *result = toNapi(JSC::jsEmptyString(JSC::getVM(globalObject)), globalObject); + bool disposeNow = false; + napi_status status = createExternalString(env, str, length, finalize_callback, finalize_hint, result, copied, disposeNow); + if (disposeNow) { env->doFinalizer(finalize_callback, str, finalize_hint); - // Ownership transferred; return ok even if doFinalizer promoted a - // pre-existing napi_throw to the VM, so the caller doesn't double-free. return napi_set_last_error(env, napi_ok); } - - Ref impl = WTF::ExternalStringImpl::create({ reinterpret_cast(str), static_cast(length) }, finalize_hint, [finalize_callback, env](void* hint, void* str, unsigned length) { - NAPI_LOG("utf16 string finalizer"); - env->doFinalizer(finalize_callback, str, hint); - }); - - JSString* out = JSC::jsString(JSC::getVM(globalObject), WTF::String(WTF::move(impl))); - ensureStillAliveHere(out); - *result = toNapi(out, globalObject); - ensureStillAliveHere(out); - - NAPI_RETURN_SUCCESS(env); + return status; } extern "C" JS_EXPORT napi_status node_api_create_property_key_latin1(napi_env env, const char* str, size_t length, napi_value* result) @@ -2891,9 +2899,8 @@ extern "C" napi_status napi_get_value_bigint_int64(napi_env env, napi_value valu JSValue jsValue = toJS(value); NAPI_RETURN_EARLY_IF_FALSE(env, jsValue.isHeapBigInt(), napi_bigint_expected); - // toBigInt64 can throw if the value is not a bigint. we have already checked, so we shouldn't - // hit an exception here and it's okay to assert at the end *result = jsValue.toBigInt64(toJS(env)); + NAPI_RETURN_IF_VM_EXCEPTION(env); JSBigInt* bigint = jsValue.asHeapBigInt(); auto length = bigint->length(); @@ -2925,8 +2932,6 @@ extern "C" napi_status napi_get_value_bigint_uint64(napi_env env, napi_value val JSValue jsValue = toJS(value); NAPI_RETURN_EARLY_IF_FALSE(env, jsValue.isHeapBigInt(), napi_bigint_expected); - // toBigInt64 can throw if the value is not a bigint. we have already checked, so we shouldn't - // hit an exception here and it's okay to assert at the end *result = jsValue.toBigUInt64(toJS(env)); NAPI_RETURN_IF_VM_EXCEPTION(env); diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index 4ad87609e8e7..4696c1dab0bd 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -96,6 +96,8 @@ unsafe extern "C" { /// The reference to its VM's handle the env holds (`BunVmHandleRef`). fn NapiEnv__vmHandle(env: *mut NapiEnv) -> *const bun_jsc::vm_handle::Shared; fn napi_set_last_error(env: napi_env, status: NapiStatus) -> napi_status; + fn NapiUngatedScope__construct(storage: *mut c_void, env: *mut NapiEnv); + fn NapiUngatedScope__destruct(storage: *mut c_void); } impl NapiEnv { @@ -465,6 +467,43 @@ macro_rules! preamble { }}; } +/// napi.cpp's `NapiUngatedScope` (see the comment there), constructed in place in this storage. +#[repr(C, align(8))] +struct UngatedScope([core::mem::MaybeUninit; 80]); + +struct UngatedScopeGuard<'a>(&'a mut UngatedScope); + +impl UngatedScope { + #[inline] + fn enter<'a>( + storage: &'a mut core::mem::MaybeUninit, + env: &NapiEnv, + ) -> UngatedScopeGuard<'a> { + let this = storage.write(UngatedScope([core::mem::MaybeUninit::uninit(); 80])); + // SAFETY: storage is 80 bytes, 8-aligned, and stays put until the guard drops; napi.cpp + // static_asserts that NapiUngatedScope fits. + unsafe { NapiUngatedScope__construct(this.0.as_mut_ptr().cast(), env.as_mut_ptr()) }; + UngatedScopeGuard(this) + } +} + +impl Drop for UngatedScopeGuard<'_> { + #[inline] + fn drop(&mut self) { + // SAFETY: constructed by `enter`, destroyed exactly once here. + unsafe { NapiUngatedScope__destruct(self.0.0.as_mut_ptr().cast()) }; + } +} + +/// `get_env!`, then the rest of the function runs under napi.cpp's `NapiUngatedScope` (which see): no JS or addon code. +macro_rules! ungated { + ($env:ident, $raw:expr) => { + let $env = get_env!($raw); + let mut napi_ungated_storage = ::core::mem::MaybeUninit::::uninit(); + let _napi_ungated_scope = UngatedScope::enter(&mut napi_ungated_storage, $env); + }; +} + macro_rules! get_out { ($env:expr, $ptr:expr) => { // SAFETY: caller passes raw out pointer; we treat non-null as &mut borrow. @@ -514,7 +553,7 @@ unsafe extern "C" { #[unsafe(no_mangle)] extern "C" fn napi_get_undefined(env_: napi_env, result_: *mut napi_value) -> napi_status { bun_output::scoped_log!(napi, "napi_get_undefined"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); result.set(env, JSValue::UNDEFINED); @@ -524,7 +563,7 @@ extern "C" fn napi_get_undefined(env_: napi_env, result_: *mut napi_value) -> na #[unsafe(no_mangle)] extern "C" fn napi_get_null(env_: napi_env, result_: *mut napi_value) -> napi_status { bun_output::scoped_log!(napi, "napi_get_null"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); result.set(env, JSValue::NULL); @@ -542,7 +581,7 @@ extern "C" fn napi_get_boolean( result_: *mut napi_value, ) -> napi_status { bun_output::scoped_log!(napi, "napi_get_boolean"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); result.set(env, JSValue::from(value)); @@ -552,7 +591,7 @@ extern "C" fn napi_get_boolean( #[unsafe(no_mangle)] extern "C" fn napi_create_array(env_: napi_env, result_: *mut napi_value) -> napi_status { bun_output::scoped_log!(napi, "napi_create_array"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); let arr = match JSValue::create_empty_array(env.to_js(), 0) { @@ -570,7 +609,7 @@ extern "C" fn napi_create_array_with_length( result_: *mut napi_value, ) -> napi_status { bun_output::scoped_log!(napi, "napi_create_array_with_length"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); @@ -605,7 +644,7 @@ extern "C" fn napi_create_int32( result_: *mut napi_value, ) -> napi_status { bun_output::scoped_log!(napi, "napi_create_int32"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); result.set(env, JSValue::js_number(value as f64)); @@ -619,7 +658,7 @@ extern "C" fn napi_create_uint32( result_: *mut napi_value, ) -> napi_status { bun_output::scoped_log!(napi, "napi_create_uint32"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); result.set(env, JSValue::js_number(value as f64)); @@ -633,7 +672,7 @@ extern "C" fn napi_create_int64( result_: *mut napi_value, ) -> napi_status { bun_output::scoped_log!(napi, "napi_create_int64"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); result.set(env, JSValue::js_number(value as f64)); @@ -647,7 +686,7 @@ extern "C" fn napi_create_string_latin1( length: usize, result_: *mut napi_value, ) -> napi_status { - let env = get_env!(env_); + ungated!(env, env_); let result = get_out!(env, result_); let slice: &[u8] = 'brk: { @@ -703,7 +742,7 @@ extern "C" fn napi_create_string_utf8( length: usize, result_: *mut napi_value, ) -> napi_status { - let env = get_env!(env_); + ungated!(env, env_); let result = get_out!(env, result_); let slice: &[u8] = 'brk: { @@ -744,7 +783,7 @@ extern "C" fn napi_create_string_utf16( length: usize, result_: *mut napi_value, ) -> napi_status { - let env = get_env!(env_); + ungated!(env, env_); let result = get_out!(env, result_); let slice: &[u16] = 'brk: { @@ -969,7 +1008,7 @@ unsafe extern "C" { #[unsafe(no_mangle)] extern "C" fn napi_is_array(env_: napi_env, value_: napi_value, result_: *mut bool) -> napi_status { bun_output::scoped_log!(napi, "napi_is_array"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); let value = value_.get(); @@ -1132,7 +1171,7 @@ extern "C" fn napi_open_handle_scope( result_: *mut napi_handle_scope, ) -> napi_status { bun_output::scoped_log!(napi, "napi_open_handle_scope"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); *result = NapiHandleScope::open(env, false); @@ -1145,7 +1184,7 @@ extern "C" fn napi_close_handle_scope( handle_scope: napi_handle_scope, ) -> napi_status { bun_output::scoped_log!(napi, "napi_close_handle_scope"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); if !handle_scope.is_null() { NapiHandleScope::close(handle_scope, env); @@ -1235,7 +1274,7 @@ extern "C" fn napi_open_escapable_handle_scope( result_: *mut napi_escapable_handle_scope, ) -> napi_status { bun_output::scoped_log!(napi, "napi_open_escapable_handle_scope"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); *result = NapiHandleScope::open(env, true); @@ -1248,7 +1287,7 @@ extern "C" fn napi_close_escapable_handle_scope( scope: napi_escapable_handle_scope, ) -> napi_status { bun_output::scoped_log!(napi, "napi_close_escapable_handle_scope"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); if !scope.is_null() { NapiHandleScope::close(scope, env); @@ -1264,7 +1303,7 @@ extern "C" fn napi_escape_handle( result_: *mut napi_value, ) -> napi_status { bun_output::scoped_log!(napi, "napi_escape_handle"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); // SAFETY: scope_ is a raw NAPI handle; non-null is treated as &NapiHandleScope. @@ -1332,7 +1371,7 @@ unsafe extern "C" { #[unsafe(no_mangle)] extern "C" fn napi_is_error(env_: napi_env, value_: napi_value, result_: *mut bool) -> napi_status { bun_output::scoped_log!(napi, "napi_is_error"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let value = value_.get(); if value.is_empty() { @@ -1358,7 +1397,7 @@ extern "C" fn napi_is_arraybuffer( result_: *mut bool, ) -> napi_status { bun_output::scoped_log!(napi, "napi_is_arraybuffer"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let result = get_out!(env, result_); let value = value_.get(); @@ -1403,7 +1442,7 @@ extern "C" fn napi_get_arraybuffer_info( byte_length: *mut usize, ) -> napi_status { bun_output::scoped_log!(napi, "napi_get_arraybuffer_info"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let arraybuffer = arraybuffer_.get(); let Some(array_buffer) = arraybuffer.as_array_buffer(env.to_js()) else { @@ -1437,7 +1476,7 @@ extern "C" fn napi_get_typedarray_info( maybe_byte_offset: *mut usize, ) -> napi_status { bun_output::scoped_log!(napi, "napi_get_typedarray_info"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let typedarray = typedarray_.get(); if typedarray.is_empty_or_undefined_or_null() { @@ -1498,7 +1537,7 @@ extern "C" fn napi_is_dataview( result_: *mut bool, ) -> napi_status { bun_output::scoped_log!(napi, "napi_is_dataview"); - let env = get_env!(env_); + ungated!(env, env_); let result = get_out!(env, result_); let value = value_.get(); if value.is_empty() { @@ -1519,7 +1558,7 @@ extern "C" fn napi_get_dataview_info( maybe_byte_offset: *mut usize, ) -> napi_status { bun_output::scoped_log!(napi, "napi_get_dataview_info"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let dataview = dataview_.get(); if dataview.is_empty() { @@ -1616,7 +1655,7 @@ extern "C" fn napi_is_promise( is_promise_: *mut bool, ) -> napi_status { bun_output::scoped_log!(napi, "napi_is_promise"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let value = value_.get(); let is_promise = get_out!(env, is_promise_); @@ -1657,7 +1696,7 @@ extern "C" fn napi_create_date(env_: napi_env, time: f64, result_: *mut napi_val #[unsafe(no_mangle)] extern "C" fn napi_is_date(env_: napi_env, value_: napi_value, is_date_: *mut bool) -> napi_status { bun_output::scoped_log!(napi, "napi_is_date"); - let env = get_env!(env_); + ungated!(env, env_); env.check_gc(); let is_date = get_out!(env, is_date_); let value = value_.get(); @@ -2061,7 +2100,7 @@ extern "C" fn napi_get_buffer_info( length: *mut usize, ) -> napi_status { bun_output::scoped_log!(napi, "napi_get_buffer_info"); - let env = get_env!(env_); + ungated!(env, env_); let value = value_.get(); // Keep the pointer valid for the buffer's lifetime, as in Node. if !data.is_null() && !value.materialize_array_buffer_view_buffer() { diff --git a/test/napi/napi-app/module.js b/test/napi/napi-app/module.js index 166dd881168e..03bbf0892c7d 100644 --- a/test/napi/napi-app/module.js +++ b/test/napi/napi-app/module.js @@ -1452,4 +1452,46 @@ nativeTests.test_threadsafe_function_microtask_order = async () => { } }; +// A script that only ever calls the ungated napi functions (ungated-calls- +// spin-worker.js has the worker version) still has to be stoppable when the +// stop is requested while one of those calls is running. Several rounds, since +// whether it lands inside a call or in the loop itself is down to timing. +nativeTests.test_ungated_calls_vm_timeout = () => { + const vm = require("node:vm"); + const spin = nativeTests.make_ungated_calls_spinner(); + for (let i = 0; i < 5; i++) { + try { + vm.runInNewContext("for (;;) spin(bigint, string);", { spin, bigint: -7n, string: "ungated" }, { timeout: 20 }); + console.log("returned"); + } catch (e) { + console.log(e.code); + } + } +}; + +nativeTests.test_ungated_calls_worker_terminate = async () => { + const { Worker } = require("node:worker_threads"); + const path = require("node:path"); + for (let i = 0; i < 2; i++) { + const worker = new Worker(path.join(__dirname, "ungated-calls-spin-worker.js")); + await new Promise((resolve, reject) => { + worker.once("message", resolve); + worker.once("error", reject); + }); + console.log("terminate() resolved with", await worker.terminate()); + } +}; + +// See ungated_calls_through_timeout in standalone_tests.cpp: 200ms of ungated +// calls under a 20ms timeout. +nativeTests.test_ungated_calls_through_vm_timeout = () => { + const vm = require("node:vm"); + try { + vm.runInNewContext("f(200)", { f: nativeTests.ungated_calls_through_timeout }, { timeout: 20 }); + console.log("returned"); + } catch (e) { + console.log(e.code); + } +}; + module.exports = nativeTests; diff --git a/test/napi/napi-app/standalone_tests.cpp b/test/napi/napi-app/standalone_tests.cpp index d6ecd232fce4..24e2a6b634ee 100644 --- a/test/napi/napi-app/standalone_tests.cpp +++ b/test/napi/napi-app/standalone_tests.cpp @@ -2920,6 +2920,209 @@ static napi_value test_pending_exception_gate(const Napi::CallbackInfo &info) { return ok(env); } +// The ungated functions (see test_pending_exception_gate) whose bodies contain +// an exception check: a bigint, a string and a symbol round trip, then an +// array, a string and a typeof/is_array check (implemented separately in +// Bun). Statuses are ignored on purpose. +static void ungated_calls_round(napi_env env, napi_value bigint, + napi_value string) { + int64_t i64 = 0; + uint64_t u64 = 0; + bool lossless = false; + napi_get_value_bigint_int64(env, bigint, &i64, &lossless); + napi_get_value_bigint_uint64(env, bigint, &u64, &lossless); + + char utf8[16]; + char16_t utf16[16]; + size_t written = 0; + napi_get_value_string_utf8(env, string, utf8, sizeof utf8, &written); + napi_get_value_string_utf16(env, string, utf16, 16, &written); + + napi_value out = nullptr; + napi_create_bigint_int64(env, i64, &out); + napi_create_bigint_uint64(env, u64, &out); + napi_create_symbol(env, string, &out); + + bool is = false; + napi_create_array_with_length(env, 4, &out); + napi_is_array(env, out, &is); + napi_create_string_utf8(env, utf8, written, &out); + napi_create_int32(env, (int32_t)written, &out); + napi_get_boolean(env, is, &out); +} + +// spin(bigint, string): a callback that does nothing but ungated calls, for a +// script or worker to loop on while it gets terminated (node:vm `timeout`, +// worker.terminate()). Several rounds per call, so the request nearly always +// lands while one of these calls is running; the loop must still stop once +// control is back in JS. A plain napi_callback rather than a Napi::Function so +// that only the calls under test are made. +static napi_value ungated_calls_spin(napi_env env, napi_callback_info info) { + size_t argc = 2; + napi_value argv[2]; + if (napi_get_cb_info(env, info, &argc, argv, nullptr, nullptr) != napi_ok) { + return nullptr; + } + for (int i = 0; i < 32; i++) { + ungated_calls_round(env, argv[0], argv[1]); + } + return nullptr; +} + +static napi_value make_ungated_calls_spinner(const Napi::CallbackInfo &info) { + napi_env env = info.Env(); + napi_value spin; + NODE_API_CALL(env, napi_create_function(env, "spin", NAPI_AUTO_LENGTH, + ungated_calls_spin, nullptr, &spin)); + return spin; +} + +// Leaves an exception pending on the engine itself, not just recorded by +// napi_throw_error: Bun's napi_call_function raises the recorded exception +// before refusing the call. Returns false if the setup failed (an error has +// been thrown in that case). +static bool arm_engine_exception(napi_env env) { + napi_value global, noop; + NODE_API_CALL_CUSTOM_RETURN(env, false, napi_get_global(env, &global)); + NODE_API_CALL_CUSTOM_RETURN( + env, false, + napi_create_function( + env, "noop", NAPI_AUTO_LENGTH, + [](napi_env, napi_callback_info) -> napi_value { return nullptr; }, + nullptr, &noop)); + NODE_API_CALL_CUSTOM_RETURN( + env, false, napi_throw_error(env, "EPENDING", "still pending")); + printf("napi_call_function: status=%d\n", + (int)napi_call_function(env, global, noop, 0, nullptr, nullptr)); + return true; +} + +// The ungated calls while an engine exception is pending must succeed +// (test_pending_exception_gate checks the recorded case) and that exception +// must still be the one pending afterwards. +static napi_value +test_ungated_calls_with_engine_exception(const Napi::CallbackInfo &info) { + napi_env env = info.Env(); + +#ifndef _WIN32 + BlockingStdoutScope stdout_scope; +#endif + + napi_value bigint, string; + NODE_API_CALL(env, napi_create_bigint_int64(env, -7, &bigint)); + NODE_API_CALL( + env, napi_create_string_utf8(env, "ungated", NAPI_AUTO_LENGTH, &string)); + if (!arm_engine_exception(env)) { + return nullptr; + } + + int64_t i64 = 0; + uint64_t u64 = 0; + bool lossless = false; + char utf8[16] = {0}; + size_t written = 0; + napi_value out; + napi_status st; + st = napi_get_value_bigint_int64(env, bigint, &i64, &lossless); + printf("napi_get_value_bigint_int64: status=%d value=%" PRId64 "\n", (int)st, + i64); + st = napi_get_value_bigint_uint64(env, bigint, &u64, &lossless); + printf("napi_get_value_bigint_uint64: status=%d lossless=%d\n", (int)st, + (int)lossless); + st = napi_get_value_string_utf8(env, string, utf8, sizeof utf8, &written); + printf("napi_get_value_string_utf8: status=%d value=%s\n", (int)st, utf8); + st = napi_create_bigint_int64(env, i64, &out); + printf("napi_create_bigint_int64: status=%d\n", (int)st); + st = napi_create_bigint_uint64(env, u64, &out); + printf("napi_create_bigint_uint64: status=%d\n", (int)st); + st = napi_create_symbol(env, string, &out); + printf("napi_create_symbol: status=%d\n", (int)st); + st = napi_create_array_with_length(env, 4, &out); + printf("napi_create_array_with_length: status=%d\n", (int)st); + bool is_array = false; + st = napi_is_array(env, out, &is_array); + printf("napi_is_array: status=%d is_array=%d\n", (int)st, (int)is_array); + st = napi_create_string_utf8(env, utf8, NAPI_AUTO_LENGTH, &out); + printf("napi_create_string_utf8: status=%d\n", (int)st); + st = napi_create_int32(env, 7, &out); + printf("napi_create_int32: status=%d\n", (int)st); + + bool pending = false; + napi_is_exception_pending(env, &pending); + printf("exception pending after: %s\n", pending ? "true" : "false"); + + napi_value exception, code; + NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &exception)); + NODE_API_CALL(env, napi_get_named_property(env, exception, "code", &code)); + char code_buf[32] = {0}; + NODE_API_CALL(env, napi_get_value_string_utf8(env, code, code_buf, + sizeof code_buf, nullptr)); + printf("pending exception code: %s\n", code_buf); + fflush(stdout); + + return ok(env); +} + +// ungated_calls_through_timeout(ms): run from a node:vm script whose `timeout` +// is much shorter than `ms`. With an engine exception pending, loops through +// the ungated calls for `ms`, so the timeout is requested while they run. None +// of them may report it, the exception they found pending must still be the +// (clearable) one pending afterwards, and the timeout must still stop the +// script once this returns. +static napi_value +ungated_calls_through_timeout(const Napi::CallbackInfo &info) { + napi_env env = info.Env(); + const auto duration = + std::chrono::milliseconds(info[0].As().Int64Value()); + +#ifndef _WIN32 + BlockingStdoutScope stdout_scope; +#endif + + napi_value bigint, string; + NODE_API_CALL(env, napi_create_bigint_int64(env, -7, &bigint)); + NODE_API_CALL( + env, napi_create_string_utf8(env, "ungated", NAPI_AUTO_LENGTH, &string)); + if (!arm_engine_exception(env)) { + return nullptr; + } + + const auto deadline = std::chrono::steady_clock::now() + duration; + unsigned failures = 0; + do { + int64_t i64; + uint64_t u64; + bool lossless; + char utf8[16]; + napi_value out; + failures += + napi_get_value_bigint_int64(env, bigint, &i64, &lossless) != napi_ok; + failures += + napi_get_value_bigint_uint64(env, bigint, &u64, &lossless) != napi_ok; + failures += napi_get_value_string_utf8(env, string, utf8, sizeof utf8, + nullptr) != napi_ok; + failures += napi_create_bigint_int64(env, i64, &out) != napi_ok; + failures += napi_create_bigint_uint64(env, u64, &out) != napi_ok; + failures += napi_create_symbol(env, string, &out) != napi_ok; + failures += napi_create_array_with_length(env, 4, &out) != napi_ok; + failures += napi_is_array(env, out, &lossless) != napi_ok; + failures += + napi_create_string_utf8(env, utf8, NAPI_AUTO_LENGTH, &out) != napi_ok; + failures += napi_create_int32(env, 7, &out) != napi_ok; + } while (std::chrono::steady_clock::now() < deadline); + printf("ungated call failures: %u\n", failures); + + bool before = false, after = false; + napi_value exception; + napi_is_exception_pending(env, &before); + napi_get_and_clear_last_exception(env, &exception); + napi_is_exception_pending(env, &after); + printf("exception pending: before clear=%s after clear=%s\n", + before ? "true" : "false", after ? "true" : "false"); + fflush(stdout); + return nullptr; +} + // Regression test: PROPERTY_NAME_FROM_UTF8 must copy string data. // Previously it used StringImpl::createWithoutCopying for ASCII strings, // which could leave dangling pointers in JSC's atom string table. @@ -4197,6 +4400,9 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, test_external_buffer_with_pending_exception); REGISTER_FUNCTION(env, exports, test_pending_exception_gate); + REGISTER_FUNCTION(env, exports, make_ungated_calls_spinner); + REGISTER_FUNCTION(env, exports, test_ungated_calls_with_engine_exception); + REGISTER_FUNCTION(env, exports, ungated_calls_through_timeout); REGISTER_FUNCTION(env, exports, test_napi_get_named_property_copied_string); REGISTER_FUNCTION(env, exports, test_issue_25933); REGISTER_FUNCTION(env, exports, test_napi_make_callback_status); diff --git a/test/napi/napi-app/ungated-calls-spin-worker.js b/test/napi/napi-app/ungated-calls-spin-worker.js new file mode 100644 index 000000000000..9f9934c48820 --- /dev/null +++ b/test/napi/napi-app/ungated-calls-spin-worker.js @@ -0,0 +1,8 @@ +// Loops on the ungated napi calls until the parent terminates this worker +// (test_ungated_calls_worker_terminate in module.js). +const { parentPort } = require("node:worker_threads"); +const nativeTests = require("./build/Debug/napitests.node"); + +const spin = nativeTests.make_ungated_calls_spinner(); +parentPort.postMessage("spinning"); +for (;;) spin(-7n, "ungated"); diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 1e044ea4a4bf..7846b80ebeec 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -451,6 +451,58 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { expect(result).toContain("side_effect arr[7]=undefined"); expect(result).toContain("side_effect script_ran=false"); }); + + // Same ungated functions, but with the exception pending on the engine + // (napi_call_function raises the napi_throw_error one before refusing), + // which is the state node-addon-api builds its Error object in. They must + // still succeed and must leave that exception pending. + it("ungated functions succeed while an engine exception is pending and preserve it", async () => { + const result = await checkSameOutput("test_ungated_calls_with_engine_exception", []); + // printf() via the Windows CRT emits \r\n, so split on either ending. + expect(result.split(/\r?\n/)).toEqual([ + "napi_call_function: status=10", + "napi_get_value_bigint_int64: status=0 value=-7", + "napi_get_value_bigint_uint64: status=0 lossless=0", + "napi_get_value_string_utf8: status=0 value=ungated", + "napi_create_bigint_int64: status=0", + "napi_create_bigint_uint64: status=0", + "napi_create_symbol: status=0", + "napi_create_array_with_length: status=0", + "napi_is_array: status=0 is_array=1", + "napi_create_string_utf8: status=0", + "napi_create_int32: status=0", + "exception pending after: true", + "pending exception code: EPENDING", + ]); + }); + + // A node:vm timeout requested while the addon is inside those calls, again + // with an engine exception pending: none of the calls reports it, the + // exception they found is still the one pending when they are done, and the + // timeout still stops the script once the addon returns. + it("a termination requested during ungated calls is delivered after them, not by them", async () => { + const result = await checkSameOutput("test_ungated_calls_through_vm_timeout", []); + expect(result.split(/\r?\n/)).toEqual([ + "napi_call_function: status=10", + "ungated call failures: 0", + "exception pending: before clear=true after clear=false", + "ERR_SCRIPT_EXECUTION_TIMEOUT", + ]); + }); + + // A script / worker looping through ungated calls, stopped while inside one + // of them nearly every time. Hangs when the request is lost. + it("a node:vm timeout interrupts a script looping through ungated functions", async () => { + const result = await checkSameOutput("test_ungated_calls_vm_timeout", []); + expect(result.split(/\r?\n/)).toEqual(Array(5).fill("ERR_SCRIPT_EXECUTION_TIMEOUT")); + }); + + // Worker startup dominates this one: about two seconds per worker under a + // debug build, before any CI load. + it("worker.terminate() stops a worker looping through ungated functions", async () => { + const result = await checkSameOutput("test_ungated_calls_worker_terminate", []); + expect(result.split(/\r?\n/)).toEqual([...Array(2).fill("terminate() resolved with 1"), "resolved to undefined"]); + }, 30_000); }); describe("status code alignment with Node.js", () => {