diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 3e773009673d..528daab49fcf 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -403,7 +403,21 @@ impl ArrayBuffer { } } - pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> ArrayBuffer { + /// Wrap a borrowed slice's pointer in an `ArrayBuffer` without taking + /// ownership. The returned struct (and any JS object created from it) + /// stores `bytes.as_mut_ptr()` with no lifetime parameter. + /// + /// Prefer [`ArrayBuffer::from_owned_bytes`] / [`ArrayBuffer::from_owned_vec`] + /// — they make the ownership transfer visible in the type system. + /// + /// # Safety + /// The allocation backing `bytes` must outlive every use of the returned + /// `ArrayBuffer`, including the GC finalizer of any JS object created from + /// it. If a deallocator is installed (`to_js`, `to_js_unchecked`, + /// `to_js_with_context`), that deallocator becomes the sole owner of the + /// allocation: it must be valid to free `bytes.as_mut_ptr()` with it + /// exactly once, and nothing else may free or reuse the allocation. + pub unsafe fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> ArrayBuffer { ArrayBuffer { len: u32::try_from(bytes.len()).expect("int cast") as usize, byte_len: u32::try_from(bytes.len()).expect("int cast") as usize, @@ -435,6 +449,24 @@ impl ArrayBuffer { } } + /// [`ArrayBuffer::from_owned_bytes`] for a `Vec` whose capacity may + /// exceed its length. The excess capacity is discarded without + /// reallocating: the deallocator installed by `to_js*` frees the whole + /// allocation from its data pointer, so the capacity is not needed. + pub fn from_owned_vec(bytes: Vec, typed_array_type: JSType) -> ArrayBuffer { + // Ownership transfers to JSC (see `from_owned_bytes`); suppress the + // Vec's Drop and keep only ptr/len. + let mut bytes = core::mem::ManuallyDrop::new(bytes); + let len = bytes.len(); + ArrayBuffer { + len: u32::try_from(len).expect("int cast") as usize, + byte_len: u32::try_from(len).expect("int cast") as usize, + typed_array_type, + ptr: bytes.as_mut_ptr(), + ..Default::default() + } + } + pub fn to_js_unchecked(self, ctx: &JSGlobalObject) -> JsResult { // The reason for this is // JSC C API returns a detached arraybuffer @@ -917,15 +949,13 @@ impl MarkedArrayBuffer { } pub fn from_string(str: &[u8]) -> Result { - // 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::(); - // 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 { @@ -936,9 +966,13 @@ 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`). + 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, } } @@ -959,7 +993,7 @@ 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 { diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 4c66b633277c..b3550ed95850 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -2630,12 +2630,10 @@ pub mod JSZlib { list.shrink_to_fit(); // Ownership of the allocation transfers to JSC; freed via // `global_deallocator` once the ArrayBuffer is finalized. - let leaked: &'static mut [u8] = list.leak(); - let ptr = leaked.as_mut_ptr(); - let array_buffer = ArrayBuffer::from_bytes(leaked, jsc::JSType::Uint8Array); + let array_buffer = ArrayBuffer::from_owned_vec(list, jsc::JSType::Uint8Array); array_buffer.to_js_with_context( global_this, - ptr.cast::(), + array_buffer.ptr.cast::(), Some(global_deallocator), ) } @@ -2680,12 +2678,10 @@ pub mod JSZlib { // Ownership of the allocation transfers to JSC; freed via // `global_deallocator` once the ArrayBuffer is finalized. - let leaked: &'static mut [u8] = list.leak(); - let ptr = leaked.as_mut_ptr(); - let array_buffer = ArrayBuffer::from_bytes(leaked, jsc::JSType::Uint8Array); + let array_buffer = ArrayBuffer::from_owned_vec(list, jsc::JSType::Uint8Array); array_buffer.to_js_with_context( global_this, - ptr.cast::(), + array_buffer.ptr.cast::(), Some(global_deallocator), ) } @@ -2781,12 +2777,10 @@ pub mod JSZlib { list.shrink_to_fit(); // Ownership of the allocation transfers to JSC; freed via // `global_deallocator` once the ArrayBuffer is finalized. - let leaked: &'static mut [u8] = list.leak(); - let ptr = leaked.as_mut_ptr(); - let array_buffer = ArrayBuffer::from_bytes(leaked, jsc::JSType::Uint8Array); + let array_buffer = ArrayBuffer::from_owned_vec(list, jsc::JSType::Uint8Array); array_buffer.to_js_with_context( global_this, - ptr.cast::(), + array_buffer.ptr.cast::(), Some(global_deallocator), ) } @@ -2824,12 +2818,10 @@ pub mod JSZlib { // Ownership of the allocation transfers to JSC; freed via // `global_deallocator` once the ArrayBuffer is finalized. - let leaked: &'static mut [u8] = list.leak(); - let ptr = leaked.as_mut_ptr(); - let array_buffer = ArrayBuffer::from_bytes(leaked, jsc::JSType::Uint8Array); + let array_buffer = ArrayBuffer::from_owned_vec(list, jsc::JSType::Uint8Array); array_buffer.to_js_with_context( global_this, - ptr.cast::(), + array_buffer.ptr.cast::(), Some(global_deallocator), ) } diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index 22701199f1b3..c07826bb7b87 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -1810,11 +1810,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); + // MarkedArrayBuffer::from_owned_bytes takes ownership of the allocation + // (freed via mimalloc on the C++ side). + 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, diff --git a/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs b/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs index c18f94ec2b20..77ab9b0435f9 100644 --- a/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs +++ b/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs @@ -314,13 +314,14 @@ impl PipeReader { State::Done(bytes) => { let bytes = core::mem::take(bytes); // `defer this.state = .{ .done = &.{} }` — state.done is now empty via take(). - // PORT NOTE: `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) + // `MarkedArrayBuffer::from_owned_bytes` adopts the allocation with + // `owns_buffer = true` (freed via mimalloc on the JS side) — same + // pattern as `MarkedArrayBuffer::from_string`. + MarkedArrayBuffer::from_owned_bytes( + bytes.into_boxed_slice(), + jsc::JSType::Uint8Array, + ) + .to_node_buffer(global_this) } _ => JSValue::UNDEFINED, } diff --git a/src/runtime/ffi/FFIObject.rs b/src/runtime/ffi/FFIObject.rs index 53c0a043f59b..5274386534a3 100644 --- a/src/runtime/ffi/FFIObject.rs +++ b/src/runtime/ffi/FFIObject.rs @@ -838,9 +838,12 @@ pub fn to_array_buffer( } } - // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. + // SAFETY: ptr/len came from get_ptr_slice; the memory is owned by the + // FFI caller, who promises (per the `toArrayBuffer` API contract) that + // it stays valid until the optional finalization callback runs. let slice = unsafe { core::slice::from_raw_parts_mut(ptr, len) }; - ArrayBuffer::from_bytes(slice, jsc::JSType::ArrayBuffer).to_js_with_context( + // SAFETY: see above — lifetime/ownership is the FFI caller's contract. + unsafe { ArrayBuffer::from_bytes(slice, jsc::JSType::ArrayBuffer) }.to_js_with_context( global_this, ctx.unwrap_or(core::ptr::null_mut()), callback, diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index a9c3f6a4cef9..6fb2f520243c 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1754,8 +1754,13 @@ impl<'a> PipelineTask<'a> { out_slice.len(), ) }; - let v = ArrayBuffer::from_bytes(mut_slice, jsc::JSType::Uint8Array) - .to_js_with_context(global, core::ptr::null_mut(), Some(out.free)); + // SAFETY: `out.bytes` is the codec-owned allocation whose + // ownership transfers to JSC; `out.free` (installed below) + // is the sole release path now that the codec `Drop` is + // suppressed via ManuallyDrop. + let v = + unsafe { ArrayBuffer::from_bytes(mut_slice, jsc::JSType::Uint8Array) } + .to_js_with_context(global, core::ptr::null_mut(), Some(out.free)); match v { Ok(v) => promise.resolve(global, v)?, Err(_) => return promise.reject(global, Err(jsc::JsError::Thrown)), diff --git a/src/runtime/node.rs b/src/runtime/node.rs index 4fdb840b91e2..af6602d81d7b 100644 --- a/src/runtime/node.rs +++ b/src/runtime/node.rs @@ -410,12 +410,10 @@ impl MaybeSysExt for Maybe { // `ArrayBuffer.fromBytes` and ownership transfers to JSC — the // GC-installed deallocator (`MarkedArrayBuffer_deallocator`) // calls `mi_free` on the buffer when the JS object is - // collected. Leak the `Vec` here to hand the allocation to - // JSC; Bun's global allocator is mimalloc, so `to_js`'s + // collected. Bun's global allocator is mimalloc, so `to_js`'s // `mi_is_in_heap_region` check succeeds and the buffer is // freed by JSC, not Rust. - let bytes: &mut [u8] = Vec::leak(r.into()); - bun_jsc::ArrayBuffer::from_bytes(bytes, bun_jsc::JSType::ArrayBuffer) + bun_jsc::ArrayBuffer::from_owned_vec(r.into(), bun_jsc::JSType::ArrayBuffer) .to_js(global_object) } Err(e) => Ok(e.to_js(global_object)), @@ -643,8 +641,8 @@ impl MaybeToJs for Vec { // PORT NOTE: ownership transfers to JSC (freed via // `MarkedArrayBuffer_deallocator` → `mi_free`); see // `MaybeSysExt::to_array_buffer` above for the full rationale. - let bytes: &mut [u8] = Vec::leak(self); - bun_jsc::ArrayBuffer::from_bytes(bytes, bun_jsc::JSType::ArrayBuffer).to_js(global_object) + bun_jsc::ArrayBuffer::from_owned_vec(self, bun_jsc::JSType::ArrayBuffer) + .to_js(global_object) } } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index d264a864c36b..d0b8e8f672c4 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -7077,16 +7077,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 allocation is transferred 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 { @@ -7218,15 +7212,11 @@ impl NodeFS { }; } } - let raw = bun_core::heap::into_raw( + // Ownership transferred to JSC; freed via ArrayBuffer finalizer. + 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, ))) } @@ -7391,11 +7381,9 @@ 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 transferred to JSC; freed via ArrayBuffer finalizer. + Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes( + buf.into_boxed_slice(), bun_jsc::JSType::Uint8Array, ))) } diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 458e98a17a3c..63c5eed762ae 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -2363,12 +2363,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) + // `MarkedArrayBuffer::from_owned_bytes` adopts the allocation + // (freed by the JSC ArrayBuffer destructor). + MarkedArrayBuffer::from_owned_bytes(core::mem::take(bytes), jsc::JSType::Uint8Array) .to_node_buffer(global_this) } _ => JSValue::UNDEFINED, diff --git a/src/runtime/webcore/ArrayBufferSink.rs b/src/runtime/webcore/ArrayBufferSink.rs index 72c96eca575f..b16c83b092e5 100644 --- a/src/runtime/webcore/ArrayBufferSink.rs +++ b/src/runtime/webcore/ArrayBufferSink.rs @@ -223,7 +223,10 @@ impl ArrayBufferSink { pub fn end_from_js(&mut self, _global_this: &JSGlobalObject) -> bun_sys::Result { if self.done { - return Ok(ArrayBuffer::from_bytes(&mut [], JSType::ArrayBuffer)); + return Ok(ArrayBuffer::from_owned_bytes( + Box::default(), + JSType::ArrayBuffer, + )); } debug_assert!(self.next.is_none()); diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 607ce8df5f4a..34bbad876fe0 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3124,9 +3124,10 @@ impl BlobExt for Blob { let store = self.store().expect("infallible: store present").clone(); // SAFETY: `from_bytes` only records ptr+len into the FFI struct; the // pointer is then handed to JSC as an external buffer backing whose - // lifetime is the cloned `store` ref above. No Rust-side `&` to the - // Store bytes is live across this reborrow. Mirrors Zig `@constCast`. - jsc::ArrayBuffer::from_bytes(unsafe { &mut *buf }, TYPED_ARRAY_VIEW) + // lifetime is the cloned `store` ref above (released by the + // deallocator on GC). No Rust-side `&` to the Store bytes is live + // across this reborrow. Mirrors Zig `@constCast`. + unsafe { jsc::ArrayBuffer::from_bytes(&mut *buf, TYPED_ARRAY_VIEW) } .to_js_with_context( global, store.into_raw().cast::(), @@ -3145,7 +3146,7 @@ impl BlobExt for Blob { let store = self.take_store().expect("transfer with null store"); // SAFETY: see `Share` arm. After `take()` the store ref is moved // out of `self`, so JSC becomes the sole owner via the deallocator. - jsc::ArrayBuffer::from_bytes(unsafe { &mut *buf }, TYPED_ARRAY_VIEW) + unsafe { jsc::ArrayBuffer::from_bytes(&mut *buf, TYPED_ARRAY_VIEW) } .to_js_with_context( global, store.into_raw().cast::(), @@ -3161,11 +3162,15 @@ impl BlobExt for Blob { return Err(global.throw_out_of_memory()); } // SAFETY: `Temporary` ⇒ `buf` is a leaked `Box<[u8]>` we exclusively own; - // ownership is transferred to JSC (Zig: `JSC.MarkedArrayBuffer.fromBytes`). + // reclaim it and transfer ownership to JSC (Zig: + // `JSC.MarkedArrayBuffer.fromBytes`). // `to_js_unchecked`: `to_js`'s heap-region probe would skip the deallocator // for a non-mimalloc buffer, but `Temporary` is always default-allocator. - jsc::ArrayBuffer::from_bytes(unsafe { &mut *buf }, TYPED_ARRAY_VIEW) - .to_js_unchecked(global) + jsc::ArrayBuffer::from_owned_bytes( + unsafe { bun_core::heap::take(buf) }, + TYPED_ARRAY_VIEW, + ) + .to_js_unchecked(global) } } } diff --git a/src/runtime/webcore/ReadableStream.rs b/src/runtime/webcore/ReadableStream.rs index 0aa39990c6aa..0b564cadb69c 100644 --- a/src/runtime/webcore/ReadableStream.rs +++ b/src/runtime/webcore/ReadableStream.rs @@ -6,7 +6,6 @@ use crate::webcore::jsc::SysErrorJsc as _; use crate::webcore::jsc::{self as jsc, CallFrame, JSGlobalObject, JSValue, JsResult}; // `bun_jsc` not yet a dep; alias to local shim so `bun_jsc::Strong` etc. resolve. use crate::webcore::jsc as bun_jsc; -use bun_collections::VecExt; use bun_sys as syscall; use crate::webcore::streams; @@ -1218,14 +1217,12 @@ impl NewSource { call_frame: &CallFrame, ) -> JsResult { self.this_jsvalue = call_frame.this(); - let mut list = self.drain(); + let list = self.drain(); if list.len() > 0 { // Ownership of the buffer transfers to JSC: `to_js` installs - // `MarkedArrayBuffer_deallocator` which `mi_free`s on GC. Suppress - // `Vec::Drop` so the same allocation isn't freed twice (once - // here on scope exit, once by the GC). Mirrors `streams::Start::to_js`. - let ab = jsc::ArrayBuffer::from_bytes(list.slice_mut(), jsc::JSType::Uint8Array); - let _ = core::mem::ManuallyDrop::new(list); + // `MarkedArrayBuffer_deallocator` which `mi_free`s on GC. + // Mirrors `streams::Start::to_js`. + let ab = jsc::ArrayBuffer::from_owned_vec(list, jsc::JSType::Uint8Array); return ab.to_js(global_this); } Ok(JSValue::UNDEFINED) diff --git a/src/runtime/webcore/TextEncoder.rs b/src/runtime/webcore/TextEncoder.rs index f68f9e4ac850..dfc08e5bff0e 100644 --- a/src/runtime/webcore/TextEncoder.rs +++ b/src/runtime/webcore/TextEncoder.rs @@ -51,8 +51,8 @@ pub unsafe extern "C" fn TextEncoder__encode8( return global_this.throw_out_of_memory_value(); }; debug_assert!(bytes.len() >= slice.len()); - // PORT NOTE: ownership transfers to JSC via to_js_unchecked; leak the Vec. - ArrayBuffer::from_bytes(bytes.leak(), JSType::Uint8Array) + // Ownership of the Vec's allocation transfers to JSC via to_js_unchecked. + ArrayBuffer::from_owned_vec(bytes, JSType::Uint8Array) .to_js_unchecked(global_this) .unwrap_or(JSValue::ZERO) } @@ -104,8 +104,8 @@ pub unsafe extern "C" fn TextEncoder__encode16( uint8array } else { let bytes = strings::to_utf8_alloc_with_type(slice); - // PORT NOTE: ownership transfers to JSC via to_js_unchecked; leak the Vec. - ArrayBuffer::from_bytes(bytes.leak(), JSType::Uint8Array) + // Ownership of the Vec's allocation transfers to JSC via to_js_unchecked. + ArrayBuffer::from_owned_vec(bytes, JSType::Uint8Array) .to_js_unchecked(global_this) .unwrap_or(JSValue::ZERO) } @@ -153,8 +153,8 @@ pub unsafe extern "C" fn c(global_this: &JSGlobalObject, ptr: *const u16, len: u uint8array } else { let bytes = strings::to_utf8_alloc_with_type(slice); - // PORT NOTE: ownership transfers to JSC via to_js_unchecked; leak the Vec. - ArrayBuffer::from_bytes(bytes.leak(), JSType::Uint8Array) + // Ownership of the Vec's allocation transfers to JSC via to_js_unchecked. + ArrayBuffer::from_owned_vec(bytes, JSType::Uint8Array) .to_js_unchecked(global_this) .unwrap_or(JSValue::ZERO) } diff --git a/src/runtime/webcore/streams.rs b/src/runtime/webcore/streams.rs index 35adafa31f61..e439aed233d7 100644 --- a/src/runtime/webcore/streams.rs +++ b/src/runtime/webcore/streams.rs @@ -95,13 +95,9 @@ impl Start { Start::ChunkSize(chunk) => Ok(JSValue::from(chunk)), Start::Err(err) => Err(err.throw(global_this)), Start::OwnedAndDone(list) => { - // PORT NOTE: Zig captures `|list|` by bitwise copy with no destructor and - // hands the allocation to JSC (no-copy + MarkedArrayBuffer_deallocator). In - // Rust `list` is an owned Vec whose Drop would free the same buffer → - // double-free. Suppress Drop via ManuallyDrop so JSC is the sole owner. - let mut list = core::mem::ManuallyDrop::new(list); - let ab = ArrayBuffer::from_bytes(list.slice_mut(), JSType::Uint8Array); - ab.to_js(global_this) + // Ownership of the Vec's allocation transfers to JSC (no-copy + + // MarkedArrayBuffer_deallocator). + ArrayBuffer::from_owned_vec(list, JSType::Uint8Array).to_js(global_this) } Start::Done(list) => { ArrayBuffer::create::<{ JSType::Uint8Array }>(global_this, list.slice()) @@ -878,16 +874,14 @@ impl StreamResult { // PORT NOTE: Zig overwrites `result.* = .{ .temporary = .{} }` with no // destructor after handing the buffer to JSC. In Rust the later // `*result = Temporary(...)` in fulfill_promise drops the old Vec, - // double-freeing the allocation now owned by JSC. Move it out and suppress - // Drop so JSC's MarkedArrayBuffer_deallocator is the sole owner. - let mut taken = core::mem::ManuallyDrop::new(core::mem::take(list)); - let ab = ArrayBuffer::from_bytes(taken.slice_mut(), JSType::Uint8Array); + // double-freeing the allocation now owned by JSC. Move it out so JSC's + // MarkedArrayBuffer_deallocator is the sole owner. + let ab = ArrayBuffer::from_owned_vec(core::mem::take(list), JSType::Uint8Array); ab.to_js(global_this) } StreamResult::OwnedAndDone(list) => { // PORT NOTE: see Owned arm above — same ownership transfer to JSC. - let mut taken = core::mem::ManuallyDrop::new(core::mem::take(list)); - let ab = ArrayBuffer::from_bytes(taken.slice_mut(), JSType::Uint8Array); + let ab = ArrayBuffer::from_owned_vec(core::mem::take(list), JSType::Uint8Array); ab.to_js(global_this) } StreamResult::Temporary(temp) | StreamResult::TemporaryAndDone(temp) => { diff --git a/test/js/web/encoding/text-encoder.test.js b/test/js/web/encoding/text-encoder.test.js index 150271f3badd..1be6b2adac17 100644 --- a/test/js/web/encoding/text-encoder.test.js +++ b/test/js/web/encoding/text-encoder.test.js @@ -479,6 +479,33 @@ describe("TextEncoder", () => { }); }); + it("should round-trip large heap-allocated encode results", () => { + // Strings longer than the 2048-byte stack buffer take the heap path in + // TextEncoder__encode8/__encode16, where the encoder allocates a Vec and + // transfers ownership of the allocation to the returned Uint8Array. + const encoder = new TextEncoder(); + const decoder = new TextDecoder(); + + // Latin-1 (8-bit) source with non-ASCII so the UTF-8 output grows past + // the input length. + const latin1Seed = "héllo wörld å "; + const latin1 = Buffer.alloc(Buffer.byteLength(latin1Seed) * 200, latin1Seed).toString(); + const encoded8 = encoder.encode(latin1); + expect(encoded8.length).toBe(getByteLength(latin1)); + expect(decoder.decode(encoded8)).toBe(latin1); + + // UTF-16 source. + const utf16Seed = "❤️ Red Heart ✨ Sparkles 🔥 Fire "; + const utf16 = Buffer.alloc(Buffer.byteLength(utf16Seed) * 100, utf16Seed).toString(); + const encoded16 = encoder.encode(utf16); + expect(encoded16.length).toBe(getByteLength(utf16)); + expect(decoder.decode(encoded16)).toBe(utf16); + + Bun.gc(true); + expect(decoder.decode(encoded8)).toBe(latin1); + expect(decoder.decode(encoded16)).toBe(utf16); + }); + it("should encode utf-16 rope text", () => { gcTrace(true); var textReal = `❤️ Red Heart ✨ Sparkles 🔥 Fire`; diff --git a/test/js/web/streams/streams.test.js b/test/js/web/streams/streams.test.js index 683f8d300afb..5931af614ef3 100644 --- a/test/js/web/streams/streams.test.js +++ b/test/js/web/streams/streams.test.js @@ -1127,6 +1127,21 @@ it("Bun.file().stream() read text from large file", async () => { } }); +it("Bun.file().stream() chunks survive GC and round-trip", async () => { + // The native reader hands each chunk's heap allocation to JSC + // (ArrayBuffer::from_owned_vec → MarkedArrayBuffer_deallocator). If the + // ownership transfer regressed to a borrowed/dangling pointer, the bytes + // would be corrupted after the source buffer is freed or reused. + const expected = Buffer.alloc(512 * 1024, "bun-streams-roundtrip!").toString(); + using dir = tempDir("streams-roundtrip", { "data.txt": expected }); + const chunks = []; + for await (const chunk of Bun.file(join(String(dir), "data.txt")).stream()) { + chunks.push(chunk); + } + Bun.gc(true); + expect(Buffer.concat(chunks).toString()).toBe(expected); +}); + it("fs.createReadStream(filename) should be able to break inside async loop", async () => { for (let i = 0; i < 10; i++) { const fileStream = createReadStream(join(import.meta.dir, "..", "fetch", "fixture.png"));