From 7faa79c80d6f8704bf65055c6465495805af65cd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:52:52 +0000 Subject: [PATCH 1/9] StringOrBuffer: pin the backing ArrayBuffer lazily on first slice() MarkedArrayBuffer now stores its ArrayBuffer descriptor behind a Cell and defers pinning: from_js captures only the JSValue, and the first bytes() or slice() call pins the backing JSC::ArrayBuffer, reads vector()/ byteLength() once, and caches the result. Drop releases the pin; Unprotect clears the flag first so a ThreadSafe dropped on the JS thread leaves nothing for an off-thread Drop to do. to_thread_safe() forces the pin+snapshot before handing off. This makes argument-coercion order irrelevant for StringOrBuffer::Buffer without an ad-hoc re-snapshot at each call site: a toString()/valueOf() on a later argument can transfer or resize the buffer, and the first slice() observes the post-coercion state (detached -> empty). The six buffer.buffer = ArrayBuffer::from_typed_array(...) re-derive lines in CryptoHasher/PBKDF2/PasswordObject/scrypt/Bun.sha are removed, as are the manual pin-at-end blocks in fs.write/fs.read args (bytes() now does that on first access). hash_to_bytes and friends take &Buffer instead of a raw ArrayBuffer copy and use bytes() for the writable span. --- src/jsc/JSValue.rs | 8 + src/jsc/array_buffer.rs | 201 ++++++++++++++++----- src/jsc/bindings/bindings.cpp | 23 +++ src/jsc/bindings/headers.h | 1 + src/jsc/node_path.rs | 24 +-- src/runtime/api/BunObject.rs | 5 +- src/runtime/api/MarkdownObject.rs | 2 +- src/runtime/api/bun/subprocess/Readable.rs | 8 +- src/runtime/crypto/CryptoHasher.rs | 113 +++++------- src/runtime/crypto/PBKDF2.rs | 17 +- src/runtime/crypto/PasswordObject.rs | 9 +- src/runtime/node/node_crypto_binding.rs | 8 +- src/runtime/node/node_fs.rs | 66 ++----- src/runtime/node/types.rs | 68 ++----- 14 files changed, 283 insertions(+), 270 deletions(-) diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 3a30866ff15d..74ececea8709 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -930,6 +930,14 @@ impl JSValue { } self.as_array_buffer(global) } + /// Pin the backing `JSC::ArrayBuffer` of this value so it cannot be + /// detached; see [`as_pinned_arraybuffer`]. Returns `false` if the value + /// has no backing `ArrayBuffer` impl. Release with + /// [`JSValue::unpin_array_buffer`]. + #[inline] + pub fn pin_array_buffer(self) -> bool { + JSC__JSValue__pinArrayBuffer(self) + } /// Generic downcast. Dispatches via [`JsClass::from_js`]. #[inline] pub fn as_(self) -> Option<*mut T> { diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index de94e5c9d181..2c510f8a37cb 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -1,3 +1,4 @@ +use core::cell::Cell; use core::ffi::{c_uint, c_void}; use core::ptr; @@ -135,6 +136,14 @@ unsafe extern "C" { safe fn JSC__ArrayBuffer__deref(self_: &JSCArrayBuffer); // safe: by-value `JSValue`; no-op for non-buffer values. safe fn JSC__JSValue__unpinArrayBuffer(v: JSValue); + // safe: by-value `JSValue` known (by caller) to be a buffer-type cell; + // `&mut *mut u8` / `&mut usize` are ABI-identical to non-null out-params + // the callee fills unconditionally. + safe fn JSC__JSValue__arrayBufferLiveBytes( + v: JSValue, + out_ptr: &mut *mut u8, + out_byte_len: &mut usize, + ); } impl JSValue { @@ -815,28 +824,73 @@ impl TypedArrayType { // MarkedArrayBuffer // ────────────────────────────────────────────────────────────────────────── +/// A byte-slice view over either a JS `ArrayBuffer`/typed array or a +/// Rust-owned allocation. +/// +/// For a JS-backed buffer the `(ptr, byte_len)` snapshot is taken lazily on +/// the first [`slice`]/[`bytes`] call: the backing `JSC::ArrayBuffer` is +/// pinned, its `vector()`/`byteLength()` is read once, and the result is +/// cached. Until then only the `JSValue` is meaningful, so user JS that runs +/// between construction and first use (argument coercion on a later +/// parameter) may freely `transfer()`/`resize()` the backing store and the +/// first `slice()` observes the post-coercion state. +/// +/// `Drop` releases the pin on the JS thread. A `MarkedArrayBuffer` that will +/// be dropped off the JS thread must be routed through the +/// [`crate::Unprotect`] hook (see `StringOrBuffer::unprotect`) which clears +/// the pin first so `Drop` becomes a no-op. #[derive(Default)] pub struct MarkedArrayBuffer { - pub buffer: ArrayBuffer, - pub owns_buffer: bool, - pub pinned: bool, + buffer: Cell, + owns_buffer: Cell, + pinned: Cell, } impl MarkedArrayBuffer { - pub fn from_typed_array(ctx: &JSGlobalObject, value: JSValue) -> MarkedArrayBuffer { - MarkedArrayBuffer { - owns_buffer: false, - pinned: false, - buffer: ArrayBuffer::from_typed_array(ctx, value), + #[inline] + const fn new(buffer: ArrayBuffer, owns_buffer: bool, pinned: bool) -> Self { + Self { + buffer: Cell::new(buffer), + owns_buffer: Cell::new(owns_buffer), + pinned: Cell::new(pinned), } } + pub fn from_typed_array(ctx: &JSGlobalObject, value: JSValue) -> MarkedArrayBuffer { + Self::new(ArrayBuffer::from_typed_array(ctx, value), false, false) + } + pub fn from_array_buffer(ctx: &JSGlobalObject, value: JSValue) -> MarkedArrayBuffer { - MarkedArrayBuffer { - owns_buffer: false, - pinned: false, - buffer: ArrayBuffer::from_array_buffer(ctx, value), - } + Self::new(ArrayBuffer::from_array_buffer(ctx, value), false, false) + } + + /// Wrap an already-pinned descriptor. Used by callers that pinned via + /// [`JSValue::as_pinned_arraybuffer`] themselves. + #[inline] + pub fn from_pinned(buffer: ArrayBuffer) -> MarkedArrayBuffer { + Self::new(buffer, false, true) + } + + /// A non-owning view that neither owns the allocation nor the original's + /// pin. [`bytes`] on the borrow takes its own pin (pin count is a + /// counter), released by its own `Drop`. + #[inline] + pub fn borrow(&self) -> MarkedArrayBuffer { + Self::new(self.buffer.get(), false, false) + } + + /// Adopt a Rust-owned byte descriptor (freed by [`destroy`] / + /// [`to_js`], not by `Drop`). + #[inline] + pub fn from_owned(buffer: ArrayBuffer) -> MarkedArrayBuffer { + Self::new(buffer, true, false) + } + + /// Wrap a JS-backed descriptor (its `value` must be set). [`bytes`] pins + /// on first access. + #[inline] + pub fn from_unpinned(buffer: ArrayBuffer) -> MarkedArrayBuffer { + Self::new(buffer, false, false) } pub fn from_string(str: &[u8]) -> Result { @@ -852,50 +906,95 @@ impl MarkedArrayBuffer { } pub fn from_js(global: &JSGlobalObject, value: JSValue) -> Option { - let array_buffer = value.as_array_buffer(global)?; - Some(MarkedArrayBuffer { - buffer: array_buffer, - owns_buffer: false, - pinned: false, - }) + Some(Self::new(value.as_array_buffer(global)?, false, false)) } pub fn from_js_pinned(global: &JSGlobalObject, value: JSValue) -> Option { - let buffer = value.as_pinned_arraybuffer(global)?; - Some(MarkedArrayBuffer { - buffer, - owns_buffer: false, - pinned: true, - }) + Some(Self::new(value.as_pinned_arraybuffer(global)?, false, true)) } pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { - MarkedArrayBuffer { - buffer: ArrayBuffer::from_bytes(bytes, typed_array_type), - owns_buffer: true, - pinned: false, + Self::new(ArrayBuffer::from_bytes(bytes, typed_array_type), true, false) + } + + pub const EMPTY: MarkedArrayBuffer = Self::new(ArrayBuffer::EMPTY, false, false); + + /// Copy of the inner descriptor. `ptr`/`byte_len` reflect whatever the + /// most recent [`bytes`] call cached (or the construction-time snapshot + /// if [`bytes`] has not run yet). + #[inline] + pub fn buffer(&self) -> ArrayBuffer { + self.buffer.get() + } + + #[inline] + pub fn value(&self) -> JSValue { + self.buffer.get().value + } + + #[inline] + pub fn is_pinned(&self) -> bool { + self.pinned.get() + } + + #[inline] + pub fn owns_buffer(&self) -> bool { + self.owns_buffer.get() + } + + /// Release the pin taken by [`bytes`]/[`from_js_pinned`] and clear the + /// flag so `Drop` is a no-op. JS-thread only. + #[inline] + pub fn unpin(&self) { + if self.pinned.replace(false) { + self.buffer.get().unpin(); } } - pub const EMPTY: MarkedArrayBuffer = MarkedArrayBuffer { - owns_buffer: false, - pinned: false, - buffer: ArrayBuffer::EMPTY, - }; + /// Pin the backing `JSC::ArrayBuffer`, read its `vector()`/`byteLength()` + /// once, and cache the result; subsequent calls return the cache. For a + /// Rust-owned or already-pinned buffer this returns the existing + /// snapshot. A detached buffer pins nothing and yields `(null, 0)`. + /// + /// Must run on the JS thread for the first call on a JS-backed buffer; + /// callers that hand the buffer to a threadpool must call this (directly + /// or via [`slice`]) before the hand-off. + pub fn bytes(&self) -> (*mut u8, usize) { + let mut ab = self.buffer.get(); + if self.pinned.get() || self.owns_buffer.get() || ab.value.is_empty() { + return (ab.ptr, ab.byte_len); + } + if ab.value.pin_array_buffer() { + self.pinned.set(true); + } + let mut ptr: *mut u8 = core::ptr::null_mut(); + let mut len: usize = 0; + JSC__JSValue__arrayBufferLiveBytes(ab.value, &mut ptr, &mut len); + ab.ptr = ptr; + ab.byte_len = len; + self.buffer.set(ab); + (ptr, len) + } #[inline] pub fn slice(&self) -> &[u8] { - self.buffer.byte_slice() + let (ptr, len) = self.bytes(); + if ptr.is_null() { + return &[]; + } + // SAFETY: `ptr`/`len` describe the pinned JSC-owned backing store (or + // the Rust-owned allocation for `owns_buffer`), valid while `self` + // keeps the JSValue rooted / the allocation alive. + unsafe { core::slice::from_raw_parts(ptr, len) } } /// 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; /// `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; + if self.owns_buffer.replace(false) { // SAFETY: buffer.ptr was allocated by the global allocator (heap::alloc / allocator.dupe). - unsafe { bun_alloc::default_alloc::free(self.buffer.ptr.cast()) }; + unsafe { bun_alloc::default_alloc::free(self.buffer.get().ptr.cast()) }; } } @@ -903,21 +1002,22 @@ impl MarkedArrayBuffer { // `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; + let mut buf = self.buffer.get(); JSValue::create_buffer(global, buf.byte_slice_mut()) } pub fn to_js(&self, global: &JSGlobalObject) -> JsResult { - if !self.buffer.value.is_empty_or_undefined_or_null() { - return Ok(self.buffer.value); + let buffer = self.buffer.get(); + if !buffer.value.is_empty_or_undefined_or_null() { + return Ok(buffer.value); } - if self.buffer.byte_len == 0 { + if buffer.byte_len == 0 { // SAFETY: null `ptr` with `len == 0` and no deallocator — every // obligation of the callee's contract holds trivially. return unsafe { make_typed_array_with_bytes_no_copy( global, - self.buffer.typed_array_type.to_typed_array_type(), + buffer.typed_array_type.to_typed_array_type(), ptr::null_mut(), 0, None, @@ -932,16 +1032,23 @@ impl MarkedArrayBuffer { unsafe { make_typed_array_with_bytes_no_copy( global, - self.buffer.typed_array_type.to_typed_array_type(), - self.buffer.ptr.cast(), - self.buffer.byte_len, + buffer.typed_array_type.to_typed_array_type(), + buffer.ptr.cast(), + buffer.byte_len, Some(MarkedArrayBuffer_deallocator), - self.buffer.ptr.cast(), + buffer.ptr.cast(), ) } } } +impl Drop for MarkedArrayBuffer { + #[inline] + fn drop(&mut self) { + self.unpin(); + } +} + // ────────────────────────────────────────────────────────────────────────── // Deallocators // ────────────────────────────────────────────────────────────────────────── diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index d261ed095ccc..dcd50363692c 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3212,6 +3212,29 @@ bool JSC__JSValue__asArrayBuffer( return true; } +// Re-read the current vector()/byteLength() from a buffer-type cell without an +// exception scope. Caller guarantees `encodedValue` is a live +// JSArrayBuffer/JSArrayBufferView cell (type-checked at the point the +// Bun::ArrayBuffer descriptor was constructed). A detached buffer yields +// (nullptr, 0). Used by `MarkedArrayBuffer::slice()` to compute the byte view +// lazily so a later argument's toString()/valueOf() that transfers or resizes +// the buffer is observed instead of leaving a stale (ptr, len). +CPP_DECL void JSC__JSValue__arrayBufferLiveBytes( + JSC::EncodedJSValue encodedValue, uint8_t** out_ptr, size_t* out_byte_len) +{ + JSC::JSValue value = JSC::JSValue::decode(encodedValue); + auto* cell = value.asCell(); + if (cell->type() == JSC::JSType::ArrayBufferType) { + auto* buffer = uncheckedDowncast(cell)->impl(); + *out_ptr = static_cast(buffer->data()); + *out_byte_len = buffer->byteLength(); + return; + } + auto* view = uncheckedDowncast(cell); + *out_ptr = static_cast(view->vector()); + *out_byte_len = view->byteLength(); +} + // Pin/unpin the backing ArrayBuffer of a JSArrayBuffer or JSArrayBufferView so // its storage cannot move or be freed while a native borrower holds a slice // into it. SharedArrayBuffer is never detachable and never moves, so it is left diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index 6cd8cc0d5b2f..892e8c401f1c 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -199,6 +199,7 @@ CPP_DECL uint32_t JSC__JSMap__size(JSC::JSMap* arg0, JSC::JSGlobalObject* arg1); CPP_DECL void JSC__JSValue__then(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, JSC::EncodedJSValue JSValue2, SYSV_ABI JSC::EncodedJSValue(* ArgFn3)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1), SYSV_ABI JSC::EncodedJSValue(* ArgFn4)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1)); CPP_DECL bool JSC__JSValue__asArrayBuffer(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, Bun__ArrayBuffer* arg2); +CPP_DECL void JSC__JSValue__arrayBufferLiveBytes(JSC::EncodedJSValue JSValue0, uint8_t** arg1, size_t* arg2); CPP_DECL unsigned char JSC__JSValue__asBigIntCompare(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, JSC::EncodedJSValue JSValue2); CPP_DECL JSC::JSPromise* JSC__JSValue__asInternalPromise(JSC::EncodedJSValue JSValue0); CPP_DECL JSC::JSPromise* JSC__JSValue__asPromise(JSC::EncodedJSValue JSValue0); diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index 84e7d12b6d05..92df25427303 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -114,13 +114,9 @@ impl Clone for PathLike { } else { s.borrow() }), - Self::Buffer(b) => Self::Buffer(MarkedArrayBuffer { - buffer: b.buffer, - // The clone borrows the JS-owned backing store; only the - // original (if any) owns the allocation. - owns_buffer: false, - pinned: false, - }), + // The clone borrows the JS-owned backing store; only the + // original (if any) owns the allocation or the pin. + Self::Buffer(b) => Self::Buffer(b.borrow()), Self::SliceWithUnderlyingString(s) => { // `dupe_ref()` alone leaves `utf8` empty (lib.rs:1603) — a // cloned PathLike would then return b"" from `slice()`. Clone @@ -150,12 +146,8 @@ impl Drop for PathLike { // `CowSlice` frees its backing in its own `Drop` iff it owns it; // a borrowed path is a no-op. Self::String(_) => {} - Self::Buffer(b) => { - if b.pinned { - b.pinned = false; - b.buffer.unpin(); - } - } + // `MarkedArrayBuffer::Drop` releases the pin. + Self::Buffer(_) => {} Self::SliceWithUnderlyingString(s) | Self::ThreadsafeString(s) => { core::mem::take(s).deinit(); } @@ -208,7 +200,8 @@ impl PathLike { *self = Self::ThreadsafeString(owned); } Self::Buffer(b) => { - b.buffer.value.protect(); + b.bytes(); + b.value().protect(); } Self::String(_) | Self::ThreadsafeString(_) | Self::EncodedSlice(_) => {} } @@ -222,7 +215,8 @@ impl Unprotect for PathLike { #[inline] fn unprotect(&mut self) { if let Self::Buffer(b) = self { - b.buffer.value.unprotect(); + b.unpin(); + b.value().unprotect(); } } } diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 1b5c317c1e02..0682bb8f74d7 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -170,7 +170,7 @@ mod static_adapters { // re-enters the VM). let _a0_guard = a0.protected(); let _a1_guard = a1.protected(); - let mut output = if a1.is_undefined_or_null() { + let output = if a1.is_undefined_or_null() { None } else { StringOrBuffer::from_js(g, a1)? @@ -180,9 +180,6 @@ mod static_adapters { "expected string, buffer, TypedArray, or Blob", ))); }; - if let Some(StringOrBuffer::Buffer(buffer)) = &mut output { - buffer.buffer = ArrayBuffer::from_typed_array(g, buffer.buffer.value); - } Crypto::SHA512_256::hash_(g, &input, output) } } diff --git a/src/runtime/api/MarkdownObject.rs b/src/runtime/api/MarkdownObject.rs index 525f590bdc70..f68c944356fd 100644 --- a/src/runtime/api/MarkdownObject.rs +++ b/src/runtime/api/MarkdownObject.rs @@ -79,7 +79,7 @@ impl PinnedView { let Some(b) = buffer.buffer() else { return Ok(None); }; - match b.buffer.value.as_pinned_arraybuffer(global) { + match b.value().as_pinned_arraybuffer(global) { Some(pinned) => Ok(Some(Self(pinned))), None => Err(global.throw_out_of_memory()), } diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index 61d62db74118..075c558a7512 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -309,11 +309,9 @@ 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, - } + Ok(jsc::MarkedArrayBuffer::from_owned( + jsc::ArrayBuffer::from_owned_bytes(own, jsc::JSType::Uint8Array), + ) .to_node_buffer(global)) } _ => Ok(JSValue::UNDEFINED), diff --git a/src/runtime/crypto/CryptoHasher.rs b/src/runtime/crypto/CryptoHasher.rs index debf7bd6f0b6..dbbe1d0be01a 100644 --- a/src/runtime/crypto/CryptoHasher.rs +++ b/src/runtime/crypto/CryptoHasher.rs @@ -12,7 +12,7 @@ use bun_jsc::{ use crate::crypto::evp::{AlgorithmExt as _, EVP}; use crate::crypto::{HMAC, create_crypto_error, evp}; use crate::generated_classes::PropertyName; -use crate::node::{BlobOrStringOrBuffer, Encoding, StringOrBuffer}; +use crate::node::{BlobOrStringOrBuffer, Buffer, Encoding, StringOrBuffer}; use bun_sha_hmac::sha as hashers; // sha3/blake2 algorithms with no BoringSSL streaming context: @@ -271,7 +271,7 @@ impl CryptoHasher { }; // ?Node.StringOrBuffer (static-method arm: only `undefined` → None) - let mut output: Option = match next_eat() { + let output: Option = match next_eat() { Some(arg) => match StringOrBuffer::from_js(global, arg)? { Some(v) => Some(v), None => { @@ -294,9 +294,6 @@ impl CryptoHasher { ); } }; - if let Some(StringOrBuffer::Buffer(buffer)) = &mut output { - buffer.buffer = ArrayBuffer::from_typed_array(global, buffer.buffer.value); - } Self::hash_(global, algorithm, &input, output) } @@ -376,7 +373,7 @@ impl CryptoHasher { global: &JSGlobalObject, evp: &mut EVP, input: &BlobOrStringOrBuffer, - output: Option, + output: Option<&Buffer>, ) -> JsResult { let mut output_digest_buf: Digest = [0u8; EVP_MAX_MD_SIZE_USIZE]; let mut output_digest_slice: &mut [u8] = &mut output_digest_buf; @@ -388,20 +385,19 @@ impl CryptoHasher { ))); } - if let Some(output_buf) = &output { + if let Some(output_buf) = output { let size = evp.size() as usize; - let bytes_len = output_buf.byte_slice().len(); + let (ptr, bytes_len) = output_buf.bytes(); if bytes_len < size { return Err(global.throw_invalid_arguments(format_args!( "TypedArray must be at least {} bytes", size ))); } - // SAFETY: `output_buf.ptr` is the JSC-owned writable backing store + // SAFETY: `ptr` is the pinned JSC-owned writable backing store // (`bytes_len >= size` checked above; not detached since len > 0); - // borrowed for this frame only. Build the `&mut` directly from the - // raw `*mut u8` field — never via `&[u8].as_ptr()` (Stacked-Borrows UB). - output_digest_slice = unsafe { core::slice::from_raw_parts_mut(output_buf.ptr, size) }; + // borrowed for this frame only. + output_digest_slice = unsafe { core::slice::from_raw_parts_mut(ptr, size) }; } let Some(len) = evp.hash(boring_engine(global), input.slice(), output_digest_slice) else { @@ -412,7 +408,7 @@ impl CryptoHasher { }; if let Some(output_buf) = output { - Ok(output_buf.value) + Ok(output_buf.value()) } else { // Clone to GC-managed memory ArrayBuffer::create_buffer(global, &output_digest_slice[0..len as usize]) @@ -441,8 +437,7 @@ impl CryptoHasher { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; - return Self::hash_to_bytes(global, &mut evp, input, Some(ab)); + return Self::hash_to_bytes(global, &mut evp, input, Some(buffer)); } // `inline else => |*str|` — every non-buffer arm yields a string-like // `defer str.deinit()` — handled by Drop. @@ -690,8 +685,7 @@ impl CryptoHasher { ) -> JsResult { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; - return this.digest_to_bytes(global, Some(ab)); + return this.digest_to_bytes(global, Some(buffer)); } // `defer str.deinit()` — handled by Drop. let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { @@ -715,26 +709,22 @@ impl CryptoHasher { fn digest_to_bytes( &self, global: &JSGlobalObject, - output: Option, + output: Option<&Buffer>, ) -> JsResult { let mut output_digest_buf: evp::Digest = [0u8; EVP_MAX_MD_SIZE_USIZE]; let buf_len = output_digest_buf.len(); let output_digest_slice: &mut [u8]; - if let Some(output_buf) = &output { - let bytes_len = output_buf.byte_slice().len(); + if let Some(output_buf) = output { + let (ptr, bytes_len) = output_buf.bytes(); if bytes_len < buf_len { return Err(global.throw_invalid_arguments(format_args!( "TypedArray must be at least {} bytes", boring_ssl::EVP_MAX_MD_SIZE ))); } - // Reshaped for borrowck. - // SAFETY: `bytes_len >= EVP_MAX_MD_SIZE` checked above; `output_buf.ptr` - // is the JSC-owned writable backing store, outliving this frame. Build - // the `&mut` directly from the raw `*mut u8` field — never via - // `&[u8].as_ptr()` (Stacked-Borrows UB). - output_digest_slice = - unsafe { core::slice::from_raw_parts_mut(output_buf.ptr, bytes_len) }; + // SAFETY: `bytes_len >= EVP_MAX_MD_SIZE` checked above; `ptr` is the + // pinned JSC-owned writable backing store, outliving this frame. + output_digest_slice = unsafe { core::slice::from_raw_parts_mut(ptr, bytes_len) }; } else { output_digest_slice = &mut output_digest_buf; } @@ -748,7 +738,7 @@ impl CryptoHasher { } if let Some(output_buf) = output { - Ok(output_buf.value) + Ok(output_buf.value()) } else { // Clone to GC-managed memory ArrayBuffer::create_buffer(global, result) @@ -930,8 +920,7 @@ impl CryptoHasherZig { ) -> JsResult { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; - return Self::hash_by_name_inner_to_bytes::(global, input, Some(ab)); + return Self::hash_by_name_inner_to_bytes::(global, input, Some(buffer)); } let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { return Err(global @@ -983,7 +972,7 @@ impl CryptoHasherZig { fn hash_by_name_inner_to_bytes( global: &JSGlobalObject, input: &BlobOrStringOrBuffer, - output: Option, + output: Option<&Buffer>, ) -> JsResult { // `defer input.deinit()` — handled by Drop. @@ -996,26 +985,21 @@ impl CryptoHasherZig { let mut h = A::init(); let digest_length_comptime = A::DIGEST_LENGTH as usize; - if let Some(output_buf) = &output { - if output_buf.byte_slice().len() < digest_length_comptime { + h.update(input.slice()); + + if let Some(output_buf) = output { + let (ptr, bytes_len) = output_buf.bytes(); + if bytes_len < digest_length_comptime { return Err(global.throw_invalid_arguments(format_args!( "TypedArray must be at least {} bytes", digest_length_comptime ))); } - } - - h.update(input.slice()); - - if let Some(output_buf) = output { - // SAFETY: length checked above; `output_buf.ptr` is the JSC-owned - // writable backing store, outliving this frame. Build the `&mut` - // directly from the raw `*mut u8` field — never via `&[u8].as_ptr()` - // (Stacked-Borrows UB). - let out = - unsafe { core::slice::from_raw_parts_mut(output_buf.ptr, digest_length_comptime) }; + // SAFETY: length checked above; `ptr` is the pinned JSC-owned + // writable backing store, outliving this frame. + let out = unsafe { core::slice::from_raw_parts_mut(ptr, digest_length_comptime) }; h.final_(out); - Ok(output_buf.value) + Ok(output_buf.value()) } else { let mut out = [0u8; EVP_MAX_MD_SIZE_USIZE]; h.final_(&mut out[..digest_length_comptime]); @@ -1260,7 +1244,7 @@ impl StaticCryptoHasher { }; // ?Node.StringOrBuffer (static-method arm: only `undefined` → None) - let mut output: Option = match next_eat() { + let output: Option = match next_eat() { Some(arg) => match StringOrBuffer::from_js(global, arg)? { Some(v) => Some(v), None => { @@ -1283,9 +1267,6 @@ impl StaticCryptoHasher { ); } }; - if let Some(StringOrBuffer::Buffer(buffer)) = &mut output { - buffer.buffer = ArrayBuffer::from_typed_array(global, buffer.buffer.value); - } Self::hash_(global, &input, output) } @@ -1331,12 +1312,12 @@ impl StaticCryptoHasher { fn hash_to_bytes( global: &JSGlobalObject, input: &BlobOrStringOrBuffer, - output: Option, + output: Option<&Buffer>, ) -> JsResult { let mut output_digest_buf: H::Digest = H::new_digest(); let output_digest_slice: &mut H::Digest; - if let Some(output_buf) = &output { - let bytes_len = output_buf.byte_slice().len(); + if let Some(output_buf) = output { + let (ptr, bytes_len) = output_buf.bytes(); if bytes_len < H::DIGEST { return Err(global.throw_invalid_arguments(format_args!( "TypedArray must be at least {} bytes", @@ -1344,10 +1325,8 @@ impl StaticCryptoHasher { ))); } // SAFETY: `bytes_len >= H::DIGEST` checked above; `H::Digest = [u8; H::DIGEST]`; - // `output_buf.ptr` is the JSC-owned writable backing store. Build the - // `&mut` directly from the raw `*mut u8` field — never via - // `&[u8].as_ptr()` (Stacked-Borrows UB). - output_digest_slice = unsafe { &mut *output_buf.ptr.cast::() }; + // `ptr` is the pinned JSC-owned writable backing store. + output_digest_slice = unsafe { &mut *ptr.cast::() }; } else { output_digest_slice = &mut output_digest_buf; } @@ -1363,7 +1342,7 @@ impl StaticCryptoHasher { } if let Some(output_buf) = output { - Ok(output_buf.value) + Ok(output_buf.value()) } else { ArrayBuffer::create_uint8_array(global, output_digest_slice.as_ref()) } @@ -1384,8 +1363,7 @@ impl StaticCryptoHasher { if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; - return Self::hash_to_bytes(global, input, Some(ab)); + return Self::hash_to_bytes(global, input, Some(buffer)); } let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { return Err(global @@ -1474,8 +1452,7 @@ impl StaticCryptoHasher { } if let Some(string_or_buffer) = output { if let StringOrBuffer::Buffer(buffer) = &string_or_buffer { - let ab = buffer.buffer; - return this.digest_to_bytes(global, Some(ab)); + return this.digest_to_bytes(global, Some(buffer)); } let Some(encoding) = Encoding::from(string_or_buffer.slice()) else { return Err(global @@ -1498,12 +1475,12 @@ impl StaticCryptoHasher { fn digest_to_bytes( &self, global: &JSGlobalObject, - output: Option, + output: Option<&Buffer>, ) -> JsResult { let mut output_digest_buf: H::Digest = H::new_digest(); let output_digest_slice: &mut H::Digest; - if let Some(output_buf) = &output { - let bytes_len = output_buf.byte_slice().len(); + if let Some(output_buf) = output { + let (ptr, bytes_len) = output_buf.bytes(); if bytes_len < H::DIGEST { return Err(global.throw_invalid_arguments(format_args!( "TypedArray must be at least {} bytes", @@ -1511,10 +1488,8 @@ impl StaticCryptoHasher { ))); } // SAFETY: `bytes_len >= H::DIGEST`; `H::Digest = [u8; H::DIGEST]`; - // `output_buf.ptr` is the JSC-owned writable backing store. Build the - // `&mut` directly from the raw `*mut u8` field — never via - // `&[u8].as_ptr()` (Stacked-Borrows UB). - output_digest_slice = unsafe { &mut *output_buf.ptr.cast::() }; + // `ptr` is the pinned JSC-owned writable backing store. + output_digest_slice = unsafe { &mut *ptr.cast::() }; } else { output_digest_slice = &mut output_digest_buf; } @@ -1523,7 +1498,7 @@ impl StaticCryptoHasher { self.digested.set(true); if let Some(output_buf) = output { - Ok(output_buf.value) + Ok(output_buf.value()) } else { ArrayBuffer::create_uint8_array(global, output_digest_buf.as_ref()) } diff --git a/src/runtime/crypto/PBKDF2.rs b/src/runtime/crypto/PBKDF2.rs index 05030cafd414..ec70824474b1 100644 --- a/src/runtime/crypto/PBKDF2.rs +++ b/src/runtime/crypto/PBKDF2.rs @@ -2,8 +2,7 @@ use core::ffi::c_uint; use bun_boringssl_sys as boringssl; use bun_jsc::{ - AnyTaskJob, AnyTaskJobCtx, ArrayBuffer, CallFrame, JSGlobalObject, JSPromiseStrong, JSValue, - JsResult, + AnyTaskJob, AnyTaskJobCtx, CallFrame, JSGlobalObject, JSPromiseStrong, JSValue, JsResult, }; use crate::node::StringOrBuffer; @@ -216,10 +215,6 @@ impl PBKDF2 { } }; - if guard.salt.slice().len() > i32::MAX as usize { - return Err(global_this.throw_invalid_arguments(format_args!("salt is too long"))); - } - guard.password = match StringOrBuffer::from_js_maybe_async( global_this, arg0, @@ -236,14 +231,12 @@ impl PBKDF2 { } }; - if guard.password.slice().len() > i32::MAX as usize { - return Err(global_this.throw_invalid_arguments(format_args!("password is too long"))); + if guard.salt.slice().len() > i32::MAX as usize { + return Err(global_this.throw_invalid_arguments(format_args!("salt is too long"))); } - if !is_async { - if let StringOrBuffer::Buffer(buffer) = &mut guard.salt { - buffer.buffer = ArrayBuffer::from_typed_array(global_this, buffer.buffer.value); - } + if guard.password.slice().len() > i32::MAX as usize { + return Err(global_this.throw_invalid_arguments(format_args!("password is too long"))); } if is_async { diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 806b682bc967..d299f9b3d4af 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -5,8 +5,7 @@ use std::io::Write as _; use bun_core::ZigString; use bun_io::KeepAlive; use bun_jsc::{ - self as jsc, ArrayBuffer, CallFrame, JSFunction, JSGlobalObject, JSValue, JsError, JsResult, - WorkPoolTask, + self as jsc, CallFrame, JSFunction, JSGlobalObject, JSValue, JsError, JsResult, WorkPoolTask, }; // `bun_jsc::{AnyTask, ConcurrentTask, EventLoop}` are *modules* (re-exported from // `bun_event_loop`); pull the concrete types out by name. @@ -909,7 +908,7 @@ fn js_password_object_verify_sync( }; } - let Some(mut password) = StringOrBuffer::from_js(global_object, arguments[0])? else { + let Some(password) = StringOrBuffer::from_js(global_object, arguments[0])? else { return Err(global_object.throw_invalid_argument_type( "verify", "password", @@ -926,10 +925,6 @@ fn js_password_object_verify_sync( )); }; - if let StringOrBuffer::Buffer(buffer) = &mut password { - buffer.buffer = ArrayBuffer::from_typed_array(global_object, buffer.buffer.value); - } - // defer password.deinit() / hash_.deinit() — Drop at scope exit. if hash_.slice().is_empty() { diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index b52e71deb6c3..98027d5d2e0b 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -979,18 +979,12 @@ mod _impl { ctx.check_scrypt_params(global)?; - let mut ctx = scopeguard::ScopeGuard::into_inner(ctx); + let ctx = scopeguard::ScopeGuard::into_inner(ctx); if IS_ASYNC { return Ok((ctx, callback)); } - for input in [&mut ctx.password, &mut ctx.salt] { - if let StringOrBuffer::Buffer(buffer) = input { - buffer.buffer = ArrayBuffer::from_typed_array(global, buffer.buffer.value); - } - } - Ok((ctx, JSValue::UNDEFINED)) } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 55aa96e4bf4b..63bd9602e934 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -3550,11 +3550,7 @@ pub mod args { impl Default for MkdirTemp { fn default() -> Self { Self { - prefix: PathLike::Buffer(Buffer { - buffer: bun_jsc::ArrayBuffer::EMPTY, - owns_buffer: false, - pinned: false, - }), + prefix: PathLike::Buffer(Buffer::EMPTY), encoding: Encoding::Utf8, } } @@ -3904,15 +3900,6 @@ pub mod args { } } } - if arguments.will_be_async && matches!(args.buffer, StringOrBuffer::Buffer(_)) { - if let Some(pinned) = bv.as_pinned_arraybuffer(ctx) { - args.buffer = StringOrBuffer::Buffer(Buffer { - buffer: pinned, - owns_buffer: false, - pinned: true, - }); - } - } Ok(args) } } @@ -3923,22 +3910,18 @@ pub mod args { pub offset: u64, pub(crate) length: u64, pub(crate) position: Option, - /// True when `from_js` pinned `buffer` for the async path; balanced in - /// `unprotect()` (the JS-thread release hook). - pub(crate) pinned: bool, } impl Read { pub(crate) fn to_thread_safe(&self) { - self.buffer.buffer.value.protect(); + self.buffer.bytes(); + self.buffer.value().protect(); } } impl Unprotect for Read { #[inline] fn unprotect(&mut self) { - if self.pinned { - self.buffer.buffer.unpin(); - } - self.buffer.buffer.value.unprotect(); + self.buffer.unpin(); + self.buffer.value().unprotect(); } } impl Read { @@ -3995,7 +3978,6 @@ pub mod args { length: 0, offset: 0, position: None, - pinned: false, }); } @@ -4107,21 +4089,7 @@ pub mod args { None }; - let (buffer, pinned) = if arguments.will_be_async { - match buffer_value.as_pinned_arraybuffer(ctx) { - Some(pinned) => ( - Buffer { - buffer: pinned, - owns_buffer: false, - pinned: true, - }, - true, - ), - None => (buffer, false), - } - } else { - (buffer, false) - }; + let _ = buffer_value; Ok(Read { fd, @@ -4129,7 +4097,6 @@ pub mod args { offset, length, position, - pinned, }) } } @@ -6116,11 +6083,10 @@ impl NodeFS { fn read_inner(&mut self, args: &args::Read) -> Maybe { debug_assert!(args.position.is_none()); - // `ArrayBuffer` is a `Copy` descriptor over JSC-owned heap bytes; copy the - // descriptor locally and use the existing safe `byte_slice_mut` accessor - // instead of rebuilding a `&mut [u8]` from a `&[u8]` borrow by hand. - let mut view = args.buffer.buffer; - let mut buf = view.byte_slice_mut(); + let (ptr, len) = args.buffer.bytes(); + // SAFETY: `ptr`/`len` describe the pinned JSC-owned backing store + // (pinned in `Read::from_js`/`to_thread_safe`), writable for this I/O. + let mut buf = unsafe { bun_core::ffi::slice_mut(ptr, len) }; let off = (args.offset as usize).min(buf.len()); buf = &mut buf[off..]; let l = (args.length as usize).min(buf.len()); @@ -6134,9 +6100,9 @@ impl NodeFS { } fn pread_inner(&mut self, args: &args::Read) -> Maybe { - // See `read_inner` — copy the `ArrayBuffer` descriptor and use its safe accessor. - let mut view = args.buffer.buffer; - let mut buf = view.byte_slice_mut(); + let (ptr, len) = args.buffer.bytes(); + // SAFETY: see `read_inner`. + let mut buf = unsafe { bun_core::ffi::slice_mut(ptr, len) }; let off = (args.offset as usize).min(buf.len()); buf = &mut buf[off..]; let l = (args.length as usize).min(buf.len()); @@ -7289,11 +7255,7 @@ impl NodeFS { array_buffer.ensure_still_alive(); return match array_buffer.as_array_buffer(global) { Some(buffer) => Ok(ret::ReadFileWithOptions::Buffer( - bun_jsc::MarkedArrayBuffer { - buffer, - owns_buffer: false, - pinned: false, - }, + bun_jsc::MarkedArrayBuffer::from_unpinned(buffer), )), // This case shouldn't really happen. None => Err(with_path_like( diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 3c18a2543297..be97455d3d47 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -299,11 +299,8 @@ impl bun_jsc::Unprotect for StringOrBuffer { #[inline] fn unprotect(&mut self) { if let Self::Buffer(buffer) = self { - if buffer.pinned { - buffer.pinned = false; - buffer.buffer.unpin(); - } - buffer.buffer.value.unprotect(); + buffer.unpin(); + buffer.value().unprotect(); } } } @@ -319,7 +316,8 @@ impl StringOrBuffer { Self::ThreadsafeString(_) => {} Self::EncodedSlice(_) => {} Self::Buffer(buffer) => { - buffer.buffer.value.protect(); + buffer.bytes(); + buffer.value().protect(); } } } @@ -353,8 +351,9 @@ impl StringOrBuffer { result } Self::Buffer(buffer) => { - if buffer.buffer.value != JSValue::ZERO { - return Ok(buffer.buffer.value); + let value = buffer.value(); + if value != JSValue::ZERO { + return Ok(value); } Ok(buffer.to_node_buffer(ctx)) } @@ -437,15 +436,11 @@ impl StringOrBuffer { | JSType::BigInt64Array | JSType::BigUint64Array | JSType::DataView => { - let buffer = if is_async { - Buffer::from_js_pinned(global, value) - .unwrap_or_else(|| Buffer::from_array_buffer(global, value)) - } else { - Buffer::from_array_buffer(global, value) - }; + let buffer = Buffer::from_array_buffer(global, value); if is_async { - buffer.buffer.value.protect(); + buffer.bytes(); + buffer.value().protect(); } *out = Self::Buffer(buffer); @@ -508,14 +503,10 @@ impl StringOrBuffer { allow_string_object: bool, ) -> JsResult { if value.is_cell() && value.js_type().is_array_buffer_like() { - let buffer = if is_async { - Buffer::from_js_pinned(global, value) - .unwrap_or_else(|| Buffer::from_array_buffer(global, value)) - } else { - Buffer::from_array_buffer(global, value) - }; + let buffer = Buffer::from_array_buffer(global, value); if is_async { - buffer.buffer.value.protect(); + buffer.bytes(); + buffer.value().protect(); } *out = Self::Buffer(buffer); return Ok(true); @@ -1142,35 +1133,10 @@ impl PathLikeExt for PathLike { }; use jsc::JSType; match arg.js_type() { - JSType::Uint8Array | JSType::DataView => { - let mut buffer = Buffer::from_js_pinned(ctx, arg) - .unwrap_or_else(|| Buffer::from_typed_array(ctx, arg)); - if let Err(err) = Valid::path_buffer(&buffer, ctx) - .and_then(|_| Valid::path_null_bytes(buffer.slice(), ctx)) - { - if buffer.pinned { - buffer.pinned = false; - buffer.buffer.unpin(); - } - return Err(err); - } - - arguments.protect_eat(); - Ok(Some(Self::Buffer(buffer))) - } - - JSType::ArrayBuffer => { - let mut buffer = Buffer::from_js_pinned(ctx, arg) - .unwrap_or_else(|| Buffer::from_array_buffer(ctx, arg)); - if let Err(err) = Valid::path_buffer(&buffer, ctx) - .and_then(|_| Valid::path_null_bytes(buffer.slice(), ctx)) - { - if buffer.pinned { - buffer.pinned = false; - buffer.buffer.unpin(); - } - return Err(err); - } + JSType::Uint8Array | JSType::DataView | JSType::ArrayBuffer => { + let buffer = Buffer::from_array_buffer(ctx, arg); + Valid::path_buffer(&buffer, ctx) + .and_then(|_| Valid::path_null_bytes(buffer.slice(), ctx))?; arguments.protect_eat(); Ok(Some(Self::Buffer(buffer))) From 715ab0a9bdd7b348db27424b5bfd0d77d6bcc759 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:55:52 +0000 Subject: [PATCH 2/9] [autofix.ci] apply automated fixes --- src/jsc/array_buffer.rs | 6 +++++- src/runtime/api/bun/subprocess/Readable.rs | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 2c510f8a37cb..d4ecad354152 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -914,7 +914,11 @@ impl MarkedArrayBuffer { } pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { - Self::new(ArrayBuffer::from_bytes(bytes, typed_array_type), true, false) + Self::new( + ArrayBuffer::from_bytes(bytes, typed_array_type), + true, + false, + ) } pub const EMPTY: MarkedArrayBuffer = Self::new(ArrayBuffer::EMPTY, false, false); diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index 075c558a7512..60a4ea3a25ef 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -309,10 +309,13 @@ impl Readable { // Ownership of the mimalloc-backed buffer transfers to JSC // (freed via `MarkedArrayBuffer_deallocator`). - Ok(jsc::MarkedArrayBuffer::from_owned( - jsc::ArrayBuffer::from_owned_bytes(own, jsc::JSType::Uint8Array), + Ok( + jsc::MarkedArrayBuffer::from_owned(jsc::ArrayBuffer::from_owned_bytes( + own, + jsc::JSType::Uint8Array, + )) + .to_node_buffer(global), ) - .to_node_buffer(global)) } _ => Ok(JSValue::UNDEFINED), } From d6f3f593901639f42e5bf06ce46ae580aaeee003 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 12:05:51 +0000 Subject: [PATCH 3/9] test: redis.set observes a detached key buffer as zero-length Covers the lazy-pin guarantee on a call site that has no re-snapshot on main: js_valkey_functions::set converts the key (Buffer) then the value (StringObject -> toString), then serializes via .slice(). On main the key's 19 stale bytes are written to the wire; with lazy pin the first slice() observes the detached buffer and serializes a zero-length key. Also trims the longest new doc comments flagged by comment-cop. --- src/jsc/array_buffer.rs | 20 +++--------- src/jsc/bindings/bindings.cpp | 9 ++--- test/js/valkey/valkey-gc.test.ts | 56 ++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index d4ecad354152..183b588ac596 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -824,21 +824,11 @@ impl TypedArrayType { // MarkedArrayBuffer // ────────────────────────────────────────────────────────────────────────── -/// A byte-slice view over either a JS `ArrayBuffer`/typed array or a -/// Rust-owned allocation. -/// -/// For a JS-backed buffer the `(ptr, byte_len)` snapshot is taken lazily on -/// the first [`slice`]/[`bytes`] call: the backing `JSC::ArrayBuffer` is -/// pinned, its `vector()`/`byteLength()` is read once, and the result is -/// cached. Until then only the `JSValue` is meaningful, so user JS that runs -/// between construction and first use (argument coercion on a later -/// parameter) may freely `transfer()`/`resize()` the backing store and the -/// first `slice()` observes the post-coercion state. -/// -/// `Drop` releases the pin on the JS thread. A `MarkedArrayBuffer` that will -/// be dropped off the JS thread must be routed through the -/// [`crate::Unprotect`] hook (see `StringOrBuffer::unprotect`) which clears -/// the pin first so `Drop` becomes a no-op. +/// Byte view over a JS `ArrayBuffer`/view or a Rust-owned allocation. For a +/// JS-backed buffer the `(ptr, byte_len)` is taken lazily on the first +/// [`bytes`]/[`slice`] call: pin, read `vector()`/`byteLength()` once, cache. +/// `Drop` releases the pin (JS thread); off-thread users route through +/// [`crate::Unprotect`] which clears the pin first. #[derive(Default)] pub struct MarkedArrayBuffer { buffer: Cell, diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index dcd50363692c..934f5c0e3eb9 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3212,13 +3212,8 @@ bool JSC__JSValue__asArrayBuffer( return true; } -// Re-read the current vector()/byteLength() from a buffer-type cell without an -// exception scope. Caller guarantees `encodedValue` is a live -// JSArrayBuffer/JSArrayBufferView cell (type-checked at the point the -// Bun::ArrayBuffer descriptor was constructed). A detached buffer yields -// (nullptr, 0). Used by `MarkedArrayBuffer::slice()` to compute the byte view -// lazily so a later argument's toString()/valueOf() that transfers or resizes -// the buffer is observed instead of leaving a stale (ptr, len). +// Read the current vector()/byteLength() from a buffer-type cell (caller +// already type-checked it). Detached -> (nullptr, 0). No exception scope. CPP_DECL void JSC__JSValue__arrayBufferLiveBytes( JSC::EncodedJSValue encodedValue, uint8_t** out_ptr, size_t* out_byte_len) { diff --git a/test/js/valkey/valkey-gc.test.ts b/test/js/valkey/valkey-gc.test.ts index 4e57b472cae0..8fe9ba02c9f4 100644 --- a/test/js/valkey/valkey-gc.test.ts +++ b/test/js/valkey/valkey-gc.test.ts @@ -618,3 +618,59 @@ test.concurrent("getBuffer replies survive GC with adopted backing stores intact expect(proc.signalCode).toBeNull(); expect(exitCode).toBe(0); }); + +// MarkedArrayBuffer pins lazily: on the sync path StringOrBuffer::from_js +// captures only the JSValue, and the first slice() call pins + reads +// vector()/byteLength(). So when the second argument's toString() transfers +// the first argument's buffer before the command is serialized, the key is +// observed as zero-length rather than serialized from a stale (freed) pointer. +test.concurrent("redis.set reads a Buffer key only after every later argument has been coerced", async () => { + const src = ` + const net = require("node:net"); + let received = Buffer.alloc(0); + const server = net.createServer(sock => { + sock.on("data", chunk => { + received = Buffer.concat([received, chunk]); + // Reply +OK to whatever arrives so the client resolves. + sock.write("+OK\\r\\n"); + }); + }); + await new Promise(r => server.listen(0, r)); + const url = "redis://127.0.0.1:" + server.address().port; + + const client = new Bun.RedisClient(url, { enableAutoPipelining: false }); + await client.connect(); + + const key = Buffer.from(new ArrayBuffer(19)); + key.set(Buffer.from("lazy-pin-test:key-A")); + class DetachKey extends String { + toString() { + structuredClone(key.buffer, { transfer: [key.buffer] }); + Bun.gc(true); + return "the-value"; + } + } + await client.set(key, new DetachKey("x")); + client.close(); + server.close(); + + // RESP: *3\\r\\n$3\\r\\nSET\\r\\n$\\r\\n\\r\\n$9\\r\\nthe-value\\r\\n + const text = received.toString("latin1"); + const m = /\\$3\\r\\nSET\\r\\n\\$(\\d+)\\r\\n/.exec(text); + if (!m) throw new Error("no SET in wire bytes: " + JSON.stringify(text)); + console.log(JSON.stringify({ detached: key.byteLength === 0, keyLen: Number(m[1]) })); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(JSON.parse(stdout.trim())).toEqual({ detached: true, keyLen: 0 }); + expect(exitCode).toBe(0); +}); From bcb35b0d53a41bd7f6627797a3e6900fa9057664 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 12:08:31 +0000 Subject: [PATCH 4/9] node_fs: drop leftover no-op discard of buffer_value --- src/runtime/node/node_fs.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 63bd9602e934..04e0d18fcd8f 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4089,8 +4089,6 @@ pub mod args { None }; - let _ = buffer_value; - Ok(Read { fd, buffer, From 66a41751117f17803d14372ebda61de69eb80161 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 12:13:06 +0000 Subject: [PATCH 5/9] harden arrayBufferLiveBytes; extract to_thread_safe/unprotect helpers bindings.cpp: use dynamicDowncast and initialize outputs so an ObjectType/FinalObjectType buffer cell (matched by as_array_buffer's fallback arm) is handled correctly instead of reaching an uncheckedDowncast on the wrong shape. array_buffer.rs: add MarkedArrayBuffer::{to_thread_safe, unprotect} and route the five pin+protect / unpin+unprotect pairings in node_path.rs, types.rs, node_fs.rs through them. test: close the redis client/server in finally. --- src/jsc/array_buffer.rs | 15 +++++++++++++++ src/jsc/bindings/bindings.cpp | 22 +++++++++++++--------- src/jsc/node_path.rs | 6 ++---- src/runtime/node/node_fs.rs | 6 ++---- src/runtime/node/types.rs | 12 ++++-------- test/js/valkey/valkey-gc.test.ts | 25 ++++++++++++++----------- 6 files changed, 50 insertions(+), 36 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 183b588ac596..b5759f49dbec 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -945,6 +945,21 @@ impl MarkedArrayBuffer { } } + /// Pin-and-snapshot via [`bytes`] then `protect()` the JS value for a + /// threadpool hand-off. Paired with [`unprotect`]. + #[inline] + pub fn to_thread_safe(&self) { + self.bytes(); + self.value().protect(); + } + + /// Undo [`to_thread_safe`]. JS-thread only. + #[inline] + pub fn unprotect(&self) { + self.unpin(); + self.value().unprotect(); + } + /// Pin the backing `JSC::ArrayBuffer`, read its `vector()`/`byteLength()` /// once, and cache the result; subsequent calls return the cache. For a /// Rust-owned or already-pinned buffer this returns the existing diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 934f5c0e3eb9..b797cf4c849d 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3212,22 +3212,26 @@ bool JSC__JSValue__asArrayBuffer( return true; } -// Read the current vector()/byteLength() from a buffer-type cell (caller -// already type-checked it). Detached -> (nullptr, 0). No exception scope. +// Read the current vector()/byteLength() from a buffer-type cell. +// Detached or non-buffer -> (nullptr, 0). No exception scope. CPP_DECL void JSC__JSValue__arrayBufferLiveBytes( JSC::EncodedJSValue encodedValue, uint8_t** out_ptr, size_t* out_byte_len) { + *out_ptr = nullptr; + *out_byte_len = 0; JSC::JSValue value = JSC::JSValue::decode(encodedValue); - auto* cell = value.asCell(); - if (cell->type() == JSC::JSType::ArrayBufferType) { - auto* buffer = uncheckedDowncast(cell)->impl(); + if (!value.isCell()) [[unlikely]] + return; + if (auto* view = dynamicDowncast(value)) { + *out_ptr = static_cast(view->vector()); + *out_byte_len = view->byteLength(); + return; + } + if (auto* jsBuffer = dynamicDowncast(value)) { + auto* buffer = jsBuffer->impl(); *out_ptr = static_cast(buffer->data()); *out_byte_len = buffer->byteLength(); - return; } - auto* view = uncheckedDowncast(cell); - *out_ptr = static_cast(view->vector()); - *out_byte_len = view->byteLength(); } // Pin/unpin the backing ArrayBuffer of a JSArrayBuffer or JSArrayBufferView so diff --git a/src/jsc/node_path.rs b/src/jsc/node_path.rs index 92df25427303..20f42fd0e204 100644 --- a/src/jsc/node_path.rs +++ b/src/jsc/node_path.rs @@ -200,8 +200,7 @@ impl PathLike { *self = Self::ThreadsafeString(owned); } Self::Buffer(b) => { - b.bytes(); - b.value().protect(); + b.to_thread_safe(); } Self::String(_) | Self::ThreadsafeString(_) | Self::EncodedSlice(_) => {} } @@ -215,8 +214,7 @@ impl Unprotect for PathLike { #[inline] fn unprotect(&mut self) { if let Self::Buffer(b) = self { - b.unpin(); - b.value().unprotect(); + b.unprotect(); } } } diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 04e0d18fcd8f..1edc6179f9bd 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -3913,15 +3913,13 @@ pub mod args { } impl Read { pub(crate) fn to_thread_safe(&self) { - self.buffer.bytes(); - self.buffer.value().protect(); + self.buffer.to_thread_safe(); } } impl Unprotect for Read { #[inline] fn unprotect(&mut self) { - self.buffer.unpin(); - self.buffer.value().unprotect(); + self.buffer.unprotect(); } } impl Read { diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index be97455d3d47..e1625e34fb89 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -299,8 +299,7 @@ impl bun_jsc::Unprotect for StringOrBuffer { #[inline] fn unprotect(&mut self) { if let Self::Buffer(buffer) = self { - buffer.unpin(); - buffer.value().unprotect(); + buffer.unprotect(); } } } @@ -316,8 +315,7 @@ impl StringOrBuffer { Self::ThreadsafeString(_) => {} Self::EncodedSlice(_) => {} Self::Buffer(buffer) => { - buffer.bytes(); - buffer.value().protect(); + buffer.to_thread_safe(); } } } @@ -439,8 +437,7 @@ impl StringOrBuffer { let buffer = Buffer::from_array_buffer(global, value); if is_async { - buffer.bytes(); - buffer.value().protect(); + buffer.to_thread_safe(); } *out = Self::Buffer(buffer); @@ -505,8 +502,7 @@ impl StringOrBuffer { if value.is_cell() && value.js_type().is_array_buffer_like() { let buffer = Buffer::from_array_buffer(global, value); if is_async { - buffer.bytes(); - buffer.value().protect(); + buffer.to_thread_safe(); } *out = Self::Buffer(buffer); return Ok(true); diff --git a/test/js/valkey/valkey-gc.test.ts b/test/js/valkey/valkey-gc.test.ts index 8fe9ba02c9f4..7c9a8a830dde 100644 --- a/test/js/valkey/valkey-gc.test.ts +++ b/test/js/valkey/valkey-gc.test.ts @@ -639,20 +639,23 @@ test.concurrent("redis.set reads a Buffer key only after every later argument ha const url = "redis://127.0.0.1:" + server.address().port; const client = new Bun.RedisClient(url, { enableAutoPipelining: false }); - await client.connect(); - const key = Buffer.from(new ArrayBuffer(19)); - key.set(Buffer.from("lazy-pin-test:key-A")); - class DetachKey extends String { - toString() { - structuredClone(key.buffer, { transfer: [key.buffer] }); - Bun.gc(true); - return "the-value"; + try { + await client.connect(); + + key.set(Buffer.from("lazy-pin-test:key-A")); + class DetachKey extends String { + toString() { + structuredClone(key.buffer, { transfer: [key.buffer] }); + Bun.gc(true); + return "the-value"; + } } + await client.set(key, new DetachKey("x")); + } finally { + client.close(); + server.close(); } - await client.set(key, new DetachKey("x")); - client.close(); - server.close(); // RESP: *3\\r\\n$3\\r\\nSET\\r\\n$\\r\\n\\r\\n$9\\r\\nthe-value\\r\\n const text = received.toString("latin1"); From ba0290f4bb5c31f6107150c71a39631574290cc0 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 12:35:49 +0000 Subject: [PATCH 6/9] don't promote Fast/OversizeTypedArray on the sync pin path bytes() now only pins a view that already has a backing ArrayBuffer (hasArrayBuffer()); FastTypedArray and OversizeTypedArray have nothing to transfer() so there is nothing to pin against, and pinning them forces slowDownAndWasteMemory()/createAdopted which shows up in vm.heap.arrayBufferSize() (test-memory-usage.js). to_thread_safe() passes force_pin=true so off-thread borrows still promote. Also: route from_js/from_js_pinned through from_unpinned/from_pinned; add signalCode assertion to the valkey test. --- src/jsc/JSValue.rs | 8 ------ src/jsc/array_buffer.rs | 42 ++++++++++++++++++++------------ src/jsc/bindings/bindings.cpp | 31 ++++++++++++++++++----- src/jsc/bindings/headers.h | 2 +- test/js/valkey/valkey-gc.test.ts | 1 + 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 74ececea8709..3a30866ff15d 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -930,14 +930,6 @@ impl JSValue { } self.as_array_buffer(global) } - /// Pin the backing `JSC::ArrayBuffer` of this value so it cannot be - /// detached; see [`as_pinned_arraybuffer`]. Returns `false` if the value - /// has no backing `ArrayBuffer` impl. Release with - /// [`JSValue::unpin_array_buffer`]. - #[inline] - pub fn pin_array_buffer(self) -> bool { - JSC__JSValue__pinArrayBuffer(self) - } /// Generic downcast. Dispatches via [`JsClass::from_js`]. #[inline] pub fn as_(self) -> Option<*mut T> { diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index b5759f49dbec..378b53743790 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -136,14 +136,14 @@ unsafe extern "C" { safe fn JSC__ArrayBuffer__deref(self_: &JSCArrayBuffer); // safe: by-value `JSValue`; no-op for non-buffer values. safe fn JSC__JSValue__unpinArrayBuffer(v: JSValue); - // safe: by-value `JSValue` known (by caller) to be a buffer-type cell; - // `&mut *mut u8` / `&mut usize` are ABI-identical to non-null out-params - // the callee fills unconditionally. - safe fn JSC__JSValue__arrayBufferLiveBytes( + // safe: by-value `JSValue`; `&mut *mut u8` / `&mut usize` are + // ABI-identical to non-null out-params the callee fills unconditionally. + safe fn JSC__JSValue__pinAndReadArrayBufferBytes( v: JSValue, + force_pin: bool, out_ptr: &mut *mut u8, out_byte_len: &mut usize, - ); + ) -> bool; } impl JSValue { @@ -834,6 +834,8 @@ pub struct MarkedArrayBuffer { buffer: Cell, owns_buffer: Cell, pinned: Cell, + /// [`bytes`] has run; `buffer.ptr`/`byte_len` hold the post-coercion read. + settled: Cell, } impl MarkedArrayBuffer { @@ -843,6 +845,7 @@ impl MarkedArrayBuffer { buffer: Cell::new(buffer), owns_buffer: Cell::new(owns_buffer), pinned: Cell::new(pinned), + settled: Cell::new(owns_buffer || pinned), } } @@ -896,11 +899,11 @@ impl MarkedArrayBuffer { } pub fn from_js(global: &JSGlobalObject, value: JSValue) -> Option { - Some(Self::new(value.as_array_buffer(global)?, false, false)) + Some(Self::from_unpinned(value.as_array_buffer(global)?)) } pub fn from_js_pinned(global: &JSGlobalObject, value: JSValue) -> Option { - Some(Self::new(value.as_pinned_arraybuffer(global)?, false, true)) + Some(Self::from_pinned(value.as_pinned_arraybuffer(global)?)) } pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { @@ -945,11 +948,13 @@ impl MarkedArrayBuffer { } } - /// Pin-and-snapshot via [`bytes`] then `protect()` the JS value for a - /// threadpool hand-off. Paired with [`unprotect`]. + /// Pin-and-snapshot then `protect()` the JS value for a threadpool + /// hand-off. A FastTypedArray (which [`bytes`] leaves unpinned) is + /// promoted to a real `ArrayBuffer` here so its storage cannot move under + /// a worker thread. Paired with [`unprotect`]. #[inline] pub fn to_thread_safe(&self) { - self.bytes(); + self.bytes_::(); self.value().protect(); } @@ -963,22 +968,27 @@ impl MarkedArrayBuffer { /// Pin the backing `JSC::ArrayBuffer`, read its `vector()`/`byteLength()` /// once, and cache the result; subsequent calls return the cache. For a /// Rust-owned or already-pinned buffer this returns the existing - /// snapshot. A detached buffer pins nothing and yields `(null, 0)`. + /// snapshot. A FastTypedArray has no backing `ArrayBuffer` to detach so it + /// is not pinned; a detached buffer yields `(null, 0)`. /// /// Must run on the JS thread for the first call on a JS-backed buffer; /// callers that hand the buffer to a threadpool must call this (directly /// or via [`slice`]) before the hand-off. pub fn bytes(&self) -> (*mut u8, usize) { + self.bytes_::() + } + + fn bytes_(&self) -> (*mut u8, usize) { let mut ab = self.buffer.get(); - if self.pinned.get() || self.owns_buffer.get() || ab.value.is_empty() { + if (self.settled.get() && (!FORCE_PIN || self.pinned.get())) || ab.value.is_empty() { return (ab.ptr, ab.byte_len); } - if ab.value.pin_array_buffer() { - self.pinned.set(true); - } let mut ptr: *mut u8 = core::ptr::null_mut(); let mut len: usize = 0; - JSC__JSValue__arrayBufferLiveBytes(ab.value, &mut ptr, &mut len); + self.pinned.set(JSC__JSValue__pinAndReadArrayBufferBytes( + ab.value, FORCE_PIN, &mut ptr, &mut len, + )); + self.settled.set(true); ab.ptr = ptr; ab.byte_len = len; self.buffer.set(ab); diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index b797cf4c849d..7379833f0a8c 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3212,26 +3212,45 @@ bool JSC__JSValue__asArrayBuffer( return true; } -// Read the current vector()/byteLength() from a buffer-type cell. -// Detached or non-buffer -> (nullptr, 0). No exception scope. -CPP_DECL void JSC__JSValue__arrayBufferLiveBytes( - JSC::EncodedJSValue encodedValue, uint8_t** out_ptr, size_t* out_byte_len) +// Pin the backing ArrayBuffer (so it cannot be detached) and read its +// vector()/byteLength(). A FastTypedArray has no ArrayBuffer to detach so it +// is left unpinned (avoiding slowDownAndWasteMemory()) unless `force_pin`, +// which promotes it for an off-thread borrow. SharedArrayBuffer is never +// detachable and is left unpinned. Detached or non-buffer -> (nullptr, 0). +// Returns true iff a pin was taken that the caller must release with +// JSC__JSValue__unpinArrayBuffer. +CPP_DECL bool JSC__JSValue__pinAndReadArrayBufferBytes( + JSC::EncodedJSValue encodedValue, bool force_pin, uint8_t** out_ptr, size_t* out_byte_len) { *out_ptr = nullptr; *out_byte_len = 0; JSC::JSValue value = JSC::JSValue::decode(encodedValue); if (!value.isCell()) [[unlikely]] - return; + return false; if (auto* view = dynamicDowncast(value)) { + bool pinned = false; + if (force_pin || view->hasArrayBuffer()) { + if (auto* buf = view->possiblySharedBuffer(); buf && !buf->isShared()) { + buf->pin(); + pinned = true; + } + } *out_ptr = static_cast(view->vector()); *out_byte_len = view->byteLength(); - return; + return pinned; } if (auto* jsBuffer = dynamicDowncast(value)) { auto* buffer = jsBuffer->impl(); + bool pinned = false; + if (!buffer->isShared()) { + buffer->pin(); + pinned = true; + } *out_ptr = static_cast(buffer->data()); *out_byte_len = buffer->byteLength(); + return pinned; } + return false; } // Pin/unpin the backing ArrayBuffer of a JSArrayBuffer or JSArrayBufferView so diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index 892e8c401f1c..cee102f44d67 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -199,7 +199,7 @@ CPP_DECL uint32_t JSC__JSMap__size(JSC::JSMap* arg0, JSC::JSGlobalObject* arg1); CPP_DECL void JSC__JSValue__then(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, JSC::EncodedJSValue JSValue2, SYSV_ABI JSC::EncodedJSValue(* ArgFn3)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1), SYSV_ABI JSC::EncodedJSValue(* ArgFn4)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1)); CPP_DECL bool JSC__JSValue__asArrayBuffer(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, Bun__ArrayBuffer* arg2); -CPP_DECL void JSC__JSValue__arrayBufferLiveBytes(JSC::EncodedJSValue JSValue0, uint8_t** arg1, size_t* arg2); +CPP_DECL bool JSC__JSValue__pinAndReadArrayBufferBytes(JSC::EncodedJSValue JSValue0, bool arg1, uint8_t** arg2, size_t* arg3); CPP_DECL unsigned char JSC__JSValue__asBigIntCompare(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* arg1, JSC::EncodedJSValue JSValue2); CPP_DECL JSC::JSPromise* JSC__JSValue__asInternalPromise(JSC::EncodedJSValue JSValue0); CPP_DECL JSC::JSPromise* JSC__JSValue__asPromise(JSC::EncodedJSValue JSValue0); diff --git a/test/js/valkey/valkey-gc.test.ts b/test/js/valkey/valkey-gc.test.ts index 7c9a8a830dde..eeb75b3f57b8 100644 --- a/test/js/valkey/valkey-gc.test.ts +++ b/test/js/valkey/valkey-gc.test.ts @@ -675,5 +675,6 @@ test.concurrent("redis.set reads a Buffer key only after every later argument ha expect(stderr).toBe(""); expect(JSON.parse(stdout.trim())).toEqual({ detached: true, keyLen: 0 }); + expect(proc.signalCode).toBeNull(); expect(exitCode).toBe(0); }); From a144b529fc98615b8aec91fdbf67f718d576cdb3 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 12:54:26 +0000 Subject: [PATCH 7/9] delete dead from_pinned/from_js_pinned; fix stale SAFETY reference --- src/jsc/array_buffer.rs | 13 +------------ src/runtime/node/node_fs.rs | 4 ++-- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 378b53743790..e6d04dac7827 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -857,13 +857,6 @@ impl MarkedArrayBuffer { Self::new(ArrayBuffer::from_array_buffer(ctx, value), false, false) } - /// Wrap an already-pinned descriptor. Used by callers that pinned via - /// [`JSValue::as_pinned_arraybuffer`] themselves. - #[inline] - pub fn from_pinned(buffer: ArrayBuffer) -> MarkedArrayBuffer { - Self::new(buffer, false, true) - } - /// A non-owning view that neither owns the allocation nor the original's /// pin. [`bytes`] on the borrow takes its own pin (pin count is a /// counter), released by its own `Drop`. @@ -902,10 +895,6 @@ impl MarkedArrayBuffer { Some(Self::from_unpinned(value.as_array_buffer(global)?)) } - pub fn from_js_pinned(global: &JSGlobalObject, value: JSValue) -> Option { - Some(Self::from_pinned(value.as_pinned_arraybuffer(global)?)) - } - pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { Self::new( ArrayBuffer::from_bytes(bytes, typed_array_type), @@ -939,7 +928,7 @@ impl MarkedArrayBuffer { self.owns_buffer.get() } - /// Release the pin taken by [`bytes`]/[`from_js_pinned`] and clear the + /// Release the pin taken by [`bytes`]/[`to_thread_safe`] and clear the /// flag so `Drop` is a no-op. JS-thread only. #[inline] pub fn unpin(&self) { diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 1edc6179f9bd..eeb9d87da867 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -6080,8 +6080,8 @@ impl NodeFS { fn read_inner(&mut self, args: &args::Read) -> Maybe { debug_assert!(args.position.is_none()); let (ptr, len) = args.buffer.bytes(); - // SAFETY: `ptr`/`len` describe the pinned JSC-owned backing store - // (pinned in `Read::from_js`/`to_thread_safe`), writable for this I/O. + // SAFETY: `ptr`/`len` describe the JSC-owned backing store pinned by + // `bytes()` above (or by `to_thread_safe()` on the async path). let mut buf = unsafe { bun_core::ffi::slice_mut(ptr, len) }; let off = (args.offset as usize).min(buf.len()); buf = &mut buf[off..]; From f714d93bbcb81c4df26c7abe6d405758ca6a4644 Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 13:39:35 +0000 Subject: [PATCH 8/9] drop always-false pinned param from MarkedArrayBuffer::new; delete unused accessors --- src/jsc/array_buffer.rs | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index e6d04dac7827..129f4fdee128 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -840,21 +840,21 @@ pub struct MarkedArrayBuffer { impl MarkedArrayBuffer { #[inline] - const fn new(buffer: ArrayBuffer, owns_buffer: bool, pinned: bool) -> Self { + const fn new(buffer: ArrayBuffer, owns_buffer: bool) -> Self { Self { buffer: Cell::new(buffer), owns_buffer: Cell::new(owns_buffer), - pinned: Cell::new(pinned), - settled: Cell::new(owns_buffer || pinned), + pinned: Cell::new(false), + settled: Cell::new(owns_buffer), } } pub fn from_typed_array(ctx: &JSGlobalObject, value: JSValue) -> MarkedArrayBuffer { - Self::new(ArrayBuffer::from_typed_array(ctx, value), false, false) + Self::new(ArrayBuffer::from_typed_array(ctx, value), false) } pub fn from_array_buffer(ctx: &JSGlobalObject, value: JSValue) -> MarkedArrayBuffer { - Self::new(ArrayBuffer::from_array_buffer(ctx, value), false, false) + Self::new(ArrayBuffer::from_array_buffer(ctx, value), false) } /// A non-owning view that neither owns the allocation nor the original's @@ -862,21 +862,21 @@ impl MarkedArrayBuffer { /// counter), released by its own `Drop`. #[inline] pub fn borrow(&self) -> MarkedArrayBuffer { - Self::new(self.buffer.get(), false, false) + Self::new(self.buffer.get(), false) } /// Adopt a Rust-owned byte descriptor (freed by [`destroy`] / /// [`to_js`], not by `Drop`). #[inline] pub fn from_owned(buffer: ArrayBuffer) -> MarkedArrayBuffer { - Self::new(buffer, true, false) + Self::new(buffer, true) } /// Wrap a JS-backed descriptor (its `value` must be set). [`bytes`] pins /// on first access. #[inline] pub fn from_unpinned(buffer: ArrayBuffer) -> MarkedArrayBuffer { - Self::new(buffer, false, false) + Self::new(buffer, false) } pub fn from_string(str: &[u8]) -> Result { @@ -896,14 +896,10 @@ impl MarkedArrayBuffer { } pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { - Self::new( - ArrayBuffer::from_bytes(bytes, typed_array_type), - true, - false, - ) + Self::new(ArrayBuffer::from_bytes(bytes, typed_array_type), true) } - pub const EMPTY: MarkedArrayBuffer = Self::new(ArrayBuffer::EMPTY, false, false); + pub const EMPTY: MarkedArrayBuffer = Self::new(ArrayBuffer::EMPTY, false); /// Copy of the inner descriptor. `ptr`/`byte_len` reflect whatever the /// most recent [`bytes`] call cached (or the construction-time snapshot @@ -918,16 +914,6 @@ impl MarkedArrayBuffer { self.buffer.get().value } - #[inline] - pub fn is_pinned(&self) -> bool { - self.pinned.get() - } - - #[inline] - pub fn owns_buffer(&self) -> bool { - self.owns_buffer.get() - } - /// Release the pin taken by [`bytes`]/[`to_thread_safe`] and clear the /// flag so `Drop` is a no-op. JS-thread only. #[inline] From 0429346b3ec84fd59700773cef012c60728b750d Mon Sep 17 00:00:00 2001 From: robobun Date: Fri, 31 Jul 2026 15:07:02 +0000 Subject: [PATCH 9/9] ci: retrigger