Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/jsc/bindings/napi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3330,6 +3330,12 @@ extern "C" bool NapiEnv__hasPendingException(napi_env env)
return scope.exception() != nullptr;
}

extern "C" bool NapiEnv__hasVMException(napi_env env)
{
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(env->vm());
return scope.exception() != nullptr;
}

extern "C" uint32_t napi_internal_get_version(napi_env env)
{
return env->napiModule().nm_version;
Expand Down
37 changes: 29 additions & 8 deletions src/runtime/napi/napi_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ 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;
fn NapiEnv__hasVMException(env: *mut NapiEnv) -> bool;
fn NapiEnv__deref(env: *mut NapiEnv);
fn NapiEnv__ref(env: *mut NapiEnv);
fn napi_set_last_error(env: napi_env, status: NapiStatus) -> napi_status;
Expand Down Expand Up @@ -120,6 +121,13 @@ impl NapiEnv {
unsafe { NapiEnv__hasPendingException(self.as_mut_ptr()) }
}

/// Checks only the JSC VM exception slot, not the env's stashed
/// `napi_throw*` exception. See `NAPI_PREAMBLE_NO_PENDING_CHECK`.
Comment thread
robobun marked this conversation as resolved.
pub fn has_vm_exception(&self) -> bool {
// SAFETY: env is non-null; C++ side is read-only here.
unsafe { NapiEnv__hasVMException(self.as_mut_ptr()) }
}

/// Assert that we're not currently performing garbage collection
pub fn check_gc(&self) {
// SAFETY: env is non-null; C++ side is read-only here.
Expand Down Expand Up @@ -435,6 +443,19 @@ macro_rules! preamble {
}};
}

/// Like `preamble!` but only guards against a VM-level exception (mirrors
/// `NAPI_PREAMBLE_NO_PENDING_CHECK`). For entry points whose bodies reach JSC
/// helpers that assert the VM has no exception.
Comment thread
robobun marked this conversation as resolved.
macro_rules! preamble_no_pending_check {
($env:expr) => {{
let env = get_env!($env);
if env.has_vm_exception() {
return env.pending_exception();
}
env
}};
}
Comment thread
robobun marked this conversation as resolved.

macro_rules! get_out {
($env:expr, $ptr:expr) => {
// SAFETY: caller passes raw out pointer; we treat non-null as &mut borrow.
Expand Down Expand Up @@ -623,7 +644,7 @@ pub(super) extern "C" fn napi_create_string_latin1(
length: usize,
result_: *mut napi_value,
) -> napi_status {
let env = get_env!(env_);
let env = preamble_no_pending_check!(env_);
let result = get_out!(env, result_);

let slice: &[u8] = 'brk: {
Expand Down Expand Up @@ -679,7 +700,7 @@ pub(super) extern "C" fn napi_create_string_utf8(
length: usize,
result_: *mut napi_value,
) -> napi_status {
let env = get_env!(env_);
let env = preamble_no_pending_check!(env_);
let result = get_out!(env, result_);

let slice: &[u8] = 'brk: {
Expand Down Expand Up @@ -720,7 +741,7 @@ pub(super) extern "C" fn napi_create_string_utf16(
length: usize,
result_: *mut napi_value,
) -> napi_status {
let env = get_env!(env_);
let env = preamble_no_pending_check!(env_);
let result = get_out!(env, result_);

let slice: &[u16] = 'brk: {
Expand Down Expand Up @@ -1347,7 +1368,7 @@ pub(super) extern "C" fn napi_is_arraybuffer(
result_: *mut bool,
) -> napi_status {
bun_output::scoped_log!(napi, "napi_is_arraybuffer");
let env = get_env!(env_);
let env = preamble_no_pending_check!(env_);
env.check_gc();
let result = get_out!(env, result_);
let value = value_.get();
Expand Down Expand Up @@ -1392,7 +1413,7 @@ pub(super) 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_);
let env = preamble_no_pending_check!(env_);
env.check_gc();
let arraybuffer = arraybuffer_.get();
let Some(array_buffer) = arraybuffer.as_array_buffer(env.to_js()) else {
Expand Down Expand Up @@ -1426,7 +1447,7 @@ pub(super) 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_);
let env = preamble_no_pending_check!(env_);
env.check_gc();
let typedarray = typedarray_.get();
if typedarray.is_empty_or_undefined_or_null() {
Expand Down Expand Up @@ -1502,7 +1523,7 @@ pub(super) 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_);
let env = preamble_no_pending_check!(env_);
env.check_gc();
let dataview = dataview_.get();
if dataview.is_empty() {
Expand Down Expand Up @@ -2066,7 +2087,7 @@ pub(super) 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_);
let env = preamble_no_pending_check!(env_);
let value = value_.get();
let Some(array_buf) = value.as_array_buffer(env.to_js()) else {
return NapiEnv::set_last_error(Some(env), NapiStatus::invalid_arg);
Expand Down
97 changes: 97 additions & 0 deletions test/napi/napi-app/standalone_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2892,6 +2892,102 @@ static napi_value test_pending_exception_gate(const Napi::CallbackInfo &info) {
return ok(env);
}

// napi_throw() followed by napi_call_function() promotes the stashed exception
// into vm.m_exception. With a VM-level exception pending, the string creators
// and buffer-info accessors below reach C++ helpers that assert "no pending
// exception" under debug/assert builds (release builds compile the check out).
// The entry points must refuse with napi_pending_exception before touching
// those helpers, and leave the pending exception intact.
static napi_value test_vm_pending_exception_no_abort(
const Napi::CallbackInfo &info) {
napi_env env = info.Env();

// Build every input before arming the exception.
napi_value global, isNaN, arraybuffer, typedarray, dataview, buffer;
void *ab_data;
void *buf_data;
NODE_API_CALL(env, napi_get_global(env, &global));
NODE_API_CALL(env, napi_get_named_property(env, global, "isNaN", &isNaN));
NODE_API_CALL(env,
napi_create_arraybuffer(env, 16, &ab_data, &arraybuffer));
NODE_API_CALL(env, napi_create_typedarray(env, napi_uint8_array, 16,
arraybuffer, 0, &typedarray));
NODE_API_CALL(env,
napi_create_dataview(env, 16, arraybuffer, 0, &dataview));
NODE_API_CALL(env,
napi_create_buffer_copy(env, 4, "abcd", &buf_data, &buffer));

// Arm: E pending on env, then napi_call_function pushes it onto the VM.
napi_value msg, err;
NODE_API_CALL(env, napi_create_string_utf8(env, "E1", 2, &msg));
NODE_API_CALL(env, napi_create_error(env, nullptr, msg, &err));
NODE_API_CALL(env, napi_throw(env, err));
napi_value r;
napi_status call_st =
napi_call_function(env, global, isNaN, 0, nullptr, &r);
printf("napi_call_function: pending_exception=%s\n",
call_st == napi_pending_exception ? "true" : "false");

napi_value out;
bool bool_out;
void *ptr_out;
size_t len_out;
napi_typedarray_type ta_type;
const char16_t utf16[] = {'x', 0};

napi_create_string_utf8(env, "x", 1, &out);
printf("napi_create_string_utf8: survived\n");
napi_create_string_latin1(env, "x", 1, &out);
printf("napi_create_string_latin1: survived\n");
napi_create_string_utf16(env, utf16, 1, &out);
printf("napi_create_string_utf16: survived\n");
napi_is_arraybuffer(env, arraybuffer, &bool_out);
printf("napi_is_arraybuffer: survived\n");
napi_get_arraybuffer_info(env, arraybuffer, &ptr_out, &len_out);
printf("napi_get_arraybuffer_info: survived\n");
napi_get_typedarray_info(env, typedarray, &ta_type, &len_out, &ptr_out,
&out, &len_out);
printf("napi_get_typedarray_info: survived\n");
napi_get_dataview_info(env, dataview, &len_out, &ptr_out, &out, &len_out);
printf("napi_get_dataview_info: survived\n");
napi_get_buffer_info(env, buffer, &ptr_out, &len_out);
printf("napi_get_buffer_info: survived\n");
napi_create_array(env, &out);
printf("napi_create_array: survived\n");
napi_create_array_with_length(env, 4, &out);
printf("napi_create_array_with_length: survived\n");

// The original exception must still be pending and catchable.
bool pending = false;
napi_is_exception_pending(env, &pending);
napi_value exc;
NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &exc));
napi_value exc_msg;
char exc_buf[16];
size_t exc_len = 0;
NODE_API_CALL(env, napi_get_named_property(env, exc, "message", &exc_msg));
NODE_API_CALL(env, napi_get_value_string_utf8(env, exc_msg, exc_buf,
sizeof(exc_buf), &exc_len));
printf("exception: pending=%s message=%s\n", pending ? "true" : "false",
exc_buf);

// Second route: napi_create_bigint_words past the engine cap throws a
// RangeError directly into the VM; the next string-creator call must also
// survive. Node accepts 1M words so this route is a no-op there.
static uint64_t words[1000001];
napi_value big;
napi_create_bigint_words(env, 0, 1000001, words, &big);
napi_create_string_utf8(env, "y", 1, &out);
printf("bigint_route: survived\n");
pending = false;
napi_is_exception_pending(env, &pending);
if (pending) {
NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &exc));
}

return ok(env);
}

// 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.
Expand Down Expand Up @@ -3562,6 +3658,7 @@ 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, test_vm_pending_exception_no_abort);
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);
Expand Down
21 changes: 21 additions & 0 deletions test/napi/napi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,27 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => {
expect(result).toContain("side_effect arr[7]=undefined");
expect(result).toContain("side_effect script_ran=false");
});

it("does not abort when a VM exception is already pending", async () => {
const result = await checkSameOutput("test_vm_pending_exception_no_abort", []);
expect(result).toContain("napi_call_function: pending_exception=true");
for (const fn of [
"napi_create_string_utf8",
"napi_create_string_latin1",
"napi_create_string_utf16",
"napi_is_arraybuffer",
"napi_get_arraybuffer_info",
"napi_get_typedarray_info",
"napi_get_dataview_info",
"napi_get_buffer_info",
"napi_create_array",
"napi_create_array_with_length",
]) {
expect(result).toContain(`${fn}: survived`);
}
expect(result).toContain("exception: pending=true message=E1");
expect(result).toContain("bigint_route: survived");
});
});

describe("napi_async_work", () => {
Expand Down
Loading