Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 4 additions & 36 deletions src/jsc/CallFrame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use core::ffi::{c_uint, c_void};

use crate::virtual_machine::VirtualMachine;
use crate::{JSGlobalObject, JSValue};
use bun_collections::IntegerBitSet;
#[cfg(debug_assertions)]
use bun_core::ZStr;

Expand Down Expand Up @@ -229,6 +228,10 @@ pub struct CallerSrcLoc {
/// This is an advanced iterator struct which is used by various APIs. In
/// Node.fs, `will_be_async` is set to true which allows string/path APIs to
/// know if they have to do threadsafe clones.
///
/// It never roots anything: while the host call runs, the arguments are kept
/// alive by the caller's frame; whatever must outlive the call takes its own
/// hold (`to_thread_safe`).
pub struct ArgumentsSlice<'a> {
/// Backing storage for the remaining-args view. Both [`Self::init`] and
/// [`Self::init_async`] borrow — `all: &'a [JSValue]` already ties this
Expand All @@ -241,7 +244,6 @@ pub struct ArgumentsSlice<'a> {
remaining_start: usize,
pub vm: &'a VirtualMachine,
pub all: &'a [JSValue],
pub(crate) protected: IntegerBitSet<32>,
pub will_be_async: bool,
}

Expand All @@ -252,40 +254,12 @@ impl<'a> ArgumentsSlice<'a> {
&self.remaining_buf[self.remaining_start..]
}

pub(crate) fn unprotect(&mut self) {
let mut iter = self.protected.iterator::<true, true>();
while let Some(i) = iter.next() {
self.all[i].unprotect();
}
self.protected = IntegerBitSet::<32>::init_empty();
}

pub fn protect_eat(&mut self) {
if self.remaining().is_empty() {
return;
}
// `remaining_buf.len() == all.len()` for both init variants, so
// `all.len() - remaining().len()` reduces to `remaining_start`.
let index = self.all.len() - self.remaining().len();
self.protected.set(index);
self.all[index].protect();
self.eat();
}

pub fn protect_eat_next(&mut self) -> Option<JSValue> {
if self.remaining().is_empty() {
return None;
}
self.next_eat()
}

pub fn init(vm: &'a VirtualMachine, slice: &'a [JSValue]) -> ArgumentsSlice<'a> {
ArgumentsSlice {
remaining_buf: Cow::Borrowed(slice),
remaining_start: 0,
vm,
all: slice,
protected: IntegerBitSet::<32>::init_empty(),
will_be_async: false,
}
}
Expand Down Expand Up @@ -314,12 +288,6 @@ impl<'a> ArgumentsSlice<'a> {
}
}

impl<'a> Drop for ArgumentsSlice<'a> {
fn drop(&mut self) {
self.unprotect();
}
}

// `CallFrame`/`VM`/`JSGlobalObject` are opaque `UnsafeCell`-backed ZST handles;
// `&T` is ABI-identical to non-null `*const T`. Out-params are exclusive `&mut`
// to plain `#[repr(C)]` PODs. `describeFrame` returns a raw C string that the
Expand Down
75 changes: 75 additions & 0 deletions src/jsc/array_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,81 @@ unsafe impl bun_ptr::ExternalSharedDescriptor for JSCArrayBuffer {
}
}

unsafe extern "C" {
safe fn JSC__JSValue__retainPinnedArrayBuffer(
value: JSValue,
out_ptr: &mut *const u8,
out_len: &mut usize,
) -> *mut JSCArrayBuffer;
safe fn JSC__ArrayBuffer__retainPinned(self_: &JSCArrayBuffer);
safe fn JSC__ArrayBuffer__releasePinned(self_: &JSCArrayBuffer);
}

/// The byte range of a JS `ArrayBuffer` or view, kept alive and in place by
/// one ref + one pin held directly on its `JSC::ArrayBuffer` — the refcounted
/// owner of the storage, not a GC cell. The JS object may be collected while
/// this lives. Dropping touches no `JSCell`, so it may run inside a GC
/// finalizer; JS thread only (the refcount is not atomic).
pub struct PinnedArrayBuffer {
owner: ptr::NonNull<JSCArrayBuffer>,
ptr: *const u8,
len: usize,
}

impl PinnedArrayBuffer {
/// `None` if `value` is not a buffer/view or is detached. A view that has
/// no `ArrayBuffer` yet gets one (see `retainPinnedArrayBuffer`).
pub fn retain(value: JSValue) -> Option<Self> {
let mut ptr: *const u8 = ptr::null();
let mut len = 0usize;
let owner = ptr::NonNull::new(JSC__JSValue__retainPinnedArrayBuffer(
value, &mut ptr, &mut len,
))?;
Some(Self { owner, ptr, len })
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#[inline]
pub fn slice(&self) -> &[u8] {
if self.len == 0 {
return &[];
}
// SAFETY: `ptr[..len]` lies inside the storage `owner` keeps allocated
// (ref) and undetachable (pin) until `Drop`.
unsafe { core::slice::from_raw_parts(self.ptr, self.len) }
}
}

impl PinnedArrayBuffer {
/// The refcount is not atomic: a pool thread may read `slice()` but must
/// never clone or drop one of these.
#[inline]
fn assert_js_thread() {
debug_assert!(
crate::virtual_machine::VirtualMachine::get_or_null().is_some(),
"PinnedArrayBuffer cloned/dropped off the JS thread"
);
}
Comment thread
dylan-conway marked this conversation as resolved.
Outdated
}

impl Clone for PinnedArrayBuffer {
fn clone(&self) -> Self {
Self::assert_js_thread();
JSC__ArrayBuffer__retainPinned(JSCArrayBuffer::opaque_ref(self.owner.as_ptr()));
Self {
owner: self.owner,
ptr: self.ptr,
len: self.len,
}
}
}

impl Drop for PinnedArrayBuffer {
fn drop(&mut self) {
Self::assert_js_thread();
JSC__ArrayBuffer__releasePinned(JSCArrayBuffer::opaque_ref(self.owner.as_ptr()));
}
}

impl JSCArrayBuffer {
pub fn as_array_buffer(&mut self) -> ArrayBuffer {
let mut out = core::mem::MaybeUninit::<ArrayBuffer>::uninit();
Expand Down
46 changes: 46 additions & 0 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3548,6 +3548,52 @@ CPP_DECL void JSC__JSValue__unpinArrayBuffer(JSC::EncodedJSValue v)
buf->unpin();
}

// Hold the backing store itself rather than the JS object: one ref + one pin on
// the `JSC::ArrayBuffer` (refcounted, not a GC cell). The wrapper may then be
// collected while the bytes stay allocated and in place, and releasing touches
// no JSCell — so a native holder with no root to lean on, destroyed from a GC
// finalizer (a Blob store) or at an async op's completion, can release inline.
// JS thread only: the refcount is not atomic.
//
// Unlike `pinStorage`, a bufferless view is given its ArrayBuffer here
// (OversizeTypedArray: adopted in place, no byte copy) because there is no
// caller root keeping the view alive. `out_ptr`/`out_len` are the view's byte
// range, read after that so they point into the storage the ArrayBuffer owns.
CPP_DECL void JSC__ArrayBuffer__retainPinned(JSC::ArrayBuffer* buf)
{
buf->ref();
if (!buf->isShared())
buf->pin();
}
CPP_DECL JSC::ArrayBuffer* JSC__JSValue__retainPinnedArrayBuffer(JSC::EncodedJSValue v, const uint8_t** out_ptr, size_t* out_len)
{
auto value = JSC::JSValue::decode(v);
JSC::ArrayBuffer* buf = nullptr;
if (auto* jb = dynamicDowncast<JSC::JSArrayBuffer>(value)) {
buf = jb->impl();
if (!buf || buf->isDetached())
return nullptr;
*out_ptr = static_cast<const uint8_t*>(buf->data());
*out_len = buf->byteLength();
} else if (auto* view = dynamicDowncast<JSC::JSArrayBufferView>(value); view && !view->isDetached()) {
buf = view->possiblySharedBuffer();
if (!buf)
return nullptr;
*out_ptr = static_cast<const uint8_t*>(view->vector());
*out_len = view->byteLength();
} else {
return nullptr;
}
JSC__ArrayBuffer__retainPinned(buf);
return buf;
}
CPP_DECL void JSC__ArrayBuffer__releasePinned(JSC::ArrayBuffer* buf)
{
if (!buf->isShared())
buf->unpin();
buf->deref();
}

// Borrow `v`'s byte storage for off-thread reading. Splits out only the
// `FastTypedArray` case from `pinArrayBuffer`, because that's the one mode
// where `possiblySharedBuffer()` actually COPIES data
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ pub use self::js_value::{
// and is wired into `event_loop::tick` directly at link time. No fn-pointer
// hook is re-exported from the crate root.
pub use self::array_buffer::{
ArrayBuffer, BinaryType, JSCArrayBuffer, MarkedArrayBuffer, TypedArrayType,
ArrayBuffer, BinaryType, JSCArrayBuffer, MarkedArrayBuffer, PinnedArrayBuffer, TypedArrayType,
};
pub use self::console_object as ConsoleObject;
pub use self::console_object::Formatter;
Expand Down
61 changes: 36 additions & 25 deletions src/jsc/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use bun_core::{SliceWithUnderlyingString, ZigStringSlice};
use bun_ptr::cow_slice::CowSlice;
use bun_sys::Fd;

use crate::array_buffer::MarkedArrayBuffer;
use crate::array_buffer::{MarkedArrayBuffer, PinnedArrayBuffer};

// ──────────────────────────────────────────────────────────────────────────
// RAII for `protect()`/`unprotect()` pairs taken by `to_thread_safe()`.
Expand All @@ -24,8 +24,8 @@ use crate::array_buffer::MarkedArrayBuffer;
// every early return between `to_thread_safe` and the manual cleanup.
// ──────────────────────────────────────────────────────────────────────────

/// Undo the `JSValue::protect()` calls taken by [`to_thread_safe`](
/// PathLike::to_thread_safe) (or an `args::*` type's `to_thread_safe`).
/// Undo the `JSValue::protect()` calls taken by an `args::*` type's
/// `to_thread_safe` (e.g. `StringOrBuffer::Buffer`).
///
/// Implementations release **only** the JS-GC protect refcount — owned Rust
/// payloads (Vec, `SliceWithUnderlyingString`, …) are freed by the type's own
Expand Down Expand Up @@ -100,7 +100,15 @@ impl<T: Unprotect + Default> Default for ThreadSafe<T> {
/// `node.PathLike`.
pub enum PathLike {
String(CowSlice<u8>),
/// A JS buffer borrowed for the duration of one call: the argument keeps
/// the cell alive and the pin keeps its storage in place. Anything held
/// past the call goes through [`PathLike::to_thread_safe`] first.
Buffer(MarkedArrayBuffer),
/// A `Buffer` after `to_thread_safe`: the same bytes, kept by a ref + pin
/// on the backing `JSC::ArrayBuffer` instead of by the JS cell, so it can
/// be released from a GC finalizer (a `Blob` store's path) as well as
/// from an async op's completion.
PinnedBuffer(PinnedArrayBuffer),
SliceWithUnderlyingString(SliceWithUnderlyingString),
ThreadsafeString(SliceWithUnderlyingString),
EncodedSlice(ZigStringSlice),
Expand Down Expand Up @@ -133,6 +141,7 @@ impl Clone for PathLike {
owns_buffer: false,
pinned: false,
}),
Self::PinnedBuffer(b) => Self::PinnedBuffer(b.clone()),
Comment thread
dylan-conway marked this conversation as resolved.
Outdated
Self::SliceWithUnderlyingString(s) => {
// `dupe_ref()` alone leaves `utf8` empty (lib.rs:1603) — a
// cloned PathLike would then return b"" from `slice()`. Clone
Expand Down Expand Up @@ -171,9 +180,8 @@ impl Drop for PathLike {
Self::SliceWithUnderlyingString(s) | Self::ThreadsafeString(s) => {
core::mem::take(s).deinit();
}
// `ZigStringSlice` releases its WTF ref / owned buffer in its own
// `Drop`.
Self::EncodedSlice(_) => {}
// `PinnedArrayBuffer` / `ZigStringSlice` release in their own `Drop`.
Self::PinnedBuffer(_) | Self::EncodedSlice(_) => {}
}
}
}
Expand All @@ -189,6 +197,7 @@ impl PathLike {
match self {
Self::String(s) => s.slice(),
Self::Buffer(b) => b.slice(),
Self::PinnedBuffer(b) => b.slice(),
Self::SliceWithUnderlyingString(s) | Self::ThreadsafeString(s) => s.slice(),
Self::EncodedSlice(s) => s.slice(),
}
Expand All @@ -198,20 +207,18 @@ impl PathLike {
match self {
Self::String(s) => s.length(),
Self::Buffer(b) => b.slice().len(),
Self::PinnedBuffer(b) => b.slice().len(),
Self::SliceWithUnderlyingString(_) | Self::ThreadsafeString(_) => 0,
Self::EncodedSlice(s) => s.slice().len(),
}
}

/// Promote any borrowed-JS
/// payload to a thread-safe representation. For `Buffer` the variant is
/// kept and the backing JS value is `protect()`ed (paired with
/// [`Unprotect::unprotect`]); the discriminant is preserved so callers
/// matching on `Buffer` after this call see the same shape.
///
/// Prefer [`Self::into_thread_safe`] which returns a [`ThreadSafe`] guard;
/// this in-place form exists for nested calls from container types'
/// `to_thread_safe`.
/// Promote any payload that is only valid for the current call into one
/// that can be held past it, read from a work-pool thread, and released on
/// the JS thread wherever the holder happens to die — an async op's
/// completion, or a `Blob` store dropped from its cell's GC finalizer.
/// Zero-copy: a `Buffer` keeps borrowing the same bytes, now owned via the
/// backing store (`PinnedBuffer`) rather than the JS object.
pub fn to_thread_safe(&mut self) {
match self {
Self::SliceWithUnderlyingString(s) => {
Expand All @@ -220,23 +227,27 @@ impl PathLike {
*self = Self::ThreadsafeString(owned);
}
Self::Buffer(b) => {
b.buffer.value.protect();
// Dropping the `Buffer` arm afterwards releases its own pin.
*self = match PinnedArrayBuffer::retain(b.buffer.value) {
Some(pinned) => Self::PinnedBuffer(pinned),
// Detached: there are no bytes to keep.
None => Self::default(),
};
}
Self::String(_) | Self::ThreadsafeString(_) | Self::EncodedSlice(_) => {}
Self::String(_)
| Self::PinnedBuffer(_)
| Self::ThreadsafeString(_)
| Self::EncodedSlice(_) => {}
}
}
}

impl Unprotect for PathLike {
/// JS-side half of cleanup — undo
/// the `protect()` taken by [`Self::to_thread_safe`] /
/// `ArgumentsSlice::protect_eat`. Owned payloads are released by `Drop`.
/// Nothing to release: [`Self::to_thread_safe`] holds the backing store,
/// not a `protect()`ed cell. Kept so container `args::*` types can forward
/// uniformly.
#[inline]
fn unprotect(&mut self) {
if let Self::Buffer(b) = self {
b.buffer.value.unprotect();
}
}
fn unprotect(&mut self) {}
}

/// `node.PathOrFileDescriptor`.
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ fn do_resolve(global_this: &JSGlobalObject, arguments: &[JSValue]) -> JsResult<J
// SAFETY: bun_vm() returns the live per-thread singleton.
let vm = global_this.bun_vm();
let mut args = ArgumentsSlice::init(vm, arguments);
let Some(specifier) = args.protect_eat_next() else {
let Some(specifier) = args.next_eat() else {
return Err(global_this
.throw_invalid_arguments(format_args!("Expected a specifier and a from path")));
};
Expand All @@ -1104,7 +1104,7 @@ fn do_resolve(global_this: &JSGlobalObject, arguments: &[JSValue]) -> JsResult<J
return Err(global_this.throw_invalid_arguments(format_args!("specifier must be a string")));
}

let Some(from) = args.protect_eat_next() else {
let Some(from) = args.next_eat() else {
return Err(global_this.throw_invalid_arguments(format_args!("Expected a from path")));
};

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/node/node_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2716,7 +2716,7 @@ pub mod args {
let fd = FD::from_js_required(ctx, arguments)?;
let buffers = VectorArrayBuffer::from_js(
ctx,
arguments.protect_eat_next().ok_or_else(|| {
arguments.next_eat().ok_or_else(|| {
ctx.throw_invalid_arguments(format_args!("Expected an ArrayBufferView[]"))
})?,
// The iovec pointers outlive this call on the async path; root
Expand Down
Loading
Loading