From ae70cef47ac711c1d724a81e75883c8790411be8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:00:25 +0000 Subject: [PATCH 1/4] napi: materialize the backing ArrayBuffer before handing out view data pointers JSC keeps typed arrays under 1000 bytes in fast mode, where the contents live in GC-heap storage. Materializing the backing ArrayBuffer (e.g. via the arraybuffer out-param of napi_get_typedarray_info, or JS touching .buffer) copies the contents into a fresh ArrayBuffer and repoints the view, abandoning the old storage. napi_get_typedarray_info captured vector() before fetching the arraybuffer out-param, so the data pointer it returned pointed at the abandoned storage and native writes through it were silently dropped. napi_get_buffer_info, napi_create_buffer and napi_create_buffer_copy handed out fast-mode pointers that went stale as soon as anything materialized the buffer. Force the view into wasteful mode before capturing the pointer in all four, matching Node, where the pointer stays valid for the lifetime of the object. Fixes #37151 --- src/jsc/JSValue.rs | 11 +++ src/jsc/bindings/bindings.cpp | 10 +++ src/jsc/bindings/napi.cpp | 4 + src/runtime/napi/napi_body.rs | 18 ++++ test/napi/napi-app/standalone_tests.cpp | 106 ++++++++++++++++++++++++ test/napi/napi.test.ts | 42 +++++++++- 6 files changed, 188 insertions(+), 3 deletions(-) diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 1ffefc5fa1bc..1a4de750072b 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -2598,6 +2598,16 @@ impl JSValue { Bun__JSValue__getArrayBufferViewByteOffset(self) } + /// Force a typed array out of JSC's "fast" mode, where small views' + /// contents live in GC-heap storage that is abandoned (copied into a + /// fresh `ArrayBuffer`) the first time the backing buffer is + /// materialized. After this call the view's data pointer is stable for + /// the view's lifetime. Returns `false` if `self` is not a view or the + /// buffer could not be allocated. + pub fn materialize_array_buffer_view_buffer(self) -> bool { + Bun__JSValue__materializeArrayBufferViewBuffer(self) + } + // ── Formatting. ──────────────────────────────────── #[inline] pub fn fmt_string(self, global: &JSGlobalObject) -> StringFormatter<'_> { @@ -2724,6 +2734,7 @@ unsafe extern "C" { global: &JSGlobalObject, ) -> JSValue; safe fn Bun__JSValue__getArrayBufferViewByteOffset(this: JSValue) -> usize; + safe fn Bun__JSValue__materializeArrayBufferViewBuffer(this: JSValue) -> bool; safe fn Bun__Process__queueNextTick1(global: &JSGlobalObject, func: JSValue, arg: JSValue); fn Bun__JSValue__deserialize( global: *const JSGlobalObject, diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 48e9cad873ec..7f60f862eb9c 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -6249,6 +6249,16 @@ CPP_DECL JSC::EncodedJSValue Bun__JSValue__getArrayBufferViewBuffer(JSC::Encoded return JSValue::encode(JSValue()); } +CPP_DECL bool Bun__JSValue__materializeArrayBufferViewBuffer(JSC::EncodedJSValue encoded) +{ + JSC::JSValue value = JSValue::decode(encoded); + if (!value || !value.isCell()) + return false; + if (auto* view = dynamicDowncast(value.asCell())) + return view->possiblySharedBuffer() != nullptr; + return false; +} + CPP_DECL size_t Bun__JSValue__getArrayBufferViewByteOffset(JSC::EncodedJSValue encoded) { JSC::JSValue value = JSValue::decode(encoded); diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index 548f5eb03171..73c97d31153b 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -2278,6 +2278,10 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length, NAPI_RETURN_STATUS_IF_EXCEPTION(env, napi_generic_failure); if (data != nullptr) { + // Node guarantees the pointer stays valid for the buffer's lifetime. + // Small views start in fast mode, whose storage is abandoned when the + // backing ArrayBuffer is materialized, so materialize it first. + uint8Array->possiblySharedBuffer(); // Node.js' code looks like this: // *data = node::Buffer::Data(buffer); // That means they unconditionally update the data pointer. diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index 12cd0b609cce..98555f911373 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -1417,6 +1417,14 @@ extern "C" fn napi_get_typedarray_info( } let _keep = jsc::EnsureStillAlive(typedarray); + // Node guarantees the data pointer stays valid for the view's lifetime. + // A fast-mode view's storage is abandoned when its buffer is materialized + // (which the arraybuffer out-param below would do anyway), so materialize + // it before capturing the pointer. + if !maybe_data.is_null() { + typedarray.materialize_array_buffer_view_buffer(); + } + let Some(array_buffer) = typedarray.as_array_buffer(env.to_js()) else { return env.invalid_arg(); }; @@ -2006,6 +2014,11 @@ extern "C" fn napi_create_buffer_copy( Ok(b) => b, Err(_) => return env.generic_failure(), }; + // Node guarantees `result_data` stays valid for the buffer's lifetime; + // a fast-mode view's storage is abandoned when its buffer is materialized. + if !result_data.is_null() { + buffer.materialize_array_buffer_view_buffer(); + } if let Some(mut array_buf) = buffer.as_array_buffer(env.to_js()) { if length > 0 { // SAFETY: caller guarantees `data` points to at least `length` bytes. @@ -2041,6 +2054,11 @@ extern "C" fn napi_get_buffer_info( bun_output::scoped_log!(napi, "napi_get_buffer_info"); let env = get_env!(env_); let value = value_.get(); + // Node guarantees the data pointer stays valid for the buffer's lifetime; + // a fast-mode view's storage is abandoned when its buffer is materialized. + if !data.is_null() { + value.materialize_array_buffer_view_buffer(); + } let Some(array_buf) = value.as_array_buffer(env.to_js()) else { return NapiEnv::set_last_error(Some(env), NapiStatus::invalid_arg); }; diff --git a/test/napi/napi-app/standalone_tests.cpp b/test/napi/napi-app/standalone_tests.cpp index 81ac5fcd2b44..d6ecd232fce4 100644 --- a/test/napi/napi-app/standalone_tests.cpp +++ b/test/napi/napi-app/standalone_tests.cpp @@ -3214,6 +3214,109 @@ test_dataview_info_byte_offset(const Napi::CallbackInfo &info) { return ok(env); } +// Writes through the data pointer that napi_get_typedarray_info returns (with +// every out-param requested, like napi-rs does) and reads the bytes back +// through the JS view. Small typed arrays use JSC's "fast" GC-heap storage, +// which is abandoned when the backing ArrayBuffer is materialized; the pointer +// must reflect the storage that survives. +static napi_value +test_typedarray_info_write_visibility(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + napi_value typedarray = info[1]; + + napi_typedarray_type type; + size_t length = 0; + void *data = nullptr; + napi_value arraybuffer = nullptr; + size_t byte_offset = SIZE_MAX; + NODE_API_CALL(env, + napi_get_typedarray_info(env, typedarray, &type, &length, &data, + &arraybuffer, &byte_offset)); + if (length > 0) { + memset(data, 0x2a, length); + } + + uint32_t first = 0, last = 0; + napi_value element; + if (length > 0) { + NODE_API_CALL(env, napi_get_element(env, typedarray, 0, &element)); + NODE_API_CALL(env, napi_get_value_uint32(env, element, &first)); + NODE_API_CALL(env, + napi_get_element(env, typedarray, + static_cast(length) - 1, &element)); + NODE_API_CALL(env, napi_get_value_uint32(env, element, &last)); + } + printf("length=%zu first=%u last=%u\n", length, first, last); + return ok(env); +} + +// The pointer from napi_get_buffer_info must stay valid after the view's +// backing ArrayBuffer is materialized (here via the arraybuffer out-param of +// napi_get_typedarray_info; JS touching .buffer does the same). +static napi_value +test_buffer_info_pointer_stability(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + napi_value buffer = info[1]; + + void *data = nullptr; + size_t length = 0; + NODE_API_CALL(env, napi_get_buffer_info(env, buffer, &data, &length)); + + napi_value arraybuffer = nullptr; + NODE_API_CALL(env, napi_get_typedarray_info(env, buffer, nullptr, nullptr, + nullptr, &arraybuffer, nullptr)); + + uint32_t first = 0, last = 0; + napi_value element; + if (length > 0) { + memset(data, 0x2b, length); + NODE_API_CALL(env, napi_get_element(env, buffer, 0, &element)); + NODE_API_CALL(env, napi_get_value_uint32(env, element, &first)); + NODE_API_CALL(env, + napi_get_element(env, buffer, + static_cast(length) - 1, &element)); + NODE_API_CALL(env, napi_get_value_uint32(env, element, &last)); + } + printf("length=%zu first=%u last=%u\n", length, first, last); + return ok(env); +} + +// Pointers returned by napi_create_buffer and napi_create_buffer_copy must +// also stay valid after the backing ArrayBuffer is materialized. +static napi_value +test_create_buffer_pointer_stability(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + napi_value element; + + void *data = nullptr; + napi_value buffer = nullptr; + NODE_API_CALL(env, napi_create_buffer(env, 64, &data, &buffer)); + napi_value arraybuffer = nullptr; + NODE_API_CALL(env, napi_get_typedarray_info(env, buffer, nullptr, nullptr, + nullptr, &arraybuffer, nullptr)); + memset(data, 0x2c, 64); + uint32_t last = 0; + NODE_API_CALL(env, napi_get_element(env, buffer, 63, &element)); + NODE_API_CALL(env, napi_get_value_uint32(env, element, &last)); + printf("create_buffer last=%u\n", last); + + const uint8_t src[16] = {0}; + void *copy_data = nullptr; + napi_value copy = nullptr; + NODE_API_CALL( + env, napi_create_buffer_copy(env, sizeof src, src, ©_data, ©)); + napi_value copy_arraybuffer = nullptr; + NODE_API_CALL(env, napi_get_typedarray_info(env, copy, nullptr, nullptr, + nullptr, ©_arraybuffer, + nullptr)); + memset(copy_data, 0x2d, sizeof src); + uint32_t copy_last = 0; + NODE_API_CALL(env, napi_get_element(env, copy, 15, &element)); + NODE_API_CALL(env, napi_get_value_uint32(env, element, ©_last)); + printf("create_buffer_copy last=%u\n", copy_last); + return ok(env); +} + static napi_value test_napi_float16_array(const Napi::CallbackInfo &info) { Napi::Env env = info.Env(); // napi_float16_array == 11; cast so older headers without the member compile. @@ -4024,6 +4127,9 @@ test_reference_ref_after_collect(const Napi::CallbackInfo &info) { void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, test_typedarray_info_byte_offset); + REGISTER_FUNCTION(env, exports, test_typedarray_info_write_visibility); + REGISTER_FUNCTION(env, exports, test_buffer_info_pointer_stability); + REGISTER_FUNCTION(env, exports, test_create_buffer_pointer_stability); REGISTER_FUNCTION(env, exports, test_dataview_info_byte_offset); REGISTER_FUNCTION(env, exports, test_napi_float16_array); REGISTER_FUNCTION(env, exports, test_create_arraybuffer_zeroed); diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 688765c651c1..5bca824b7a22 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -616,6 +616,8 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { // leaves one behind leaks. An addon that uses the handle after napi_closing // (a release, say) therefore touches freed memory, in node as well: the docs // say to make no further use of it. Bun-only: reads bun's live tsfn count. + // The spawned run takes >10s under a debug+ASAN bun, so the 5s default + // times out on slow machines. it("frees an orphaned threadsafe function whose last reference a call consumed", async () => { await using proc = spawn({ cmd: [bunExe(), join(__dirname, "napi-app/main.js"), "test_threadsafe_function_orphan_leak", "[]"], @@ -645,7 +647,7 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { exitCode: 0, signalCode: null, }); - }); + }, 30_000); // napi_create_threadsafe_function once the env has torn its threadsafe // functions down (here: from a cleanup hook that a threadsafe function's @@ -897,6 +899,36 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { ); }); + // Small typed arrays use JSC's "fast" mode, whose GC-heap storage is + // abandoned when the backing ArrayBuffer is materialized. The data pointer + // NAPI hands out must point at the storage that survives, or native writes + // are silently dropped (see issue #37151). + it("returns a data pointer whose writes are visible through small typed arrays", async () => { + for (const size of [16, 999, 2048]) { + const output = await checkSameOutput("test_typedarray_info_write_visibility", `[new Uint8Array(${size})]`); + expect(output).toBe(`length=${size} first=42 last=42`); + } + }); + + it("returns a data pointer at the reported byte offset for small typed arrays", async () => { + for (const expr of ["new Uint8Array(0)", "new Uint8Array(64)", "new Int32Array(8)"]) { + const output = await checkSameOutput("test_typedarray_info_byte_offset", `[${expr}]`); + expect(output).toEndWith("data_is_arraybuffer_data_plus_byte_offset=true"); + } + }); + + it("napi_get_buffer_info pointer stays valid after the backing ArrayBuffer is materialized", async () => { + for (const expr of ["Buffer.alloc(32)", "new Uint8Array(32)"]) { + const output = await checkSameOutput("test_buffer_info_pointer_stability", `[${expr}]`); + expect(output).toBe("length=32 first=43 last=43"); + } + }); + + it("napi_create_buffer and napi_create_buffer_copy pointers stay valid after the backing ArrayBuffer is materialized", async () => { + const output = await checkSameOutput("test_create_buffer_pointer_stability", []); + expect(output.split(/\r?\n/)).toEqual(["create_buffer last=44", "create_buffer_copy last=45"]); + }); + it("reports the view's byte offset into its backing buffer", async () => { const output = await checkSameOutput( "test_typedarray_info_byte_offset", @@ -967,6 +999,8 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { await checkSameOutput("test_napi_remove_wrap", []); }); + // 7 sequential node+bun pairs; a debug+ASAN bun takes ~2s each, so the + // 5s default times out on slow machines. it("has the right lifetime", async () => { await checkSameOutput("test_wrap_lifetime_without_ref", []); await checkSameOutput("test_wrap_lifetime_with_weak_ref", []); @@ -977,16 +1011,18 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { await checkSameOutput("test_ref_deleted_in_cleanup", []); // check that calling napi_delete_ref in the ref's finalizer is not use-after-free await checkSameOutput("test_ref_deleted_in_async_finalize", []); - }); + }, 30_000); }); describe("napi_define_class", () => { + // Spawns 4 sequential node+bun pairs; a debug+ASAN bun takes ~2s each, + // so the 5s default times out on slow machines. it("handles edge cases in the constructor", async () => { await checkSameOutput("test_napi_class", []); await checkSameOutput("test_subclass_napi_class", []); await checkSameOutput("test_napi_class_non_constructor_call", []); await checkSameOutput("test_reflect_construct_napi_class", []); - }); + }, 30_000); it("does not crash with Reflect.construct when newTarget has no prototype", async () => { await checkSameOutput("test_reflect_construct_no_prototype_crash", []); From ce3605336d93be0952fd7725f0333f98c6e1283a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 03:57:35 +0000 Subject: [PATCH 2/4] napi: create buffers with an ArrayBuffer backing store directly Instead of creating a fast-mode view and converting it (two allocations plus a copy), napi_create_buffer and napi_create_buffer_copy now create the JSC::ArrayBuffer up front and move it into the view. Throw OOM when the allocation fails, matching Node's pending-exception behavior. The accessor paths keep the one-time materialization for JS-created views and now fail instead of handing out a pointer when it fails. --- src/jsc/JSValue.rs | 10 +++--- src/jsc/bindings/bindings.cpp | 5 +-- src/jsc/bindings/napi.cpp | 53 +++++++++++++++++++++++++--- src/runtime/napi/napi_body.rs | 65 ++++++++--------------------------- test/napi/napi.test.ts | 38 ++++++++++---------- 5 files changed, 88 insertions(+), 83 deletions(-) diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 3fbd5dca4212..ed019c91c606 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -2599,12 +2599,10 @@ impl JSValue { Bun__JSValue__getArrayBufferViewByteOffset(self) } - /// Force a typed array out of JSC's "fast" mode, where small views' - /// contents live in GC-heap storage that is abandoned (copied into a - /// fresh `ArrayBuffer`) the first time the backing buffer is - /// materialized. After this call the view's data pointer is stable for - /// the view's lifetime. Returns `false` if `self` is not a view or the - /// buffer could not be allocated. + /// Force a typed array out of JSC's "fast" mode so its data pointer stays + /// stable for the view's lifetime (fast-mode storage is abandoned when the + /// backing buffer is materialized). Returns `false` only when `self` is a + /// view whose backing buffer could not be allocated; non-views are a no-op. pub fn materialize_array_buffer_view_buffer(self) -> bool { Bun__JSValue__materializeArrayBufferViewBuffer(self) } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index cac8f5b7e639..c084a885ee86 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -6470,10 +6470,11 @@ CPP_DECL bool Bun__JSValue__materializeArrayBufferViewBuffer(JSC::EncodedJSValue { JSC::JSValue value = JSValue::decode(encoded); if (!value || !value.isCell()) - return false; + return true; if (auto* view = dynamicDowncast(value.asCell())) return view->possiblySharedBuffer() != nullptr; - return false; + // Not a view: nothing to materialize; the caller's type checks decide. + return true; } CPP_DECL size_t Bun__JSValue__getArrayBufferViewByteOffset(JSC::EncodedJSValue encoded) diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index fa6bb7176167..a968e39b3ff7 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -2274,15 +2274,23 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length, Zig::GlobalObject* globalObject = toJS(env); auto* subclassStructure = globalObject->JSBufferSubclassStructure(); + // Create the backing ArrayBuffer up front and move it into the view, so the + // view is never in fast mode and the data pointer stays valid for the + // buffer's lifetime, as in Node. // In Node.js, napi_create_buffer is uninitialized memory. - auto* uint8Array = JSC::JSUint8Array::createUninitialized(globalObject, subclassStructure, length); + RefPtr arrayBuffer = ArrayBuffer::tryCreateUninitialized(length, 1); + if (!arrayBuffer) { + // Node leaves a pending exception for a failed allocation. + auto scope = DECLARE_THROW_SCOPE(env->vm()); + JSC::throwOutOfMemoryError(globalObject, scope); + RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_generic_failure)); + return napi_set_last_error(env, napi_generic_failure); + } + + auto* uint8Array = JSC::JSUint8Array::create(globalObject, subclassStructure, WTF::move(arrayBuffer), 0, length); NAPI_RETURN_STATUS_IF_EXCEPTION(env, napi_generic_failure); if (data != nullptr) { - // Node guarantees the pointer stays valid for the buffer's lifetime. - // Small views start in fast mode, whose storage is abandoned when the - // backing ArrayBuffer is materialized, so materialize it first. - uint8Array->possiblySharedBuffer(); // Node.js' code looks like this: // *data = node::Buffer::Data(buffer); // That means they unconditionally update the data pointer. @@ -2293,6 +2301,41 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length, NAPI_RETURN_SUCCESS(env); } +extern "C" napi_status napi_create_buffer_copy(napi_env env, size_t length, + const void* data, + void** result_data, + napi_value* result) +{ + NAPI_PREAMBLE(env); + NAPI_CHECK_ARG(env, result); + + Zig::GlobalObject* globalObject = toJS(env); + auto* subclassStructure = globalObject->JSBufferSubclassStructure(); + + // As above: one allocation, never fast mode, stable data pointer. + RefPtr arrayBuffer = ArrayBuffer::tryCreateUninitialized(length, 1); + if (!arrayBuffer) { + // Node leaves a pending exception for a failed allocation. + auto scope = DECLARE_THROW_SCOPE(env->vm()); + JSC::throwOutOfMemoryError(globalObject, scope); + RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_generic_failure)); + return napi_set_last_error(env, napi_generic_failure); + } + if (length > 0) { + memcpy(arrayBuffer->data(), data, length); + } + + auto* uint8Array = JSC::JSUint8Array::create(globalObject, subclassStructure, WTF::move(arrayBuffer), 0, length); + NAPI_RETURN_STATUS_IF_EXCEPTION(env, napi_generic_failure); + + if (result_data != nullptr) { + *result_data = length > 0 ? uint8Array->typedVector() : nullptr; + } + + *result = toNapi(uint8Array, globalObject); + NAPI_RETURN_SUCCESS(env); +} + // SharedTask subclass with an armed flag so that the destructor can be // armed only after the wrapping JS object (JSUint8Array / JSArrayBuffer) // is successfully created. If creation throws, the destructor runs diff --git a/src/runtime/napi/napi_body.rs b/src/runtime/napi/napi_body.rs index 13b3e130b860..4ad87609e8e7 100644 --- a/src/runtime/napi/napi_body.rs +++ b/src/runtime/napi/napi_body.rs @@ -1445,12 +1445,10 @@ extern "C" fn napi_get_typedarray_info( } let _keep = jsc::EnsureStillAlive(typedarray); - // Node guarantees the data pointer stays valid for the view's lifetime. - // A fast-mode view's storage is abandoned when its buffer is materialized - // (which the arraybuffer out-param below would do anyway), so materialize - // it before capturing the pointer. - if !maybe_data.is_null() { - typedarray.materialize_array_buffer_view_buffer(); + // Keep the pointer valid for the view's lifetime, as in Node: the + // arraybuffer out-param below would otherwise invalidate it. + if !maybe_data.is_null() && !typedarray.materialize_array_buffer_view_buffer() { + return env.generic_failure(); } let Some(array_buffer) = typedarray.as_array_buffer(env.to_js()) else { @@ -2042,47 +2040,13 @@ unsafe extern "C" { finalize_hint: *mut c_void, result: *mut napi_value, ) -> napi_status; -} - -#[unsafe(no_mangle)] -extern "C" fn napi_create_buffer_copy( - env_: napi_env, - length: usize, - data: *const u8, - result_data: *mut *mut c_void, - result_: *mut napi_value, -) -> napi_status { - bun_output::scoped_log!(napi, "napi_create_buffer_copy: {}", length); - let env = preamble!(env_); - let result = get_out!(env, result_); - let buffer: JSValue = match JSValue::create_buffer_from_length(env.to_js(), length) { - Ok(b) => b, - Err(_) => return env.generic_failure(), - }; - // Node guarantees `result_data` stays valid for the buffer's lifetime; - // a fast-mode view's storage is abandoned when its buffer is materialized. - if !result_data.is_null() { - buffer.materialize_array_buffer_view_buffer(); - } - if let Some(mut array_buf) = buffer.as_array_buffer(env.to_js()) { - if length > 0 { - // SAFETY: caller guarantees `data` points to at least `length` bytes. - let src = unsafe { bun_core::ffi::slice(data, length) }; - array_buf.slice_mut()[..length].copy_from_slice(src); - } - write_out( - result_data, - if length > 0 { - array_buf.ptr.cast::() - } else { - ptr::null_mut() - }, - ); - } - - result.set(env, buffer); - - env.ok() + pub(super) fn napi_create_buffer_copy( + env: napi_env, + length: usize, + data: *const c_void, + result_data: *mut *mut c_void, + result: *mut napi_value, + ) -> napi_status; } unsafe extern "C" { @@ -2099,10 +2063,9 @@ extern "C" fn napi_get_buffer_info( bun_output::scoped_log!(napi, "napi_get_buffer_info"); let env = get_env!(env_); let value = value_.get(); - // Node guarantees the data pointer stays valid for the buffer's lifetime; - // a fast-mode view's storage is abandoned when its buffer is materialized. - if !data.is_null() { - value.materialize_array_buffer_view_buffer(); + // Keep the pointer valid for the buffer's lifetime, as in Node. + if !data.is_null() && !value.materialize_array_buffer_view_buffer() { + return env.generic_failure(); } let Some(array_buf) = value.as_array_buffer(env.to_js()) else { return NapiEnv::set_last_error(Some(env), NapiStatus::invalid_arg); diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index a4ab226da330..3c98d5769c34 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -1017,30 +1017,30 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { await checkSameOutput("test_napi_remove_wrap", []); }); - // 7 sequential node+bun pairs; a debug+ASAN bun takes ~2s each, so the - // 5s default times out on slow machines. it("has the right lifetime", async () => { - await checkSameOutput("test_wrap_lifetime_without_ref", []); - await checkSameOutput("test_wrap_lifetime_with_weak_ref", []); - await checkSameOutput("test_wrap_lifetime_with_strong_ref", []); - await checkSameOutput("test_remove_wrap_lifetime_with_weak_ref", []); - await checkSameOutput("test_remove_wrap_lifetime_with_strong_ref", []); - // check that napi finalizers also run at VM exit, even if they didn't get run by GC - await checkSameOutput("test_ref_deleted_in_cleanup", []); - // check that calling napi_delete_ref in the ref's finalizer is not use-after-free - await checkSameOutput("test_ref_deleted_in_async_finalize", []); - }, 30_000); + await Promise.all([ + checkSameOutput("test_wrap_lifetime_without_ref", []), + checkSameOutput("test_wrap_lifetime_with_weak_ref", []), + checkSameOutput("test_wrap_lifetime_with_strong_ref", []), + checkSameOutput("test_remove_wrap_lifetime_with_weak_ref", []), + checkSameOutput("test_remove_wrap_lifetime_with_strong_ref", []), + // napi finalizers also run at VM exit, even if they didn't get run by GC + checkSameOutput("test_ref_deleted_in_cleanup", []), + // calling napi_delete_ref in the ref's finalizer is not use-after-free + checkSameOutput("test_ref_deleted_in_async_finalize", []), + ]); + }); }); describe("napi_define_class", () => { - // Spawns 4 sequential node+bun pairs; a debug+ASAN bun takes ~2s each, - // so the 5s default times out on slow machines. it("handles edge cases in the constructor", async () => { - await checkSameOutput("test_napi_class", []); - await checkSameOutput("test_subclass_napi_class", []); - await checkSameOutput("test_napi_class_non_constructor_call", []); - await checkSameOutput("test_reflect_construct_napi_class", []); - }, 30_000); + await Promise.all([ + checkSameOutput("test_napi_class", []), + checkSameOutput("test_subclass_napi_class", []), + checkSameOutput("test_napi_class_non_constructor_call", []), + checkSameOutput("test_reflect_construct_napi_class", []), + ]); + }); it("does not crash with Reflect.construct when newTarget has no prototype", async () => { await checkSameOutput("test_reflect_construct_no_prototype_crash", []); From c6e8b9e7090c8d3f92565da195b04d4df0154ef5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 04:30:03 +0000 Subject: [PATCH 3/4] napi: use a function-scoped throw scope in buffer creation, parallelize new tests Nesting a ThrowScope inside NAPI_PREAMBLE's TopExceptionScope trips validateExceptionChecks; follow the napi_create_bigint_words shape instead and drop the unreachable trailing return. Run the new tests' independent checkSameOutput calls concurrently. --- src/jsc/bindings/napi.cpp | 24 ++++++++++++------------ test/napi/napi.test.ts | 30 ++++++++++++++++++------------ 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/jsc/bindings/napi.cpp b/src/jsc/bindings/napi.cpp index a968e39b3ff7..e7f5eebd156c 100644 --- a/src/jsc/bindings/napi.cpp +++ b/src/jsc/bindings/napi.cpp @@ -2268,10 +2268,12 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length, void** data, napi_value* result) { - NAPI_PREAMBLE(env); + NAPI_PREAMBLE_NO_THROW_SCOPE(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); - Zig::GlobalObject* globalObject = toJS(env); auto* subclassStructure = globalObject->JSBufferSubclassStructure(); // Create the backing ArrayBuffer up front and move it into the view, so the @@ -2281,14 +2283,12 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length, RefPtr arrayBuffer = ArrayBuffer::tryCreateUninitialized(length, 1); if (!arrayBuffer) { // Node leaves a pending exception for a failed allocation. - auto scope = DECLARE_THROW_SCOPE(env->vm()); JSC::throwOutOfMemoryError(globalObject, scope); RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_generic_failure)); - return napi_set_last_error(env, napi_generic_failure); } auto* uint8Array = JSC::JSUint8Array::create(globalObject, subclassStructure, WTF::move(arrayBuffer), 0, length); - NAPI_RETURN_STATUS_IF_EXCEPTION(env, napi_generic_failure); + RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_generic_failure)); if (data != nullptr) { // Node.js' code looks like this: @@ -2298,7 +2298,7 @@ extern "C" napi_status napi_create_buffer(napi_env env, size_t length, } *result = toNapi(uint8Array, globalObject); - NAPI_RETURN_SUCCESS(env); + return napi_set_last_error(env, napi_ok); } extern "C" napi_status napi_create_buffer_copy(napi_env env, size_t length, @@ -2306,34 +2306,34 @@ extern "C" napi_status napi_create_buffer_copy(napi_env env, size_t length, void** result_data, napi_value* result) { - NAPI_PREAMBLE(env); + NAPI_PREAMBLE_NO_THROW_SCOPE(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); - Zig::GlobalObject* globalObject = toJS(env); auto* subclassStructure = globalObject->JSBufferSubclassStructure(); // As above: one allocation, never fast mode, stable data pointer. RefPtr arrayBuffer = ArrayBuffer::tryCreateUninitialized(length, 1); if (!arrayBuffer) { // Node leaves a pending exception for a failed allocation. - auto scope = DECLARE_THROW_SCOPE(env->vm()); JSC::throwOutOfMemoryError(globalObject, scope); RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_generic_failure)); - return napi_set_last_error(env, napi_generic_failure); } if (length > 0) { memcpy(arrayBuffer->data(), data, length); } auto* uint8Array = JSC::JSUint8Array::create(globalObject, subclassStructure, WTF::move(arrayBuffer), 0, length); - NAPI_RETURN_STATUS_IF_EXCEPTION(env, napi_generic_failure); + RETURN_IF_EXCEPTION(scope, napi_set_last_error(env, napi_generic_failure)); if (result_data != nullptr) { *result_data = length > 0 ? uint8Array->typedVector() : nullptr; } *result = toNapi(uint8Array, globalObject); - NAPI_RETURN_SUCCESS(env); + return napi_set_last_error(env, napi_ok); } // SharedTask subclass with an armed flag so that the destructor can be diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 3c98d5769c34..aed8643434d2 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -922,24 +922,30 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { // NAPI hands out must point at the storage that survives, or native writes // are silently dropped (see issue #37151). it("returns a data pointer whose writes are visible through small typed arrays", async () => { - for (const size of [16, 999, 2048]) { - const output = await checkSameOutput("test_typedarray_info_write_visibility", `[new Uint8Array(${size})]`); - expect(output).toBe(`length=${size} first=42 last=42`); - } + await Promise.all( + [16, 999, 2048].map(async size => { + const output = await checkSameOutput("test_typedarray_info_write_visibility", `[new Uint8Array(${size})]`); + expect(output).toBe(`length=${size} first=42 last=42`); + }), + ); }); it("returns a data pointer at the reported byte offset for small typed arrays", async () => { - for (const expr of ["new Uint8Array(0)", "new Uint8Array(64)", "new Int32Array(8)"]) { - const output = await checkSameOutput("test_typedarray_info_byte_offset", `[${expr}]`); - expect(output).toEndWith("data_is_arraybuffer_data_plus_byte_offset=true"); - } + await Promise.all( + ["new Uint8Array(0)", "new Uint8Array(64)", "new Int32Array(8)"].map(async expr => { + const output = await checkSameOutput("test_typedarray_info_byte_offset", `[${expr}]`); + expect(output).toEndWith("data_is_arraybuffer_data_plus_byte_offset=true"); + }), + ); }); it("napi_get_buffer_info pointer stays valid after the backing ArrayBuffer is materialized", async () => { - for (const expr of ["Buffer.alloc(32)", "new Uint8Array(32)"]) { - const output = await checkSameOutput("test_buffer_info_pointer_stability", `[${expr}]`); - expect(output).toBe("length=32 first=43 last=43"); - } + await Promise.all( + ["Buffer.alloc(32)", "new Uint8Array(32)"].map(async expr => { + const output = await checkSameOutput("test_buffer_info_pointer_stability", `[${expr}]`); + expect(output).toBe("length=32 first=43 last=43"); + }), + ); }); it("napi_create_buffer and napi_create_buffer_copy pointers stay valid after the backing ArrayBuffer is materialized", async () => { From 8e36faf565cd310210b771b5328718ba0389c38d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 04:34:15 +0000 Subject: [PATCH 4/4] test: cover the 1000-byte fast-mode boundary and an offset subarray view --- test/napi/napi.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index aed8643434d2..1e044ea4a4bf 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -922,8 +922,9 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { // NAPI hands out must point at the storage that survives, or native writes // are silently dropped (see issue #37151). it("returns a data pointer whose writes are visible through small typed arrays", async () => { + // 999/1000 straddle JSC's fastSizeLimit; 2048 is always wasteful mode. await Promise.all( - [16, 999, 2048].map(async size => { + [16, 999, 1000, 2048].map(async size => { const output = await checkSameOutput("test_typedarray_info_write_visibility", `[new Uint8Array(${size})]`); expect(output).toBe(`length=${size} first=42 last=42`); }), @@ -932,7 +933,13 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => { it("returns a data pointer at the reported byte offset for small typed arrays", async () => { await Promise.all( - ["new Uint8Array(0)", "new Uint8Array(64)", "new Int32Array(8)"].map(async expr => { + [ + "new Uint8Array(0)", + "new Uint8Array(64)", + "new Int32Array(8)", + // offset view derived from a small array + "new Uint8Array(64).subarray(16)", + ].map(async expr => { const output = await checkSameOutput("test_typedarray_info_byte_offset", `[${expr}]`); expect(output).toEndWith("data_is_arraybuffer_data_plus_byte_offset=true"); }),