Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 11 additions & 0 deletions src/jsc/JSValue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn materialize_array_buffer_view_buffer(self) -> bool {
Bun__JSValue__materializeArrayBufferViewBuffer(self)
}

// ── Formatting. ────────────────────────────────────
#[inline]
pub fn fmt_string(self, global: &JSGlobalObject) -> StringFormatter<'_> {
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<JSArrayBufferView>(value.asCell()))
return view->possiblySharedBuffer() != nullptr;
return false;
}

CPP_DECL size_t Bun__JSValue__getArrayBufferViewByteOffset(JSC::EncodedJSValue encoded)
{
JSC::JSValue value = JSValue::decode(encoded);
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/napi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
Outdated
uint8Array->possiblySharedBuffer();
// Node.js' code looks like this:
// *data = node::Buffer::Data(buffer);
// That means they unconditionally update the data pointer.
Expand Down
18 changes: 18 additions & 0 deletions src/runtime/napi/napi_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
if !maybe_data.is_null() {
typedarray.materialize_array_buffer_view_buffer();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

let Some(array_buffer) = typedarray.as_array_buffer(env.to_js()) else {
return env.invalid_arg();
};
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
Outdated
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.
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
Outdated
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);
};
Expand Down
106 changes: 106 additions & 0 deletions test/napi/napi-app/standalone_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint32_t>(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<uint32_t>(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, &copy_data, &copy));
napi_value copy_arraybuffer = nullptr;
NODE_API_CALL(env, napi_get_typedarray_info(env, copy, nullptr, nullptr,
nullptr, &copy_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, &copy_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.
Expand Down Expand Up @@ -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);
Expand Down
42 changes: 39 additions & 3 deletions test/napi/napi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[]"],
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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");
Comment thread
robobun marked this conversation as resolved.
Outdated
}
});

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",
Expand Down Expand Up @@ -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", []);
Expand All @@ -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);
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

it("does not crash with Reflect.construct when newTarget has no prototype", async () => {
await checkSameOutput("test_reflect_construct_no_prototype_crash", []);
Expand Down
Loading