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

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,14 @@ impl MarkedArrayBuffer {
})
}

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` — the handoff
/// clears `owns_buffer`, so a later `destroy()` is a no-op).
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,28 +980,44 @@ impl MarkedArrayBuffer {
}

/// 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()) };
Comment thread
claude[bot] marked this conversation as resolved.
}
}
}

pub fn to_node_buffer(&self, global: &JSGlobalObject) -> JSValue {
pub fn to_node_buffer(&mut self, global: &JSGlobalObject) -> JSValue {
// `create_buffer` installs `MarkedArrayBuffer_deallocator` over
// non-empty bytes, making JSC the owner; clear `owns_buffer` so a
// later `destroy()` cannot free the allocation a second time.
self.owns_buffer = false;
// `JSValue::create_buffer` takes `&mut [u8]` (ownership transfers to JSC
// via the deallocator). `ArrayBuffer` is `Copy` over a raw pointer, so
// copy the descriptor and project a mutable slice.
let mut buf = self.buffer;
JSValue::create_buffer(global, buf.byte_slice_mut())
}

pub fn to_js(&self, global: &JSGlobalObject) -> JsResult<JSValue> {
pub fn to_js(&mut self, global: &JSGlobalObject) -> JsResult<JSValue> {
if !self.buffer.value.is_empty_or_undefined_or_null() {
return Ok(self.buffer.value);
}
// Both paths below hand the bytes to JSC (the non-empty one installs
// `MarkedArrayBuffer_deallocator`, making JSC the owner). Clear
// `owns_buffer` before the fallible FFI call so a later `destroy()`
// cannot double-free: if the call throws, the bytes may leak, which
// is the safe direction.
self.owns_buffer = false;
if self.buffer.byte_len == 0 {
return make_typed_array_with_bytes_no_copy(
global,
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
10 changes: 4 additions & 6 deletions src/runtime/api/bun/subprocess/Readable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,10 @@ impl Readable {

// Ownership of the mimalloc-backed buffer transfers to JSC
// (freed via `MarkedArrayBuffer_deallocator`).
Ok(jsc::MarkedArrayBuffer {
buffer: jsc::ArrayBuffer::from_owned_bytes(own, jsc::JSType::Uint8Array),
owns_buffer: true,
pinned: false,
}
.to_node_buffer(global))
Ok(
jsc::MarkedArrayBuffer::from_owned_bytes(own, jsc::JSType::Uint8Array)
.to_node_buffer(global),
)
}
_ => Ok(JSValue::UNDEFINED),
}
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
36 changes: 13 additions & 23 deletions src/runtime/node/node_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4716,14 +4716,14 @@ pub mod ret {
// items dropped here (auto free)
Ok(array)
}
Readdir::Buffers(items) => {
Readdir::Buffers(mut items) => {
// `JSValue.fromAny(_, []Buffer, _)` — generic-slice arm:
// build an empty array, push `item.toJS(globalObject)` for
// each. Ownership of every `Buffer`'s bytes transfers to
// JSC via `MarkedArrayBuffer::to_js`; the boxed slice
// itself is freed when `items` drops.
let array = JSValue::create_empty_array(global_object, items.len())?;
for (i, item) in items.iter().enumerate() {
for (i, item) in items.iter_mut().enumerate() {
let res = item.to_js(global_object)?;
if res == JSValue::ZERO {
return Ok(JSValue::ZERO);
Expand Down 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