Skip to content
Closed
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
36 changes: 22 additions & 14 deletions src/jsc/array_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -923,15 +923,13 @@
}

pub fn from_string(str: &[u8]) -> Result<MarkedArrayBuffer, bun_alloc::AllocError> {
// allocator.dupe(u8, str) → Box::<[u8]>::from(str), but we need a raw
// pointer because the buffer is later freed via the default allocator
// (`MarkedArrayBuffer_deallocator` → `default_alloc::free`).
let buf: Box<[u8]> = Box::from(str);
let len = buf.len();
let ptr = bun_core::heap::into_raw(buf).cast::<u8>();
// SAFETY: ptr/len from heap::alloc; backed by the global allocator.
let bytes = unsafe { bun_core::ffi::slice_mut(ptr, len) };
Ok(MarkedArrayBuffer::from_bytes(bytes, JSType::Uint8Array))
// allocator.dupe(u8, str) → Box::<[u8]>::from(str); the buffer is later
// freed via the default allocator (`MarkedArrayBuffer_deallocator` →
// `default_alloc::free`).
Ok(MarkedArrayBuffer::from_owned_bytes(
Box::from(str),
JSType::Uint8Array,
))
}

pub fn from_js(global: &JSGlobalObject, value: JSValue) -> Option<MarkedArrayBuffer> {
Expand All @@ -952,9 +950,13 @@
})
}

pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer {
/// Take ownership of a default-allocator `Box<[u8]>` and wrap it as an
/// owning `MarkedArrayBuffer`. The buffer is freed exactly once: either by
/// [`MarkedArrayBuffer::destroy`] or by the deallocator installed when the
/// buffer is handed to JSC (`to_js` / `to_node_buffer`).
pub fn from_owned_bytes(bytes: Box<[u8]>, typed_array_type: JSType) -> MarkedArrayBuffer {
MarkedArrayBuffer {
buffer: ArrayBuffer::from_bytes(bytes, typed_array_type),
buffer: ArrayBuffer::from_owned_bytes(bytes, typed_array_type),
owns_buffer: true,
pinned: false,
}
Expand All @@ -977,13 +979,19 @@
}

/// Releases the owned byte buffer if this `MarkedArrayBuffer` was created with an
/// allocator (e.g. via `from_string`/`from_bytes`). Does not free the struct itself;
/// allocator (e.g. via `from_string`/`from_owned_bytes`). Does not free the struct itself;
/// `MarkedArrayBuffer` is passed and stored by value, so callers own its storage.
pub fn destroy(&mut self) {
if self.owns_buffer {
self.owns_buffer = false;
// SAFETY: buffer.ptr was allocated by the global allocator (heap::alloc / allocator.dupe).
unsafe { bun_alloc::default_alloc::free(self.buffer.ptr.cast()) };
// An empty `Box<[u8]>` has no backing allocation — `from_owned_bytes`
// stored its dangling pointer, so there is nothing to free (same
// reasoning as `JSValue::create_buffer_from_box`).
if self.buffer.byte_len != 0 {
// SAFETY: `owns_buffer` is only set by `from_owned_bytes`, which
// took the pointer from a non-empty default-allocator `Box<[u8]>`.
unsafe { bun_alloc::default_alloc::free(self.buffer.ptr.cast()) };

Check warning on line 993 in src/jsc/array_buffer.rs

View check run for this annotation

Claude / Claude Code Review

SAFETY comment overstates owns_buffer invariant

The SAFETY comment claims "`owns_buffer` is only set by `from_owned_bytes`", but `owns_buffer` is `pub` and `src/runtime/api/bun/subprocess/Readable.rs:313-317` constructs a `MarkedArrayBuffer { …, owns_buffer: true, … }` struct literal directly. The underlying invariant still holds there (it goes through `ArrayBuffer::from_owned_bytes`), so this is not a soundness issue — but since this PR's stated goal is to route every caller through the typed constructor, it'd be cleanest to migrate that sit
Comment thread
claude[bot] marked this conversation as resolved.
}
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/runtime/api/bun/Terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1796,11 +1796,11 @@ impl Terminal {
return true;
}
v.extend_from_slice(chunk);
// MarkedArrayBuffer::from_bytes takes a `&mut [u8]` it will own (freed
// via mimalloc on the C++ side) — leak the Box and hand over the slice.
let bytes: &'static mut [u8] = Box::leak(v.into_boxed_slice());
let data = MarkedArrayBuffer::from_bytes(bytes, jsc::JSType::Uint8Array)
.to_node_buffer(global_this);
// Ownership of the boxed slice transfers to JSC (freed via the
// deallocator installed by `to_node_buffer`).
let data =
MarkedArrayBuffer::from_owned_bytes(v.into_boxed_slice(), jsc::JSType::Uint8Array)
.to_node_buffer(global_this);

global_this.bun_vm().event_loop_mut().run_callback(
callback,
Expand Down
16 changes: 8 additions & 8 deletions src/runtime/api/bun/subprocess/SubprocessPipeReader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,14 @@ impl PipeReader {
match &mut self.state {
State::Done(bytes) => {
let bytes = core::mem::take(bytes);
// `state.done` is now empty via `take()`.
// `MarkedArrayBuffer::from_bytes` takes a borrowed `&mut [u8]`
// with `owns_buffer = true` (freed via mimalloc on the JS side); leak the
// boxed slice so JS becomes the owner — same pattern as
// `MarkedArrayBuffer::from_string`.
let slice: &'static mut [u8] = Box::leak(bytes.into_boxed_slice());
MarkedArrayBuffer::from_bytes(slice, jsc::JSType::Uint8Array)
.to_node_buffer(global_this)
// `state.done` is now empty via `take()`. Ownership of the
// boxed slice transfers to JSC (freed via the deallocator
// installed by `to_node_buffer`).
MarkedArrayBuffer::from_owned_bytes(
bytes.into_boxed_slice(),
jsc::JSType::Uint8Array,
)
.to_node_buffer(global_this)
}
_ => JSValue::UNDEFINED,
}
Expand Down
32 changes: 11 additions & 21 deletions src/runtime/node/node_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7069,16 +7069,10 @@ impl NodeFS {
if let Some(file) = unsafe { &mut *graph }.find(path.as_bytes()) {
let contents: &[u8] = file.contents.as_bytes();
return if args.encoding == Encoding::Buffer {
// PORTING.md §Forbidden bans `Vec::leak()`; round-trip through
// `into_boxed_slice()` so the allocation layout JSC frees with
// matches what we hand it (capacity == len).
let raw =
bun_core::heap::into_raw(contents.to_vec().into_boxed_slice());
// SAFETY: ownership of the allocation is transferred to JSC; the
// ArrayBuffer finalizer reconstructs the Box and frees it
// (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI).
Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes(
unsafe { &mut *raw },
// Ownership of the boxed slice transfers to JSC; the
// ArrayBuffer finalizer frees it.
Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes(
contents.to_vec().into_boxed_slice(),
bun_jsc::JSType::Uint8Array,
)))
} else if string_type == ReadFileStringType::Default {
Expand Down Expand Up @@ -7209,15 +7203,12 @@ impl NodeFS {
};
}
}
let raw = bun_core::heap::into_raw(
// Ownership of the boxed slice transfers to JSC; the
// ArrayBuffer finalizer frees it.
Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes(
temporary_read_buffer_before_stat_call
.to_vec()
.into_boxed_slice(),
);
// SAFETY: ownership transferred to JSC; freed via ArrayBuffer finalizer
// (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI).
Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes(
unsafe { &mut *raw },
bun_jsc::JSType::Uint8Array,
)))
}
Expand Down Expand Up @@ -7377,11 +7368,10 @@ impl NodeFS {
match args.encoding {
Encoding::Buffer => {
buf.truncate(final_len);
let raw = bun_core::heap::into_raw(buf.into_boxed_slice());
// SAFETY: ownership transferred to JSC; freed via ArrayBuffer finalizer
// (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI).
Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes(
unsafe { &mut *raw },
// Ownership of the boxed slice transfers to JSC; the
// ArrayBuffer finalizer frees it.
Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes(
buf.into_boxed_slice(),
bun_jsc::JSType::Uint8Array,
)))
}
Expand Down
9 changes: 3 additions & 6 deletions src/runtime/shell/subproc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2305,12 +2305,9 @@ impl PipeReader {
pub fn to_buffer(&mut self, global_this: &JSGlobalObject) -> JSValue {
match &mut self.state {
PipeReaderState::Done(bytes) => {
// `MarkedArrayBuffer::from_bytes` adopts the allocation (freed
// by the JSC ArrayBuffer destructor). `heap::release` names that
// FFI hand-off — it is `Box::leak` under the hood; the JSC
// ArrayBuffer destructor is the reclaim, not this scope.
let slice: &'static mut [u8] = bun_core::heap::release(core::mem::take(bytes));
MarkedArrayBuffer::from_bytes(slice, jsc::JSType::Uint8Array)
// Ownership of the boxed slice transfers to JSC (freed via the
// deallocator installed by `to_node_buffer`).
MarkedArrayBuffer::from_owned_bytes(core::mem::take(bytes), jsc::JSType::Uint8Array)
.to_node_buffer(global_this)
}
_ => JSValue::UNDEFINED,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/target
Loading
Loading