From 9894e918c5c81a8ec706c63ad93caa88998d5c4c Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 07:11:41 +0000 Subject: [PATCH 01/11] bun_ptr: RefPtr::new_cyclic + SelfRoot; BackRef::dangling only for Shared/Mut new_cyclic builds a refcounted value that stores its own root pointer as an opaque SelfRoot; the token can only be turned into a ThisPtr through the constructed &T, so it cannot be followed early or late. A Root back-reference can hand out ThisPtrs, so BackRef::dangling() is limited to Shared/Mut. --- src/ptr/lib.rs | 43 +++++++++++++++++++++++++++- src/ptr/ref_count.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/src/ptr/lib.rs b/src/ptr/lib.rs index bfc818cbd971..303ccd736d65 100644 --- a/src/ptr/lib.rs +++ b/src/ptr/lib.rs @@ -162,7 +162,48 @@ impl BackRef { } } -impl BackRef { +/// The root pointer of a value built by [`RefPtr::new_cyclic`], stored inside +/// that value. It has no accessors of its own: the only way to use it is +/// [`SelfRoot::this_ptr`], which takes the enclosing `&T` — so it cannot be +/// followed before the value exists or after it is gone. +#[repr(transparent)] +pub struct SelfRoot(pub(crate) core::ptr::NonNull); + +impl SelfRoot { + /// The enclosing value as a [`ThisPtr`]. `owner` must be the value this + /// token is stored in (checked). + #[inline] + pub fn this_ptr(&self, owner: &T) -> ThisPtr { + assert!( + core::ptr::eq(self.0.as_ptr().cast_const(), owner), + "SelfRoot used from a value it does not belong to" + ); + // SAFETY: `owner` is a live `&T` at `self.0` (asserted), so the value is + // constructed; `self.0` keeps the allocation-root provenance a last + // release needs. + unsafe { ThisPtr::new(self.0.as_ptr()) } + } + + /// The enclosing value as a root back-reference. + #[inline] + pub fn backref(&self, owner: &T) -> BackRef { + self.this_ptr(owner).into() + } +} + +/// Provenance markers a placeholder [`BackRef::dangling`] may carry. Not +/// [`Root`]: a `Root` back-reference can hand out a [`ThisPtr`], so it is +/// only ever minted from a real one (see [`RefPtr::new_cyclic`]). +pub trait DanglingOk: sealed::Sealed {} +impl DanglingOk for Shared {} +impl DanglingOk for Mut {} +mod sealed { + pub trait Sealed {} + impl Sealed for super::Shared {} + impl Sealed for super::Mut {} +} + +impl BackRef { #[inline] pub const fn dangling() -> Self { BackRef(core::ptr::NonNull::dangling(), core::marker::PhantomData) diff --git a/src/ptr/ref_count.rs b/src/ptr/ref_count.rs index 646a5f68866e..8d098be53ddc 100644 --- a/src/ptr/ref_count.rs +++ b/src/ptr/ref_count.rs @@ -545,6 +545,21 @@ impl RefPtr { /// /// # Safety /// `raw_ptr` must point to a live `T`. + /// [`new`](Self::new) for a `T` that stores its own root pointer (to hand + /// out [`ThisPtr`](crate::ThisPtr)s from `&self` entry points). `init` + /// receives a [`SelfRoot`](crate::SelfRoot) to store in the value; the + /// token cannot be dereferenced on its own, only through the `&T` that + /// exists once construction is done. + pub fn new_cyclic(init: impl FnOnce(crate::SelfRoot) -> T) -> Self { + let raw: NonNull = bun_core::heap::into_raw_nn(Box::::new_uninit()).cast::(); + let value = init(crate::SelfRoot(raw)); + // SAFETY: `raw` is a live, uninitialized, properly aligned `T` slot. + unsafe { raw.as_ptr().write(value) }; + // SAFETY: freshly written, so live. + debug_assert!(unsafe { T::rc_has_one_ref(raw.as_ptr()) }); + Self(raw) + } + #[inline] pub unsafe fn init_ref(raw_ptr: *mut T) -> Self { // SAFETY: caller contract @@ -975,4 +990,57 @@ mod tests { assert_eq!(type_base_name("a::b::Foo"), "Foo"); assert_eq!(type_base_name("Foo"), "Foo"); } + + struct Cyclic { + ref_count: RefCount, + self_ref: crate::SelfRoot, + payload: Box, + } + + impl Drop for Cyclic { + fn drop(&mut self) { + DROPS.fetch_add(1, Ordering::SeqCst); + } + } + + impl RefCounted for Cyclic { + unsafe fn get_ref_count(this: *mut Self) -> *mut RefCount { + // SAFETY: caller contract — field projection on a live allocation. + unsafe { &raw mut (*this).ref_count } + } + unsafe fn destructor(this: *mut Self) { + // SAFETY: caller contract — refcount hit zero, sole owner. + drop(unsafe { bun_core::heap::take(this) }); + } + } + + #[test] + fn new_cyclic_self_root_round_trip() { + let _serial = serial(); + let before = drops(); + let p = RefPtr::new_cyclic(|self_ref| Cyclic { + ref_count: RefCount::init(), + self_ref, + payload: Box::new(9), + }); + assert_eq!(*p.payload, 9); + { + // The token hands back a `ThisPtr` to the same allocation; a ref + // taken through it bumps and releases the count. + let this = p.self_ref.this_ptr(&*p); + assert_eq!(this.as_ptr().cast_const(), p.as_ptr().cast_const()); + let _guard = RefPtr::from_this(this); + assert_eq!(*this.payload, 9); + assert_eq!(p.ref_count.get(), 2); + } + assert_eq!(p.ref_count.get(), 1); + assert_eq!(drops(), before); + // Last release goes through the root the token stored. + let this = p.self_ref.this_ptr(&*p); + let guard = RefPtr::from_this(this); + drop(p); + assert_eq!(drops(), before); + drop(guard); + assert_eq!(drops(), before + 1); + } } From f05da9e58225a902612a1d1ddc4a9376da1ffb14 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 07:20:24 +0000 Subject: [PATCH 02/11] ref_count: init_ref keeps its own doc block --- src/ptr/ref_count.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ptr/ref_count.rs b/src/ptr/ref_count.rs index 8d098be53ddc..7c530fc3500c 100644 --- a/src/ptr/ref_count.rs +++ b/src/ptr/ref_count.rs @@ -541,10 +541,6 @@ impl RefPtr { Self(ptr) } - /// Take a new ref on `*raw_ptr`. - /// - /// # Safety - /// `raw_ptr` must point to a live `T`. /// [`new`](Self::new) for a `T` that stores its own root pointer (to hand /// out [`ThisPtr`](crate::ThisPtr)s from `&self` entry points). `init` /// receives a [`SelfRoot`](crate::SelfRoot) to store in the value; the @@ -560,6 +556,10 @@ impl RefPtr { Self(raw) } + /// Take a new ref on `*raw_ptr`. + /// + /// # Safety + /// `raw_ptr` must point to a live `T`. #[inline] pub unsafe fn init_ref(raw_ptr: *mut T) -> Self { // SAFETY: caller contract From 93483c68c783088aa7e491e7984ea6158b7b9e15 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 07:23:43 +0000 Subject: [PATCH 03/11] cron: CronJob stores a SelfRoot (RefPtr::new_cyclic) instead of a patched-in Root back-reference --- src/runtime/api/cron.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/runtime/api/cron.rs b/src/runtime/api/cron.rs index ae57dd6065f1..5bff13462b79 100644 --- a/src/runtime/api/cron.rs +++ b/src/runtime/api/cron.rs @@ -28,7 +28,7 @@ use bun_jsc::{ JSGlobalObject, JSObject, JSValue, JsCell, JsRef, JsResult, }; use bun_paths as path; -use bun_ptr::{BackRef, RefPtr, ThisPtr}; +use bun_ptr::{RefPtr, ThisPtr}; use bun_resolver::fs::FileSystem; #[cfg(not(target_os = "macos"))] use bun_resolver::fs::RealFS; @@ -1372,7 +1372,7 @@ pub struct CronJob { ref_count: Cell, /// Set from the allocating `RefPtr` so `&self` host fns can reach the /// `ThisPtr`-taking paths that may release refs. - self_ref: Cell>, + self_ref: bun_ptr::SelfRoot, // pub: `bun_core::from_field_ptr!(CronJob, event_loop_timer)` needs `offset_of!` visibility. // `JsCell` is `#[repr(transparent)]`, so the byte offset of the inner // `EventLoopTimer` is identical and the dispatch.rs `owner!` macro works @@ -1654,7 +1654,7 @@ impl CronJob { #[bun_jsc::host_fn(method)] pub(crate) fn stop(&self, _global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - Self::self_stop(self.self_ref.get().this_ptr(), self.global.bun_vm()); + Self::self_stop(self.self_ref.this_ptr(self), self.global.bun_vm()); Ok(frame.this()) } @@ -1708,9 +1708,9 @@ impl CronJob { let vm = global.bun_vm().as_mut(); - let job = RefPtr::new(CronJob { + let job = RefPtr::new_cyclic(|self_ref| CronJob { ref_count: Cell::new(1), - self_ref: Cell::new(BackRef::dangling()), + self_ref, event_loop_timer: JsCell::new(EventLoopTimer::init_paused(EventLoopTimerTag::CronJob)), global: GlobalRef::from(global), parsed, @@ -1722,7 +1722,6 @@ impl CronJob { pending_ref: JsCell::new(None), in_fire: Cell::new(false), }); - job.self_ref.set(BackRef::from(job.this_ptr())); let Some(next_time) = job.compute_next_timespec() else { return Err(global.throw_invalid_arguments(format_args!( From 19307ec754d91d4fef39c9445a49446f2e27becb Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 07:12:02 +0000 Subject: [PATCH 04/11] fetch: take FetchTasklet and fetch.rs to zero unsafe --- src/boringssl_sys/boringssl.rs | 31 + src/event_loop/ConcurrentTask.rs | 92 +- src/event_loop/lib.rs | 2 +- src/http/AsyncHTTP.rs | 198 +- src/http/HTTPRequestBody.rs | 11 +- src/http/HTTPThread.rs | 31 +- src/http/Signals.rs | 4 +- src/http/ThreadSafeStreamBuffer.rs | 111 +- src/http/h2_client/encode.rs | 5 +- src/http/lib.rs | 222 +- src/install/NetworkTask.rs | 8 +- src/jsc/AbortSignal.rs | 54 + src/jsc/FetchHeaders.rs | 95 + src/jsc/JSValue.rs | 9 + src/jsc/VmHandle.rs | 75 +- src/jsc/Weak.rs | 8 +- src/jsc/event_loop.rs | 4 + src/jsc/lib.rs | 8 +- src/picohttp/lib.rs | 8 + src/ptr/js_cell.rs | 6 + src/ptr/lib.rs | 58 + src/runtime/cli/run_command.rs | 2 +- src/runtime/dispatch.rs | 57 +- src/runtime/jsc_hooks.rs | 8 +- src/runtime/server/RequestContext.rs | 7 +- src/runtime/webcore/Blob.rs | 7 +- src/runtime/webcore/Body.rs | 24 +- src/runtime/webcore/ByteStream.rs | 57 +- src/runtime/webcore/ReadableStream.rs | 168 +- src/runtime/webcore/Response.rs | 184 +- src/runtime/webcore/fetch.rs | 237 +- .../webcore/fetch/FetchRequestBodySink.rs | 158 +- src/runtime/webcore/fetch/FetchTasklet.rs | 2399 ++++++++--------- src/runtime/webcore/s3/client.rs | 18 +- src/runtime/webcore/s3/simple_request.rs | 5 +- src/runtime/webcore/streams.rs | 62 +- 36 files changed, 2444 insertions(+), 1989 deletions(-) diff --git a/src/boringssl_sys/boringssl.rs b/src/boringssl_sys/boringssl.rs index afcf6b212db3..79be1d82c9f4 100644 --- a/src/boringssl_sys/boringssl.rs +++ b/src/boringssl_sys/boringssl.rs @@ -305,6 +305,37 @@ impl Drop for OwnedSslCtx { } } +/// Owns one `X509`; `X509_free`s it on drop. +#[repr(transparent)] +pub struct OwnedX509(core::ptr::NonNull); + +impl OwnedX509 { + /// Parses one DER-encoded certificate; `None` if it does not parse. + pub fn from_der(der: &[u8]) -> Option { + let len = c_long::try_from(der.len()).ok()?; + let mut ptr = der.as_ptr(); + // SAFETY: `d2i_X509` reads at most `len` bytes from `ptr`, which `der` covers. + let raw = unsafe { d2i_X509(core::ptr::null_mut(), &raw mut ptr, len) }; + core::ptr::NonNull::new(raw).map(Self) + } + + pub fn as_ptr(&self) -> *mut X509 { + self.0.as_ptr() + } + + pub fn as_mut(&mut self) -> &mut X509 { + // SAFETY: we own the live `X509` for as long as `self` is borrowed. + unsafe { self.0.as_mut() } + } +} + +impl Drop for OwnedX509 { + fn drop(&mut self) { + // SAFETY: we own exactly one reference, released once. + unsafe { X509_free(self.0.as_ptr()) } + } +} + /// Owns the `STACK_OF(GENERAL_NAME)` that `X509V3_EXT_d2i` returns for a /// subjectAltName extension. Frees every `GENERAL_NAME` and then the stack. pub struct GeneralNames(core::ptr::NonNull); diff --git a/src/event_loop/ConcurrentTask.rs b/src/event_loop/ConcurrentTask.rs index 245c9500ea3c..e7c5cb3067d3 100644 --- a/src/event_loop/ConcurrentTask.rs +++ b/src/event_loop/ConcurrentTask.rs @@ -72,9 +72,10 @@ pub mod task_tag { Close, CppTask, DuplexUpgradeContext, - FetchTasklet, - FetchTaskletDeinit, + FetchTasklet, // progress update (bun_runtime FetchTasklet) + FetchTaskletHandBack, // the HTTP thread is done with the fetch FetchTaskletPromiseSettle, + FetchTaskletRequestDataDrain, // the streaming request body buffer drained FSWatchTask, GetAddrInfoLibuvComplete, HotReloadTask, @@ -132,6 +133,25 @@ pub struct Task { pub ptr: *mut (), } +/// A task whose queued pointer is a [`ThisPtr`](bun_ptr::ThisPtr) to +/// `Target`, which keeps itself alive for the task; the dispatcher runs it or +/// releases it through here. Implemented by a zero-sized hop type per tag so +/// the ref protocol lives next to `Target`. +pub trait TaskHop { + type Target; + /// The tag constant from [`task_tag`]; the `bun_runtime::dispatch` match + /// arms MUST agree. + const TAG: TaskTag; + fn run(this: bun_ptr::ThisPtr) -> crate::JsResult<()>; + /// As [`Taskable::release_unrun`]. + fn release_unrun(this: bun_ptr::ThisPtr); + + #[inline] + fn task(this: bun_ptr::ThisPtr) -> Task { + Task::new(Self::TAG, this.as_ptr().cast::<()>()) + } +} + /// What it takes to be queued as a [`Task`]: a tag, and how the task is /// freed when it will never run. Implement on every type that can be /// enqueued; the impl lives in whatever crate owns the type. @@ -216,6 +236,10 @@ pub struct ConcurrentTask { /// dispatch. Immutable after construction; read only on the consumer thread, /// so it does not need to share a word with the contended `next` link. pub auto_delete: bool, + /// A [`ReusableConcurrentTask`] is armed (posted and not yet consumed). + /// Cleared by the consumer after its last read of the node; `false` for a + /// freshly built node. + pub queued: core::sync::atomic::AtomicBool, } impl Default for ConcurrentTask { @@ -226,6 +250,52 @@ impl Default for ConcurrentTask { task: unsafe { bun_core::ffi::zeroed_unchecked() }, next: Link::new(), auto_delete: false, + queued: core::sync::atomic::AtomicBool::new(false), + } + } +} + +/// An intrusive [`ConcurrentTask`] its owner posts again and again, at most one +/// post in flight: [`arm`](Self::arm) hands the node out only once the consumer +/// is done with the previous post. +pub struct ReusableConcurrentTask(core::cell::UnsafeCell); + +// SAFETY: the node is written only by the thread that wins `queued` +// (false -> true) and read only by the consumer, which clears `queued` after +// its last read; `queued` itself is atomic. +unsafe impl Sync for ReusableConcurrentTask {} +// SAFETY: as above; the payload is a tag and an address. +unsafe impl Send for ReusableConcurrentTask {} + +impl Default for ReusableConcurrentTask { + fn default() -> Self { + Self(core::cell::UnsafeCell::new(ConcurrentTask::default())) + } +} + +impl ReusableConcurrentTask { + /// The node loaded with `task`, ready to post; `None` while the previous + /// post has not been consumed yet. + pub fn arm(&self, task: Task) -> Option> { + let node = self.0.get(); + // SAFETY: `queued` is atomic; winning false -> true makes this thread the + // node's only writer until the consumer clears it (`consumed`). + unsafe { + if (*node) + .queued + .compare_exchange( + false, + true, + core::sync::atomic::Ordering::AcqRel, + core::sync::atomic::Ordering::Acquire, + ) + .is_err() + { + return None; + } + (*node).task = task; + (*node).auto_delete = false; + Some(core::ptr::NonNull::new_unchecked(node)) } } } @@ -241,7 +311,7 @@ impl Default for ConcurrentTask { const _: () = assert!( core::mem::size_of::() == core::mem::size_of::() + 2 * core::mem::size_of::(), - "ConcurrentTask = Task + next ptr + auto_delete (padded)" + "ConcurrentTask = Task + next ptr + auto_delete/queued (padded)" ); // SAFETY: `link()` always projects to the same embedded `next: Link` @@ -277,6 +347,7 @@ impl ConcurrentTask { task, next: Link::new(), auto_delete: true, + queued: core::sync::atomic::AtomicBool::new(false), }); // SAFETY: `new` heap-allocates via `heap::into_raw` — never null. unsafe { core::ptr::NonNull::new_unchecked(raw) } @@ -307,6 +378,7 @@ impl ConcurrentTask { task: Task::init(of), next: Link::new(), auto_delete: auto_deinit == AutoDeinit::AutoDeinit, + queued: core::sync::atomic::AtomicBool::new(false), }; self } @@ -322,11 +394,25 @@ impl ConcurrentTask { let (task, auto_delete) = (this.as_ref().task, this.as_ref().auto_delete()); if auto_delete { drop(bun_core::heap::take(this.as_ptr())); + } else { + Self::consumed(this); } task } } + /// Consuming thread: done reading an intrusive node (its `task` copied, + /// the batch iterator past it); a [`ReusableConcurrentTask`] may be armed again. + /// + /// # Safety + /// `this` is live and not read again by the consumer for this post. + pub unsafe fn consumed(this: core::ptr::NonNull) { + // SAFETY: fn contract. + unsafe { this.as_ref() } + .queued + .store(false, core::sync::atomic::Ordering::Release); + } + /// A weak poster got `task` back because the target VM has closed: free /// it if it is a heap task (`create*`); an intrusive one belongs to its /// container. diff --git a/src/event_loop/lib.rs b/src/event_loop/lib.rs index 9a0be447363f..ca9c1c3aefc3 100644 --- a/src/event_loop/lib.rs +++ b/src/event_loop/lib.rs @@ -29,7 +29,7 @@ pub mod any_event_loop; // ─── public surface ───────────────────────────────────────────────────────── pub type JsResult = core::result::Result; -pub use ConcurrentTask::{Task, TaskTag, Taskable, task_tag}; +pub use ConcurrentTask::{ReusableConcurrentTask, Task, TaskHop, TaskTag, Taskable, task_tag}; // snake_case alias for the file-level-struct module so higher tiers avoid // the type/module namespace collision on the PascalCase form. diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index a632cc72ec67..280b74f4dde8 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -1,5 +1,5 @@ use core::ptr::NonNull; -use core::sync::atomic::{AtomicUsize, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use bun_ast::{Loc, Log}; use bun_core::FeatureFlags; @@ -51,6 +51,12 @@ pub struct AsyncHTTP<'a> { pub elapsed: u64, pub(crate) signals: Signals, + + /// Set (release) by the HTTP thread on the caller's original right before + /// the terminal result callback / shutdown release + /// ([`HTTPClientResultCallback::hand_back`]): from then on the HTTP thread + /// never touches the original again ([`crate::InFlight::reclaim`]). + pub(crate) handed_back: AtomicBool, } bun_threading::intrusive_work_task!(['a] AsyncHTTP<'a>, task); @@ -84,22 +90,6 @@ const fn noop_callback() -> HTTPClientResultCallback { } } -/// Free a `URL.href` slice that the caller marked as owned. -/// -/// # Safety -/// `href` must have been allocated via the global allocator as a `Box<[u8]>` -/// and ownership ceded to this module via `is_url_owned = true`. -#[inline] -unsafe fn free_owned_href(href: &'static [u8]) { - if !href.is_empty() { - // SAFETY: caller guarantees `href` is the sole reference to a - // global-allocator `Box<[u8]>` allocation. The fat `*mut [u8]` is - // obtained directly from the borrowed slice — no need to round-trip - // through `(ptr, len)` + `from_raw_parts`. - unsafe { bun_core::heap::destroy(core::ptr::from_ref(href).cast_mut()) }; - } -} - /// Read the HTTP-thread monotonic timer in nanoseconds. #[inline] fn http_thread_timer_read() -> u64 { @@ -293,23 +283,6 @@ impl<'a> AsyncHTTP<'a> { ); } - /// Copy HTTP-thread progress state into the JS-thread "real" instance. - /// - /// Copy exactly the fields the JS side observes between progress - /// callbacks: the post-redirect `url`, response/timing fields written by - /// `on_async_http_callback`, and the `client` flags/counters used for - /// shutdown decisions and error formatting. Owned allocations stay with - /// `src` (the HTTP-thread copy keeps running while `has_more`). - pub fn sync_progress_from(&mut self, src: &AsyncHTTP<'a>) { - self.url = src.url.clone(); - self.elapsed = src.elapsed; - self.err = src.err; - self.response = src.response; - self.client.url = src.client.url.clone(); - self.client.flags = src.client.flags; - self.client.remaining_redirect_count = src.client.remaining_redirect_count; - } - pub fn clear_data(&mut self) { self.response = None; } @@ -319,81 +292,108 @@ impl<'a> AsyncHTTP<'a> { // Preconnect // ────────────────────────────────────────────────────────────────────────── -struct Preconnect { - // `Option` so we can write the field after the heap address is fixed - // (late-init); `None` is never observed after `preconnect()` populates it. +/// A `fetch.preconnect()` warm-up request: parses `href` once +/// ([`PreparedPreconnect::url`], for the caller to validate) and owns it for the +/// request's lifetime once [`start`](PreparedPreconnect::start)ed. +pub struct PreparedPreconnect { + // Late-init: written by `start` once the heap address is what the HTTP + // thread will see; dropped before `_owned_href`. async_http: Option>, url: URL<'static>, - is_url_owned: bool, + _owned_href: Option>, } -impl Preconnect { - fn on_result(this: *mut Preconnect, _: *mut AsyncHTTP<'static>, _: HTTPClientResult<'_>) { - // SAFETY: `this` was produced by `heap::alloc` in `preconnect()` and is - // uniquely owned here; `async_http` was fully written before scheduling. +impl PreparedPreconnect { + pub fn new(href: Box<[u8]>) -> Box { + // SAFETY: `href`'s heap bytes move into the same box as the `URL` that + // borrows them and are freed after it and after `async_http` (field order). + let url = URL::parse(unsafe { bun_ptr::detach_lifetime(&href) }); + Box::new(Self { + async_http: None, + url, + _owned_href: Some(href), + }) + } + + pub fn url(&self) -> &URL<'_> { + &self.url + } + + fn on_result(this: *mut Self, _: *mut AsyncHTTP<'static>, _: HTTPClientResult<'_>) { + // SAFETY: `this` is the box `start` leaked and is uniquely owned here; + // `async_http` was fully written before scheduling. unsafe { (*this) .async_http .as_mut() - .expect("Preconnect.async_http set in preconnect()") + .expect("PreparedPreconnect.async_http set in start()") .clear_data(); - if (*this).is_url_owned { - // SAFETY: `is_url_owned` is the caller's promise that `url.href` - // is a global-allocator `Box<[u8]>` we now own. - free_owned_href((*this).url.href); - } - // Reclaim and drop the heap allocation (runs Drop on `async_http` - // — which in turn drops `HTTPClient`). drop(bun_core::heap::take(this)); } } -} -pub fn preconnect(url: URL<'static>, is_url_owned: bool) { - if !FeatureFlags::IS_FETCH_PRECONNECT_SUPPORTED { - if is_url_owned { - // SAFETY: `is_url_owned` is the caller's promise that `url.href` is a - // global-allocator `Box<[u8]>` we now own. - unsafe { free_owned_href(url.href) }; - } - return; + /// `HTTPClientResultCallback::release_at_shutdown`: the HTTP thread is + /// parking with this request still out; `on_result` will never run. + /// + /// # Safety + /// `this` is the box `start` leaked; nothing touches it afterwards. + unsafe fn release_at_shutdown(this: *mut ()) { + // SAFETY: fn contract. + drop(unsafe { bun_core::heap::take(this.cast::()) }); } - // Write-before-read: `Bun__fetchPreconnect` reaches here without going - // through any path that calls `HTTPThread::init`, so `schedule()` below - // would deref the uninitialized `HTTP_THREAD` static (UB on niche-bearing - // fields) if `fetch.preconnect()` is the process's first HTTP operation. - // `init` is idempotent (`Once`) and every other JS-side entry point - // (`send_sync`, `FetchTasklet::start`, S3) passes default opts too. - crate::http_thread::init(&Default::default()); + pub fn start(self: Box) { + if !FeatureFlags::IS_FETCH_PRECONNECT_SUPPORTED { + return; + } - let this: *mut Preconnect = bun_core::heap::into_raw(Box::new(Preconnect { - async_http: None, - url, - is_url_owned, - })); + // Write-before-read: `Bun__fetchPreconnect` reaches here without going + // through any path that calls `HTTPThread::init`, so `schedule()` below + // would deref the uninitialized `HTTP_THREAD` static (UB on niche-bearing + // fields) if `fetch.preconnect()` is the process's first HTTP operation. + // `init` is idempotent (`Once`) and every other JS-side entry point + // (`send_sync`, `FetchTasklet::queue`, S3) passes default opts too. + crate::http_thread::init(&Default::default()); - // SAFETY: `this` is a freshly Box-allocated, uniquely-owned pointer; we - // in-place write `async_http` before any read and before it can be observed - // by another thread. - unsafe { - let url = (*this).url.clone(); - let async_http = (*this).async_http.insert(AsyncHTTP::init( - Method::GET, - url, - headers::EntryList::default(), - b"", - b"", - HTTPClientResultCallback::new::(this, Preconnect::on_result), - FetchRedirect::Manual, - Options::default(), - )); - async_http.client.flags.is_preconnect_only = true; - - crate::HTTPThread::schedule(Batch::from(core::ptr::addr_of_mut!(async_http.task))); + let this: *mut Self = bun_core::heap::into_raw(self); + + // SAFETY: `this` is a freshly Box-allocated, uniquely-owned pointer; we + // in-place write `async_http` before any read and before it can be observed + // by another thread. + unsafe { + let url = (*this).url.clone(); + let async_http = (*this).async_http.insert(AsyncHTTP::init( + Method::GET, + url, + headers::EntryList::default(), + b"", + b"", + HTTPClientResultCallback::new_with_release::( + this, + Self::on_result, + Self::release_at_shutdown, + ), + FetchRedirect::Manual, + Options::default(), + )); + async_http.client.flags.is_preconnect_only = true; + + crate::HTTPThread::schedule(Batch::from(core::ptr::addr_of_mut!(async_http.task))); + } } } +/// Warm up a connection to `url`, whose href the caller keeps alive for the +/// process (`--fetch-preconnect`). +pub fn preconnect(url: URL<'static>) { + Box::new(PreparedPreconnect { + async_http: None, + url, + _owned_href: None, + }) + .start(); +} + // ────────────────────────────────────────────────────────────────────────── // impl AsyncHTTP — init / reset / schedule // ────────────────────────────────────────────────────────────────────────── @@ -422,6 +422,19 @@ impl<'a> AsyncHTTP<'a> { let signals = options.signals.unwrap_or_default(); + // Hop 0 resolves from the same settings later hops do + // (`HTTPClient::reevaluate_proxy_for_redirect`). + let http_proxy = match (options.http_proxy, options.proxy_settings.as_deref()) { + (Some(proxy), _) => Some(proxy), + (None, Some(settings)) => settings.resolve(&url).map(|href| { + // SAFETY: self-borrow, as in `reevaluate_proxy_for_redirect`: + // `href` points into `proxy_settings`' boxed storage, which moves + // into `client` below and lives as long as it (>= `'a`). + unsafe { URL::parse(href).erase_lifetime() } + }), + (None, None) => None, + }; + let client = make_client( method, url.clone(), @@ -431,7 +444,7 @@ impl<'a> AsyncHTTP<'a> { headers_buf, signals, async_http_id, - options.http_proxy, + http_proxy, options.proxy_headers, redirect_type, ); @@ -454,6 +467,7 @@ impl<'a> AsyncHTTP<'a> { async_http_id, elapsed: 0, signals, + handed_back: AtomicBool::new(false), }; if let Some(val) = options.unix_socket_path { this.client.unix_socket_path = val; @@ -597,7 +611,7 @@ fn send_sync_callback( // `read_item`. unsafe { result.body_into(&mut (*(*this).response_buffer).list); - (*this).write_item(result.detach_lifetime()); + (*this).write_item(result.into_owned()); } } @@ -750,7 +764,7 @@ impl<'a> AsyncHTTP<'a> { } let elapsed = (*this).elapsed; bun_core::scoped_log!(AsyncHTTP, "onAsyncHTTPCallback: {:?}", elapsed); - callback.run(async_http, result); + callback.hand_back(async_http, result); // SAFETY: `async_http` is the `async_http` field of a // `ThreadlocalAsyncHTTP` heap-allocated by HTTPThread via diff --git a/src/http/HTTPRequestBody.rs b/src/http/HTTPRequestBody.rs index 02869263aea3..7bde67417af0 100644 --- a/src/http/HTTPRequestBody.rs +++ b/src/http/HTTPRequestBody.rs @@ -24,6 +24,15 @@ pub struct Stream { } impl Stream { + /// The HTTP side of `buffer`: takes its own reference, released in + /// [`detach`](Self::detach). + pub fn attach(buffer: &bun_ptr::RefPtr) -> Stream { + Stream { + buffer: core::ptr::NonNull::new(buffer.clone().into_raw()), + ended: false, + } + } + /// Mutable access to the JS-side `ThreadSafeStreamBuffer` while attached. /// /// INVARIANT: while `buffer` is `Some`, this `Stream` holds an intrusive @@ -41,7 +50,7 @@ impl Stream { pub(crate) fn detach(&mut self) { if let Some(buffer) = self.buffer.take() { // Intrusive refcount decrement. - // `buffer` is a live `ThreadSafeStreamBuffer::new` heap allocation; + // `buffer` is a live `ThreadSafeStreamBuffer::create` heap allocation; // this side holds the intrusive ref taken at attach, released here. ThreadSafeStreamBuffer::deref(buffer); } diff --git a/src/http/HTTPThread.rs b/src/http/HTTPThread.rs index 60b23b121ac0..bc474c2429c0 100644 --- a/src/http/HTTPThread.rs +++ b/src/http/HTTPThread.rs @@ -876,22 +876,21 @@ impl HttpThread { self.wakeup(); } - pub fn schedule_cert_check_resume(&mut self, http: &AsyncHttp) { - bun_core::scoped_log!(HTTPThread, "scheduleCertCheckResume {}", http.async_http_id); + pub fn schedule_cert_check_resume(&mut self, async_http_id: u32) { + bun_core::scoped_log!(HTTPThread, "scheduleCertCheckResume {}", async_http_id); { let _guard = self.queued_cert_check_resumes_lock.lock_guard(); - self.queued_cert_check_resumes.push(CertCheckResumeMessage { - async_http_id: http.async_http_id, - }); + self.queued_cert_check_resumes + .push(CertCheckResumeMessage { async_http_id }); } self.wakeup(); } - pub fn schedule_request_write(&mut self, http: &AsyncHttp, kind: WriteMessageType) { + pub fn schedule_request_write(&mut self, async_http_id: u32, kind: WriteMessageType) { { let _guard = self.queued_writes_lock.lock_guard(); self.queued_writes.push(WriteMessage { - async_http_id: http.async_http_id, + async_http_id, kind, }); } @@ -930,11 +929,11 @@ impl HttpThread { let release_unstarted = |http: NonNull>| { // SAFETY: heap-owned by the caller, alive until its completion, // and never touched by us again after this. - let release = unsafe { (*http.as_ptr()).result_callback }; - if let Some(f) = release.release_at_shutdown { - // SAFETY: paired ctx/fn from `HTTPClientResultCallback::new_with_release`. - unsafe { f(release.ctx) }; - } + unsafe { + (*http.as_ptr()) + .result_callback + .hand_back_at_shutdown(http.as_ptr()) + }; }; for http in core::mem::take(&mut self.deferred_tasks) { release_unstarted(http); @@ -965,9 +964,11 @@ impl HttpThread { client.close_proxy_tunnel(false); drop(core::mem::take(&mut client.custom_ssl_ctx)); drop(core::mem::take(&mut client.state)); - if let Some(f) = release.release_at_shutdown { - f(release.ctx); - } + let real = (*nn.as_ptr()) + .async_http + .real + .expect("in-flight copy has an original"); + release.hand_back_at_shutdown(real.as_ptr()); std::alloc::dealloc( nn.as_ptr().cast::(), std::alloc::Layout::new::>(), diff --git a/src/http/Signals.rs b/src/http/Signals.rs index 1d343542a181..09cae88ebea4 100644 --- a/src/http/Signals.rs +++ b/src/http/Signals.rs @@ -107,7 +107,7 @@ impl Default for Store { } impl Store { - pub fn to(&mut self) -> Signals { + pub fn to(&self) -> Signals { Signals { header_progress: Some(NonNull::from(&self.header_progress)), response_body_streaming: Some(NonNull::from(&self.response_body_streaming)), @@ -117,7 +117,7 @@ impl Store { } } - pub fn to_with_backpressure(&mut self) -> Signals { + pub fn to_with_backpressure(&self) -> Signals { Signals { body_receive_mode: Some(NonNull::from(&self.body_receive_mode)), ..self.to() diff --git a/src/http/ThreadSafeStreamBuffer.rs b/src/http/ThreadSafeStreamBuffer.rs index cfb3d46bb13d..d80616954420 100644 --- a/src/http/ThreadSafeStreamBuffer.rs +++ b/src/http/ThreadSafeStreamBuffer.rs @@ -1,57 +1,42 @@ -use core::ffi::c_void; +use core::cell::UnsafeCell; +use std::sync::Arc; use bun_io::StreamBuffer; use bun_threading::Mutex; +/// Told (on the HTTP thread, with the buffer lock held) that the buffer drained. +pub trait DrainHandler: Send + Sync { + fn on_drain(&self); +} + +/// A request-body byte buffer the JS thread fills and the HTTP thread drains, +/// each side holding one counted reference. #[derive(bun_ptr::ThreadSafeRefCounted)] pub struct ThreadSafeStreamBuffer { - pub(crate) buffer: StreamBuffer, + /// Guarded by `mutex`. + buffer: UnsafeCell, pub(crate) mutex: Mutex, - /// Intrusive atomic refcount. Starts at 2: 1 for main thread and 1 for http thread. pub(crate) ref_count: bun_ptr::ThreadSafeRefCount, /// Called by the http thread when the buffer drains; guarded by `mutex`, like `buffer`. - callback: Option, -} - -pub struct Callback { - pub(crate) callback: fn(*mut c_void), - pub(crate) context: *mut c_void, -} - -impl Callback { - pub(crate) fn init(callback: fn(*mut T), context: *mut T) -> Self { - Self { - // SAFETY: fn(*mut T) and fn(*mut c_void) have identical ABI; - // `context` is only ever passed back to this callback, which - // knows its real type. - callback: unsafe { bun_ptr::cast_fn_ptr::(callback) }, - context: context.cast::(), - } - } - - pub(crate) fn call(&self) { - (self.callback)(self.context); - } + callback: UnsafeCell>>, } -impl Default for ThreadSafeStreamBuffer { - fn default() -> Self { - Self { - buffer: StreamBuffer::default(), - mutex: Mutex::default(), - // .initExactRefs(2) — 1 for main thread and 1 for http thread - ref_count: bun_ptr::ThreadSafeRefCount::init_exact_refs(2), - callback: None, - } - } -} +// SAFETY: `buffer` and `callback` are only reached with `mutex` held (the +// guard types below, and the HTTP thread's `acquire`/`release` bracket). +unsafe impl Sync for ThreadSafeStreamBuffer {} +// SAFETY: owned fields are `Send`; the handler is `Send + Sync`. +unsafe impl Send for ThreadSafeStreamBuffer {} impl ThreadSafeStreamBuffer { - /// `bun.TrivialNew(@This())` — heap-allocate with the given field values. - /// Callers on both threads hold raw `*mut ThreadSafeStreamBuffer` and - /// release via `deref()`, so return a raw pointer (heap::alloc). - pub fn new(init: Self) -> *mut Self { - bun_core::heap::into_raw(Box::new(init)) + /// A new buffer with one reference (the caller's). The HTTP side takes its + /// own through [`crate::http_request_body::Stream::attach`]. + pub fn create(drain_handler: Arc) -> bun_ptr::RefPtr { + bun_ptr::RefPtr::new(Self { + buffer: UnsafeCell::new(StreamBuffer::default()), + mutex: Mutex::default(), + ref_count: bun_ptr::ThreadSafeRefCount::init(), + callback: UnsafeCell::new(Some(drain_handler)), + }) } /// Upgrade an attached intrusive-ref handle to `&mut Self`. @@ -70,7 +55,7 @@ impl ThreadSafeStreamBuffer { } pub fn deref(this: core::ptr::NonNull) { - // SAFETY: `this` is a live heap allocation produced by `new`. + // SAFETY: `this` is a live heap allocation produced by `create`. unsafe { bun_ptr::ThreadSafeRefCount::::deref(this.as_ptr()) }; } @@ -79,41 +64,41 @@ impl ThreadSafeStreamBuffer { // The mutex stays locked until `release()`. Prefer `lock()` (RAII // guard) for simple critical sections; this split form remains for // callers that interleave release with disjoint `self` access. - &mut self.buffer + self.buffer.get_mut() + } + + /// The buffer, for a caller between [`acquire`](Self::acquire) and + /// [`release`](Self::release). + pub(crate) fn buffer_held(&mut self) -> &mut StreamBuffer { + debug_assert!(self.mutex.is_held_by_current_thread()); + self.buffer.get_mut() } pub(crate) fn release(&mut self) { self.mutex.unlock(); } - /// RAII spelling of `acquire()`/`release()` — locks the mutex and returns a - /// guard that derefs to the inner `StreamBuffer` and unlocks on `Drop`. - /// Use this instead of a bare `acquire`/`release` pair so the lock is - /// released on every return path. + /// Locks the buffer; the guard derefs to it and unlocks on `Drop`. #[inline] - pub fn lock(&mut self) -> StreamBufferGuard<'_> { + pub fn lock(&self) -> StreamBufferGuard<'_> { self.mutex.lock(); StreamBufferGuard(self) } - /// Should only be called in the main thread and before scheduling it to the http thread - pub fn set_drain_callback(&mut self, callback: fn(*mut T), context: *mut T) { - self.callback = Some(Callback::init(callback, context)); - } - /// Main thread; the request may still be in flight on the http thread. - pub fn clear_drain_callback(&mut self) { + pub fn clear_drain_callback(&self) { let _guard = self.mutex.lock_guard(); - self.callback = None; + // SAFETY: `callback` is guarded by `mutex`, which we hold. + unsafe { *self.callback.get() = None }; } /// This is exclusively called from the http thread. /// Buffer must be acquired before calling this. - pub(crate) fn report_drain(&self) { + pub(crate) fn report_drain(&mut self) { debug_assert!(self.mutex.is_held_by_current_thread()); - if self.buffer.is_empty() { - if let Some(callback) = &self.callback { - callback.call(); + if self.buffer.get_mut().is_empty() { + if let Some(callback) = self.callback.get_mut() { + callback.on_drain(); } } } @@ -121,20 +106,22 @@ impl ThreadSafeStreamBuffer { /// RAII guard returned by [`ThreadSafeStreamBuffer::lock`]. Derefs to the /// protected `StreamBuffer` and releases the mutex on `Drop`. -pub struct StreamBufferGuard<'a>(&'a mut ThreadSafeStreamBuffer); +pub struct StreamBufferGuard<'a>(&'a ThreadSafeStreamBuffer); impl core::ops::Deref for StreamBufferGuard<'_> { type Target = StreamBuffer; #[inline] fn deref(&self) -> &StreamBuffer { - &self.0.buffer + // SAFETY: the guard holds `mutex`, which guards `buffer`. + unsafe { &*self.0.buffer.get() } } } impl core::ops::DerefMut for StreamBufferGuard<'_> { #[inline] fn deref_mut(&mut self) -> &mut StreamBuffer { - &mut self.0.buffer + // SAFETY: the guard holds `mutex`, which guards `buffer`. + unsafe { &mut *self.0.buffer.get() } } } diff --git a/src/http/h2_client/encode.rs b/src/http/h2_client/encode.rs index 89ec1baa18d6..f6836f385bab 100644 --- a/src/http/h2_client/encode.rs +++ b/src/http/h2_client/encode.rs @@ -340,9 +340,8 @@ pub(crate) fn drain_send_body(session: &mut ClientSession, stream: &mut Stream, // SAFETY: data_ptr[cursor..cursor+data_len] is the readable slice. let data = unsafe { bun_core::ffi::slice(data_ptr.add(cursor), data_len) }; let sent = write_data_windowed(session, stream, data, ended, cap); - // We still hold the lock from `acquire()` above; `sb` is the sole - // live borrow, so reborrowing `&mut sb.buffer` is a child access. - let buffer = &mut sb.buffer; + // We still hold the lock from `acquire()` above. + let buffer = sb.buffer_held(); buffer.cursor += sent; let drained = buffer.is_empty(); if drained { diff --git a/src/http/lib.rs b/src/http/lib.rs index 1231c3dec455..f5906efbbe22 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -63,7 +63,7 @@ pub use internal_state::InternalState; pub use proxy_tunnel::ProxyTunnel; pub use send_file::SendFile; pub use signals::Signals; -pub use thread_safe_stream_buffer::ThreadSafeStreamBuffer; +pub use thread_safe_stream_buffer::{DrainHandler, ThreadSafeStreamBuffer}; #[path = "ssl_config.rs"] pub mod ssl_config; pub use ssl_config::SSLConfig; @@ -171,6 +171,7 @@ use bun_core::MutableString; use bun_http_types::FetchRedirect::CommonAbortReason; use bun_ptr::RefPtr; use core::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering}; +use std::sync::Arc; #[repr(u8)] #[derive(Copy, Clone, PartialEq, Eq, Default)] @@ -516,16 +517,10 @@ impl<'a> HTTPClientResult<'a> { } } - /// Widen the borrow to `'static` for self-referential storage. - /// - /// `body` is the only lifetime-carrying field; it borrows the HTTP - /// thread's `decoded_body` scratch buffer, which is cleared immediately - /// after the callback returns, so the stored form carries `body: &[]`. - /// - /// # Safety - /// Caller must not read `.body` from the returned value. + /// The result without its borrowed `body` view (the bytes, if any, are in + /// `body_owned` on the terminal callback), for keeping past the callback. #[inline] - pub unsafe fn detach_lifetime(self) -> HTTPClientResult<'static> { + pub fn into_owned(self) -> HTTPClientResult<'static> { HTTPClientResult { body: &[], body_owned: self.body_owned, @@ -558,6 +553,7 @@ pub struct HTTPClientResultCallback { /// allocator); the JS thread is parked in `shutdown_for_exit` waiting /// for the ack. `None` ⇒ no-op (the default for callers whose `ctx` /// is process-lifetime or whose code path never reaches `global_exit`). + /// Set by `new_with_release` and `from_handler`. pub(crate) release_at_shutdown: Option, } @@ -566,6 +562,40 @@ impl HTTPClientResultCallback { (self.function)(self.ctx, async_http, result); } + /// The terminal result for the HTTP thread's copy `clone`: nothing touches + /// the caller's original after this, so mark it handed back, then deliver. + /// + /// # Safety + /// `clone` is the HTTP thread's live `ThreadlocalAsyncHTTP` copy. + pub(crate) unsafe fn hand_back( + self, + clone: *mut AsyncHTTP<'static>, + result: HTTPClientResult<'_>, + ) { + debug_assert!(!result.has_more); + // SAFETY: fn contract; `real` outlives the copy. + if let Some(real) = unsafe { (*clone).real } { + // SAFETY: as above. + unsafe { (*real.as_ptr()).handed_back.store(true, Ordering::Release) }; + } + self.run(clone, result); + } + + /// `process.exit()` with the request still out: mark the caller's + /// `original` handed back and run the owner's shutdown release, if any. + /// + /// # Safety + /// `original` is the caller's live `AsyncHTTP`; the HTTP thread never + /// touches it again. + pub(crate) unsafe fn hand_back_at_shutdown(self, original: *mut AsyncHTTP<'static>) { + // SAFETY: fn contract. + unsafe { (*original).handed_back.store(true, Ordering::Release) }; + if let Some(release) = self.release_at_shutdown { + // SAFETY: paired ctx/fn from `new_with_release` / `from_handler`. + unsafe { release(self.ctx) }; + } + } + // Type-erases a typed callback behind a raw context pointer. pub fn new( this: *mut T, @@ -596,6 +626,178 @@ impl HTTPClientResultCallback { } } +/// The receiving end of a request started through +/// [`HTTPClientResultCallback::from_handler`], called on the HTTP thread. +pub trait HTTPClientResultHandler: Send + Sync + 'static { + /// A progress (`result.has_more`) or terminal result. By the terminal one + /// the request has already been handed back, so nothing of it is passed. + fn on_result(&self, result: HTTPClientResult<'_>); + /// The process is exiting with the request still out: nothing more will be + /// delivered. HTTP thread; the JS thread is parked. + fn release_at_shutdown(&self) {} +} + +impl HTTPClientResultCallback { + /// Deliver results to `handler`, which the request holds until its terminal + /// result (or shutdown release) has been delivered. + pub fn from_handler(handler: Arc) -> Self { + fn on_result( + ctx: *mut (), + async_http: *mut AsyncHTTP<'static>, + result: HTTPClientResult<'_>, + ) { + let _ = async_http; + let terminal = !result.has_more; + // SAFETY: `ctx` is the `Arc` `from_handler` leaked, released only + // below / in `release`. + unsafe { (*ctx.cast_const().cast::()).on_result(result) }; + if terminal { + // SAFETY: the terminal result is delivered once; this is that ref. + drop(unsafe { Arc::from_raw(ctx.cast_const().cast::()) }); + } + } + unsafe fn release(ctx: *mut ()) { + // SAFETY: as in `on_result`; no result follows a shutdown release. + let handler = unsafe { Arc::from_raw(ctx.cast_const().cast::()) }; + handler.release_at_shutdown(); + } + Self { + ctx: Arc::into_raw(handler).cast_mut().cast::<()>(), + function: on_result::, + release_at_shutdown: Some(release::), + } + } +} + +/// An [`AsyncHTTP`] together with the storage its request borrows (URL, header +/// buffer, body bytes, hostname, ...) point into, in one allocation. +pub struct OwnedRequest(Box>); + +struct RequestCell { + /// Borrows `storage`: declared first so it is dropped first. `None` only + /// while `OwnedRequest::new` builds it and after `into_storage`. + http: Option>, + storage: S, +} + +impl OwnedRequest { + /// Build the request from borrows of `storage`. + pub fn new(storage: S, build: impl for<'a> FnOnce(&'a S) -> AsyncHTTP<'a>) -> Self { + let mut cell = Box::new(RequestCell { + http: None, + storage, + }); + let http = build(&cell.storage); + // SAFETY: `http` borrows only `cell.storage`, which stays in this heap + // cell, is never lent out `&mut` or moved while `http` is alive, and is + // dropped after it; every accessor re-ties the lifetime to a borrow of + // `self`. + let http = unsafe { core::mem::transmute::, AsyncHTTP<'static>>(http) }; + cell.http = Some(http); + Self(cell) + } + + pub fn storage(&self) -> &S { + &self.0.storage + } + + /// Drop the request and take the storage back. + pub fn into_storage(mut self) -> S { + self.0.http = None; + self.0.storage + } + + pub fn http(&self) -> &AsyncHTTP<'_> { + self.0.http.as_ref().expect("built") + } + + /// Adjust the request before it is started. + pub fn with_http_mut(&mut self, f: impl for<'a> FnOnce(&mut AsyncHTTP<'a>) -> R) -> R { + let http = self.0.http.as_mut().expect("built"); + // SAFETY: `'a` is fresh for `f` and outlives the `&mut`, so `f` can + // store into the request only `'static` data or what it already + // borrows (`self.0.storage`); see `new`. + f(unsafe { core::mem::transmute::<&mut AsyncHTTP<'static>, &mut AsyncHTTP<'_>>(http) }) + } + + /// Queue the request on `batch` for the HTTP thread. The request stays + /// allocated, untouched by this thread, until the HTTP thread hands it back. + pub fn start(self, batch: &mut bun_threading::thread_pool::Batch) -> InFlight { + let id = self.http().async_http_id; + let ptr = NonNull::from(Box::leak(self.0)); + // SAFETY: `ptr` is the live allocation just leaked; the HTTP thread takes + // it over from `schedule` until it sets `handed_back`. + unsafe { + (*ptr.as_ptr()) + .http + .as_mut() + .expect("built") + .schedule(batch) + }; + InFlight { ptr, id } + } +} + +/// The caller's handle on a started [`OwnedRequest`]: its id for the +/// `HTTPThread::schedule_*` calls, and the way to take it back once the HTTP +/// thread is done with it. Dropping it before then leaks the request. +pub struct InFlight { + ptr: NonNull>, + id: u32, +} + +impl InFlight { + pub fn async_http_id(&self) -> u32 { + self.id + } + + /// Whether the HTTP thread has handed the request back (its terminal result + /// or shutdown release was delivered). + pub fn handed_back(&self) -> bool { + // SAFETY: the allocation is live until `reclaim`/`drop`; `handed_back` + // is atomic and the HTTP thread only ever reads the bytes around it. + unsafe { + (*self.ptr.as_ptr()) + .http + .as_ref() + .expect("built") + .handed_back + .load(Ordering::Acquire) + } + } + + /// The request's storage. The HTTP thread is reading the bytes `http` + /// borrowed from it: `S` must not free or reallocate those through `&S`. + pub fn storage(&self) -> &S { + // SAFETY: live until `reclaim`/`drop`; never written while in flight. + unsafe { &(*self.ptr.as_ptr()).storage } + } + + /// Take the request back; `Err(self)` while the HTTP thread still has it. + pub fn reclaim(self) -> Result, Self> { + if !self.handed_back() { + return Err(self); + } + let this = core::mem::ManuallyDrop::new(self); + // SAFETY: `start` leaked exactly this box, and the HTTP thread is done with it. + Ok(OwnedRequest(unsafe { Box::from_raw(this.ptr.as_ptr()) })) + } +} + +impl Drop for InFlight { + fn drop(&mut self) { + if self.handed_back() { + // SAFETY: as in `reclaim`. + drop(unsafe { Box::from_raw(self.ptr.as_ptr()) }); + } else { + debug_assert!( + false, + "InFlight request dropped before the HTTP thread handed it back" + ); + } + } +} + // Exists for heap stats reasons. pub struct ThreadlocalAsyncHTTP<'a> { pub(crate) async_http: AsyncHTTP<'a>, diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index fe70b5b5e4b2..3cad4a76afd1 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -346,14 +346,14 @@ impl NetworkTask { } // SAFETY: the HTTP thread is the sole writer for this call; nothing // exclusive to `*this` outlives this block, which ends before the - // cross-thread push below. `detach_lifetime` erases the callback-scoped - // `'_` to `'static` and clears `body` to `&[]`; the body bytes were - // stashed into `(*this).response_buffer` above. + // cross-thread push below. `into_owned` drops the callback-scoped + // `body` view; the body bytes were stashed into `(*this).response_buffer` + // above. unsafe { // Preserve metadata captured on an earlier streaming callback; the // final `result` won't have it. let saved_metadata = (*this).response.metadata.take(); - (*this).response = result.detach_lifetime(); + (*this).response = result.into_owned(); if (*this).response.metadata.is_none() { (*this).response.metadata = saved_metadata; } diff --git a/src/jsc/AbortSignal.rs b/src/jsc/AbortSignal.rs index 5e5e64bb4e05..16684d6a8f65 100644 --- a/src/jsc/AbortSignal.rs +++ b/src/jsc/AbortSignal.rs @@ -73,6 +73,34 @@ pub trait AbortListener { fn on_abort(&mut self, reason: JSValue); } +/// A native `abort` listener registered with [`AbortSignal::listen_native`]; +/// called on the signal's JS thread. +pub trait NativeAbortListener: Sized { + fn on_abort(this: bun_ptr::ThisPtr, reason: JSValue); +} + +/// A native listener's registration on an [`AbortSignal`]: holds a reference +/// on the signal and one unit of its pending activity, and removes the +/// listener when dropped. The listener keeps this for as long as it lives at +/// the address it registered. +pub struct AbortListenerRegistration { + signal: AbortSignalRef, + ctx: *mut c_void, +} + +impl AbortListenerRegistration { + pub fn signal(&self) -> &AbortSignal { + &self.signal + } +} + +impl Drop for AbortListenerRegistration { + fn drop(&mut self) { + self.signal.clean_native_bindings(self.ctx); + self.signal.pending_activity_unref(); + } +} + impl AbortSignal { pub fn listen(&self, ctx: *mut C) -> &AbortSignal { extern "C" fn callback(ptr: *mut c_void, reason: JSValue) { @@ -84,6 +112,25 @@ impl AbortSignal { self.add_listener(ctx.cast::(), callback::) } + /// Call `C::on_abort` on `listener`'s pointee when the signal aborts, for + /// as long as the returned registration is held. + pub fn listen_native( + &self, + listener: bun_ptr::BackRef, + ) -> AbortListenerRegistration { + extern "C" fn callback(ptr: *mut c_void, reason: JSValue) { + // SAFETY: `ptr` is the root pointer `listen_native` registered; its + // pointee holds the registration, which unregisters this callback + // before the pointee goes away. + C::on_abort(unsafe { bun_ptr::ThisPtr::new(ptr.cast::()) }, reason); + } + let ctx = listener.this_ptr().as_ptr().cast::(); + let signal = self.retain(); + self.pending_activity_ref(); + self.add_listener(ctx, callback::); + AbortListenerRegistration { signal, ctx } + } + pub fn add_listener( &self, ctx: *mut c_void, @@ -164,6 +211,13 @@ impl AbortSignal { unsafe { AbortSignalRef::adopt(WebCore__AbortSignal__ref(self)) } } + /// Take a counted reference on this signal. + pub fn retain(&self) -> AbortSignalRef { + // SAFETY: `&AbortSignal` only exists for a live C++ `WebCore::AbortSignal` + // (opaque FFI handle); `ref_()` returns it with the count bumped. + unsafe { AbortSignalRef::adopt(self.ref_()) } + } + pub fn unref(&self) { WebCore__AbortSignal__unref(self) } diff --git a/src/jsc/FetchHeaders.rs b/src/jsc/FetchHeaders.rs index f2d4eb6fb7c6..152d1bdd2f89 100644 --- a/src/jsc/FetchHeaders.rs +++ b/src/jsc/FetchHeaders.rs @@ -308,3 +308,98 @@ impl FetchHeaders { // `WebCore__FetchHeaders__put` extern decl above and the `fast_*` methods take // it by value, so the re-export is ABI-transparent. pub use bun_http_types::Method::HeaderName as HTTPHeaderName; + +/// RAII handle to a C++-owned `WebCore::FetchHeaders`. +/// +/// Holds exactly one ref on the C++ intrusive refcount; `Drop` releases it via +/// `WebCore__FetchHeaders__deref`. NOT a `std::rc::Rc` (the payload lives on +/// the C++ heap and is opaque here). +/// +/// Intentionally not `Clone`: the only "share" operation the surface +/// exposes is `clone_this()`, which deep-copies a fresh `FetchHeaders` on the +/// C++ side. Transferring ownership is by-move. +#[repr(transparent)] +pub struct HeadersRef(NonNull); + +impl HeadersRef { + /// Adopt a freshly-created `FetchHeaders*` (refcount already 1). + /// + /// # Safety + /// `ptr` must be a valid `WebCore::FetchHeaders*` and the caller must + /// transfer ownership of one ref. + #[inline] + pub unsafe fn adopt(ptr: NonNull) -> Self { + Self(ptr) + } + + #[inline] + pub fn as_ptr(&self) -> *mut FetchHeaders { + self.0.as_ptr() + } + + /// `FetchHeaders.createEmpty()` — fresh C++ allocation, refcount 1. + #[inline] + pub fn create_empty() -> Self { + // SAFETY: C++ allocates a new FetchHeaders with refcount 1; never null. + unsafe { Self::adopt(FetchHeaders::create_empty()) } + } + + /// `FetchHeaders.createFromUWS(req)` — fresh C++ allocation, refcount 1. + #[inline] + pub fn create_from_uws(uws_request: *mut core::ffi::c_void) -> Self { + // SAFETY: C++ allocates a new FetchHeaders with refcount 1; never null. + unsafe { Self::adopt(FetchHeaders::create_from_uws(uws_request)) } + } + + /// `FetchHeaders.createFromPicoHeaders(list)` — fresh C++ allocation, refcount 1. + #[inline] + pub fn create_from_pico_headers(pico_headers_list: &[bun_http::picohttp::Header]) -> Self { + // SAFETY: C++ allocates a new FetchHeaders with refcount 1; never null. + unsafe { Self::adopt(FetchHeaders::create_from_pico_headers(pico_headers_list)) } + } + + /// `FetchHeaders.createFromJS(global, value)` — may throw, may return null. + #[inline] + pub fn create_from_js(global: &JSGlobalObject, value: JSValue) -> JsResult> { + // SAFETY: C++ returns a +1 ref or null. + Ok(FetchHeaders::create_from_js(global, value)?.map(|p| unsafe { Self::adopt(p) })) + } + + /// `FetchHeaders.cloneThis(global)` — deep copy on the C++ side. + #[inline] + pub fn clone_this(&self, global: &JSGlobalObject) -> JsResult> { + // SAFETY: C++ returns a +1 ref or null. + Ok(bun_opaque::opaque_deref_mut(self.0.as_ptr()) + .clone_this(global)? + .map(|p| unsafe { Self::adopt(p) })) + } +} + +impl core::ops::Deref for HeadersRef { + type Target = FetchHeaders; + #[inline] + fn deref(&self) -> &FetchHeaders { + // `FetchHeaders` is an opaque ZST FFI handle (S008); `self.0` is live + // for the lifetime of `self` — safe `*const → &` via `opaque_deref`. + bun_opaque::opaque_deref(self.0.as_ptr()) + } +} + +impl core::ops::DerefMut for HeadersRef { + #[inline] + fn deref_mut(&mut self) -> &mut FetchHeaders { + // `FetchHeaders` is an opaque ZST FFI handle (S008); `self.0` is live + // for the lifetime of `self` — safe `*mut → &mut` via `opaque_deref_mut`. + bun_opaque::opaque_deref_mut(self.0.as_ptr()) + } +} + +impl Drop for HeadersRef { + #[inline] + fn drop(&mut self) { + // `self.0` is live; releasing our +1 ref via WebCore__FetchHeaders__deref. + // Explicit UFCS to avoid `core::ops::Deref::deref` resolution ambiguity. + // `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref. + FetchHeaders::deref(bun_opaque::opaque_deref_mut(self.0.as_ptr())); + } +} diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 09e31b639806..5fcebe2f1138 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -980,6 +980,15 @@ impl JSValue { self.as_::().map(|p| unsafe { &*p }) } + /// [`as_class_ref`](Self::as_class_ref) without the prototype-chain walk + /// ([`as_direct`](Self::as_direct)): subclasses are not matched. Caller must + /// have verified `is_cell()`. + #[inline] + pub fn as_direct_class_ref(self) -> Option<&'static T> { + // SAFETY: as for `as_class_ref`. + self.as_direct::().map(|p| unsafe { &*p }) + } + /// [`as_class_ref`](Self::as_class_ref) as a [`ThisPtr`](bun_ptr::ThisPtr), /// for `m_ctx` payloads that are intrusively refcounted: lets the caller /// take its own ref (`RefPtr::from_this`) or dispatch into a diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 8b767f90b8a7..22ef0f7e5dd5 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -262,6 +262,28 @@ impl Ticket { pub fn cancelled(&self) -> bool { self.shared.state() >= State::Draining } + + /// This ticket in a form that can be handed back through `&self` + /// ([`InFlightTicket::hand_back`]), for work whose state is shared. + pub fn in_flight(self) -> InFlightTicket { + let this = core::mem::ManuallyDrop::new(self); + InFlightTicket { + // SAFETY: `this` is never dropped; its one `shared` moves here. + shared: unsafe { core::ptr::read(&raw const this.shared) }, + kind: this.kind, + #[cfg(debug_assertions)] + id: this.id, + returned: core::sync::atomic::AtomicBool::new(false), + } + } + + fn give_back(shared: &Shared, #[cfg(debug_assertions)] id: u64) { + #[cfg(debug_assertions)] + shared.debug.live.lock().at.remove(&id); + if shared.tickets.fetch_sub(1, Ordering::SeqCst) == 1 && shared.state() >= State::Draining { + shared.notify(); + } + } } impl Clone for Ticket { @@ -274,16 +296,57 @@ impl Clone for Ticket { impl Drop for Ticket { fn drop(&mut self) { - #[cfg(debug_assertions)] - self.shared.debug.live.lock().at.remove(&self.id); - if self.shared.tickets.fetch_sub(1, Ordering::SeqCst) == 1 - && self.shared.state() >= State::Draining - { - self.shared.notify(); + Ticket::give_back( + &self.shared, + #[cfg(debug_assertions)] + self.id, + ); + } +} + +/// A [`Ticket`] kept in state shared with the JS thread: the other thread +/// posts through it and hands it back, once, through `&self` when its last +/// touch of the VM is done. Dropping it unreturned hands it back. +pub struct InFlightTicket { + shared: Arc, + kind: LoopKind, + #[cfg(debug_assertions)] + id: u64, + returned: core::sync::atomic::AtomicBool, +} + +impl InFlightTicket { + /// [`Ticket::post`]. Not after [`hand_back`](Self::hand_back). + pub fn post(&self, task: NonNull) { + debug_assert!( + !self.returned.load(Ordering::Relaxed), + "post after hand_back" + ); + debug_assert!( + self.shared.state() != State::Closed, + "ticket post after its VM closed (a ticket was created after the wait)" + ); + self.shared.deliver(self.kind, task); + } + + /// Give the ticket back (what dropping a [`Ticket`] does); later calls do nothing. + pub fn hand_back(&self) { + if !self.returned.swap(true, Ordering::SeqCst) { + Ticket::give_back( + &self.shared, + #[cfg(debug_assertions)] + self.id, + ); } } } +impl Drop for InFlightTicket { + fn drop(&mut self) { + self.hand_back(); + } +} + // ── VmHandle (uncounted) ────────────────────────────────────────────────── /// See the module documentation. `repr(transparent)` over the `Arc` so a diff --git a/src/jsc/Weak.rs b/src/jsc/Weak.rs index 33a84ec916e9..8e7ef5784c19 100644 --- a/src/jsc/Weak.rs +++ b/src/jsc/Weak.rs @@ -103,11 +103,15 @@ impl Weak { } } + /// A weak handle whose `ref_type` finalize callback is handed `owner` when + /// `value` is collected. The owner keeps the returned handle (dropping it + /// clears the callback) for as long as it lives at that address, and is + /// the type that callback expects. pub fn create( value: JSValue, global_this: &JSGlobalObject, ref_type: WeakRefType, - ctx: &mut T, + owner: bun_ptr::BackRef, ) -> Self { if !value.is_empty() { return Self { @@ -115,7 +119,7 @@ impl Weak { global_this, value, ref_type, - Some(NonNull::from(ctx).cast::()), + Some(NonNull::from(owner).cast::()), )), _ctx: PhantomData, }; diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index f3c3b0804675..4db5d4d5d42e 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -632,6 +632,10 @@ impl EventLoop { // LinearFifo's fields are private — `write_item` is the // public path (single-slot copy, same complexity). let _ = self.tasks.write_item(task_ref.task); + if to_destroy != Some(task) { + // SAFETY: `task` is live; this loop is done reading it. + unsafe { ConcurrentTask::ConcurrentTask::consumed(NonNull::new_unchecked(task)) }; + } } if let Some(dest) = to_destroy { diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 03faa8dec0e7..1ccf3494db2c 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -765,7 +765,9 @@ pub use self::dom_form_data::DOMFormData; pub use self::url::{URL, URLJsc}; pub use self::zig_stack_frame::ZigStackFrame; pub use self::zig_stack_trace::ZigStackTrace; -pub use abort_signal::{AbortSignal, AbortSignalRef}; +pub use abort_signal::{ + AbortListenerRegistration, AbortSignal, AbortSignalRef, NativeAbortListener, +}; // `VM` / `JSGlobalObject` — opaque FFI handles to C++-owned objects. Defined // once in their dedicated port files (`VM.rs` / `JSGlobalObject.rs`) and @@ -895,7 +897,7 @@ pub use self::resolved_source_tag::ResolvedSourceTag; // ────────────────────────────────────────────────────────────────────────── #[path = "FetchHeaders.rs"] pub mod fetch_headers; -pub use self::fetch_headers::{FetchHeaders, HTTPHeaderName}; +pub use self::fetch_headers::{FetchHeaders, HTTPHeaderName, HeadersRef}; /// `BuiltinName` — fast-path property keys preallocated as `JSC::Identifier`s /// in C++ (`BunBuiltinNames.h`). Passed to `JSValue::fast_get` as a `u8` index @@ -1196,7 +1198,7 @@ pub mod virtual_machine; pub mod vm_handle; pub use self::virtual_machine as VirtualMachine; pub use self::virtual_machine::InitOptions as VirtualMachineInitOptions; -pub use self::vm_handle::{ConcurrentPoster, LoopKind, Posted, Ticket, VmHandle}; +pub use self::vm_handle::{ConcurrentPoster, InFlightTicket, LoopKind, Posted, Ticket, VmHandle}; #[path = "ModuleLoader.rs"] pub mod module_loader; diff --git a/src/picohttp/lib.rs b/src/picohttp/lib.rs index 5dd20f5e13e7..bccd120d1b27 100644 --- a/src/picohttp/lib.rs +++ b/src/picohttp/lib.rs @@ -75,6 +75,14 @@ pub struct Header { value_len: usize, } +// SAFETY: a `Header` is two borrowed `&[u8]` views spelled as (ptr, len) for C +// layout; it owns nothing and has `&[u8]`'s thread-safety. The borrow's +// lifetime is erased (see `Header::new`), so whoever sends one must keep the +// backing buffer alive and unchanged for as long as the receiver reads it. +unsafe impl Send for Header {} +// SAFETY: as above. +unsafe impl Sync for Header {} + impl Default for Header { #[inline] fn default() -> Self { diff --git a/src/ptr/js_cell.rs b/src/ptr/js_cell.rs index e9d9c7567c33..3aab94a79933 100644 --- a/src/ptr/js_cell.rs +++ b/src/ptr/js_cell.rs @@ -73,6 +73,12 @@ impl JsCell { unsafe { &mut *self.0.get() } } + /// Mutable access through an exclusive borrow of the cell itself. + #[inline(always)] + pub fn as_mut(&mut self) -> &mut T { + self.0.get_mut() + } + /// Closure-scoped mutable access. The `&mut T` cannot escape `f`, so the /// only way to violate the aliasing invariant is for `f` itself to /// re-enter a path that touches this same cell — which the diff --git a/src/ptr/lib.rs b/src/ptr/lib.rs index 303ccd736d65..13b76d8a735b 100644 --- a/src/ptr/lib.rs +++ b/src/ptr/lib.rs @@ -276,6 +276,13 @@ impl From> for BackRef { } } +impl From> for core::ptr::NonNull { + #[inline] + fn from(p: ThisPtr) -> Self { + p.0 + } +} + impl Copy for BackRef {} impl Clone for BackRef { #[inline] @@ -653,6 +660,14 @@ impl ThisPtr { self.0.as_ptr() } + /// Record this pointer as a write-capable back-reference (for handle enums + /// whose dispatcher forms the `&mut`). The holder takes on the `BackRef` + /// invariant. + #[inline] + pub fn backref_mut(self) -> BackRef { + BackRef(self.0, core::marker::PhantomData) + } + /// Fresh shared borrow of the pointee. /// /// Sound under the [`new`](Self::new) invariant: the pointee is live and @@ -683,6 +698,49 @@ impl core::ops::Deref for ThisPtr { } } +// ───────────────────────────────────────────────────────────────────────────── +// OwnedThis — single-owner heap allocation that hands out `ThisPtr`s. +// +// `Box` asserts unique access on every touch, which is wrong for a callback +// hub whose address is also held by C / JS / a task queue and re-entered while +// a method on it is running. `OwnedThis` keeps the ownership (drop frees) but +// only ever lends the pointee as `ThisPtr` / `&T`. +// ───────────────────────────────────────────────────────────────────────────── + +/// The unique owner of a heap-allocated `T` that is otherwise reached through +/// [`ThisPtr`] copies. Dropping it drops and frees the `T`; every `ThisPtr` +/// lent from it must be dead by then (the usual back-reference obligation). +pub struct OwnedThis(core::ptr::NonNull); + +impl OwnedThis { + #[inline] + pub fn new(value: T) -> Self { + OwnedThis(core::ptr::NonNull::from(Box::leak(Box::new(value)))) + } + + /// A dispatch handle to the pointee (root provenance). + #[inline] + pub fn this_ptr(&self) -> ThisPtr { + ThisPtr(self.0) + } +} + +impl core::ops::Deref for OwnedThis { + type Target = T; + #[inline] + fn deref(&self) -> &T { + // SAFETY: we own the live allocation. + unsafe { self.0.as_ref() } + } +} + +impl Drop for OwnedThis { + fn drop(&mut self) { + // SAFETY: `new` leaked exactly this `Box`; we are its unique owner. + drop(unsafe { Box::from_raw(self.0.as_ptr()) }); + } +} + // SAFETY: `BackRef` is morally `&T` (Deref/get) with, for `P = Mut`, an // unsafe `get_mut` escape hatch whose exclusivity is the caller's per-site // obligation. Match `&T` auto-trait bounds: `&T: Send ⇔ T: Sync`, diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index ffa4802bb9d0..05ebd2d3a24c 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -874,7 +874,7 @@ Full documentation is available at https://bun.com/docs/cli/run Global::exit(1); } - bun_http::async_http::preconnect(url, false); + bun_http::async_http::preconnect(url); } } diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index afae984518da..cdf1f62ea373 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -80,7 +80,7 @@ use crate::shell::interpreter::ShellTask; use crate::shell::io_writer::Poll as ShellBufferedWriterPoll; use crate::shell::states::r#async::Async as ShellAsync; -use crate::webcore::fetch::fetch_tasklet::FetchTasklet; +use crate::webcore::fetch::fetch_tasklet as fetch; use crate::webcore::file_sink::FlushPendingTask as FlushPendingFileSinkTask; #[cfg(not(windows))] use crate::webcore::file_sink::Poll as FileSinkPoll; @@ -189,6 +189,17 @@ pub(crate) fn run_task( task.ptr.cast::<$ty>() }; } + /// `<$hop as TaskHop>::run` on the queued `ThisPtr`, SAFETY spelled once. + macro_rules! hop { + ($hop:ty) => {{ + // SAFETY: §Dispatch — `task.tag` is `<$hop>::TAG`, set together with + // `task.ptr` from a `ThisPtr` whose pointee keeps itself alive for + // the queued task; not touched here after the call. + <$hop as bun_event_loop::TaskHop>::run(unsafe { + bun_ptr::ThisPtr::new(cast_ptr!(<$hop as bun_event_loop::TaskHop>::Target)) + }) + }}; + } /// `CompressionStream::::run_from_js_thread` takes `*mut T` (full /// allocation provenance — R-2) so its trailing `T::deref()` may free the box. macro_rules! compression_arm { @@ -244,15 +255,6 @@ pub(crate) fn run_task( cast_ptr!(crate::api::js_bundle_completion_task::JSBundleCompletionTask), )?; } - task_tag::FetchTaskletPromiseSettle => { - // SAFETY: boxed at the fetch completion site; the arm consumes it. - let holder = unsafe { - bun_core::heap::take(cast_ptr!( - crate::webcore::fetch::fetch_tasklet::FetchTaskletPromiseSettle - )) - }; - holder.run()?; - } task_tag::DuplexUpgradeContext => { // SAFETY: tag identifies pointee; the queue owns the live context // until `run_event` (which may free it). @@ -333,17 +335,10 @@ pub(crate) fn run_task( | task_tag::ShellYesTask => run_task_cold(task), // ── fetch / S3 ─────────────────────────────────────────────────── - task_tag::FetchTasklet => { - cast!(FetchTasklet).on_progress_update()?; - } - task_tag::FetchTaskletDeinit => { - // SAFETY: posted by `deref_from_thread` with the last ref. - unsafe { - crate::webcore::fetch::FetchTaskletDeinitHop::run(cast_ptr!( - crate::webcore::fetch::FetchTaskletDeinitHop - )) - }; - } + task_tag::FetchTasklet => hop!(fetch::ProgressHop)?, + task_tag::FetchTaskletHandBack => hop!(fetch::HandBackHop)?, + task_tag::FetchTaskletPromiseSettle => hop!(fetch::PromiseSettleHop)?, + task_tag::FetchTaskletRequestDataDrain => hop!(fetch::RequestBodyDrainHop)?, // `cast_ptr!` yields the heap-allocated S3 task; JS-thread dispatch // is the sole owner here. task_tag::S3HttpSimpleTask => { @@ -589,7 +584,7 @@ fn run_task_cold(task: Task) { /// `release_task_unrun` track `bun_event_loop::task_tag::COUNT`. Bump when /// adding a variant — and give it an arm in both. const _: () = assert!( - task_tag::COUNT == 61, + task_tag::COUNT == 62, "dispatch::run_task / release_task_unrun arm count out of sync with bun_event_loop::task_tag", ); @@ -1219,6 +1214,15 @@ fn __bun_release_task_unrun(task: bun_event_loop::Task) { unsafe { <$ty as Taskable>::release_unrun(task.ptr.cast::<$ty>()) } }}; } + /// `<$hop as TaskHop>::release_unrun` on the queued `ThisPtr`, SAFETY spelled once. + macro_rules! release_hop { + ($hop:ty) => {{ + // SAFETY: as `release!`; the tag is `<$hop>::TAG`. + <$hop as bun_event_loop::TaskHop>::release_unrun(unsafe { + bun_ptr::ThisPtr::new(task.ptr.cast::<<$hop as bun_event_loop::TaskHop>::Target>()) + }) + }}; + } match task.tag { task_tag::AnyTaskJob => { // The one erased tag: every payload is a `Job` reached through its header. @@ -1235,11 +1239,10 @@ fn __bun_release_task_unrun(task: bun_event_loop::Task) { task_tag::ShellYesTask => release!(ShellYesTask), task_tag::CppTask => release!(CppTask), task_tag::DuplexUpgradeContext => release!(crate::socket::DuplexUpgradeContext), - task_tag::FetchTasklet => release!(FetchTasklet), - task_tag::FetchTaskletDeinit => release!(crate::webcore::fetch::FetchTaskletDeinitHop), - task_tag::FetchTaskletPromiseSettle => { - release!(crate::webcore::fetch::fetch_tasklet::FetchTaskletPromiseSettle) - } + task_tag::FetchTasklet => release_hop!(fetch::ProgressHop), + task_tag::FetchTaskletHandBack => release_hop!(fetch::HandBackHop), + task_tag::FetchTaskletPromiseSettle => release_hop!(fetch::PromiseSettleHop), + task_tag::FetchTaskletRequestDataDrain => release_hop!(fetch::RequestBodyDrainHop), task_tag::FSWatchTask => release!(FSWatchTask), task_tag::HotReloadTask => release!(hot_reloader::HotReloadTask), task_tag::WatchReloadTask => release!(hot_reloader::WatchReloadTask), diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 5eb8bfa11816..e840a76e72b2 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1795,10 +1795,10 @@ fn stop_active_handles(vm: &mut VirtualMachine, reason: StopReason) -> SweepResu ActiveHandle::WindowsNamedPipe(c) => unsafe { crate::socket::WindowsNamedPipeContext::stop_for_vm_teardown(c.as_ptr()) }, - // SAFETY: live until it unregisters in `deinit`. - ActiveHandle::Fetch(t) => unsafe { - crate::webcore::fetch::FetchTasklet::stop_for_vm_teardown(t.as_ptr()) - }, + ActiveHandle::Fetch(t) => crate::webcore::fetch::FetchTasklet::stop_for_vm_teardown( + // SAFETY: registered (from its `ThisPtr`) until it unregisters on drop ⇒ live. + unsafe { bun_ptr::ThisPtr::new(t.as_ptr()) }, + ), // SAFETY: live until they unregister in `on_response`. ActiveHandle::S3Request(t) => unsafe { crate::webcore::s3::simple_request::S3HttpSimpleTask::stop_for_vm_teardown( diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index 094f34edc381..69abc7681915 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -3291,8 +3291,11 @@ where } } - if lock.on_receive_value.is_some() || lock.task.is_some() { - // someone else is waiting for the stream or waiting for `onStartStreaming` + if lock.on_receive_value.is_some() + || lock.task.is_some() + || !lock.producer.is_dead() + { + // someone else is waiting for the stream or a producer is waiting for `onStartStreaming` let readable = match value.to_readable_stream(global_this) { Ok(readable) => readable, Err(err) => { diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 55ac224a743b..848b21469d30 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -1425,7 +1425,6 @@ impl BlobExt for Blob { proxy_url, aws_options.request_payer, None, - core::ptr::null_mut(), ); } @@ -4607,7 +4606,6 @@ pub(crate) fn write_file_with_source_destination( proxy_url, aws_options.request_payer, None, - core::ptr::null_mut(), ); } else { return Ok(JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( @@ -4703,7 +4701,6 @@ pub(crate) fn write_file_with_source_destination( proxy_url, aws_options.request_payer, None, - core::ptr::null_mut(), ); } else { return Ok( @@ -4990,7 +4987,6 @@ pub(crate) fn write_file_internal( proxy_url, aws_options.request_payer, None, - core::ptr::null_mut(), )?)); } destination_blob.detach(); @@ -5074,6 +5070,7 @@ pub(crate) fn write_file_internal( unreachable!() }; let producer_hook = locked.on_start_buffering.take().zip(locked.task); + let producer = locked.producer; locked.task = Some(NonNull::new(task).unwrap().cast::()); locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap); // SAFETY: `task` was just heap-allocated; consumed in `then_wrap`. @@ -5082,6 +5079,8 @@ pub(crate) fn write_file_internal( // `then_wrap` may run and `*body_value` be replaced inside. if let Some((on_start_buffering, producer_task)) = producer_hook { on_start_buffering(producer_task); + } else { + producer.start_buffering(); } Ok(ControlFlow::Break(promise)) } diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index e684b69f0fdd..bd13e92b6a7c 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -233,6 +233,9 @@ pub struct PendingValue { /// may resolve or fail this body synchronously from inside the call — /// replacing the `Value` this `PendingValue` lives in — so callers install /// their `promise`/`on_receive_value` first and touch nothing afterwards. + /// A producer that leaves these three hooks unset is signalled through + /// `producer` (`SourceHandle::{start_buffering, start_streaming, + /// readable_stream_available}`) instead. pub(crate) on_start_buffering: Option)>, pub(crate) on_start_streaming: Option) -> DrainResult>, pub(crate) on_readable_stream_available: @@ -278,7 +281,7 @@ impl Default for PendingValue { impl PendingValue { /// Once `readable` is set the live handle is `NewSource.producer`; these - /// hooks go stale when the producer (e.g. `FetchTasklet`) is freed. + /// hooks go stale when the producer is freed. fn detach_producer(&mut self) { self.on_start_buffering = None; self.on_start_streaming = None; @@ -423,11 +426,14 @@ impl PendingValue { self.promise = Some(promise_value); promise_value.protect(); + // Last use of `self`: the producer may settle the body (and so + // replace `*self`) before this returns. if let Some(on_start_buffering) = self.on_start_buffering.take() { - // Last use of `self`: the producer may settle the body (and so - // replace `*self`) before this returns. let task = self.task.unwrap(); on_start_buffering(task); + } else { + let producer = self.producer; + producer.start_buffering(); } Ok(promise_value) } @@ -862,6 +868,8 @@ impl Value { if let Some(drain) = locked.on_start_streaming.take() { drain_result = drain(locked.task.unwrap()); + } else if let Some(drained) = locked.producer.start_streaming() { + drain_result = drained; } if matches!(drain_result, DrainResult::Aborted) { @@ -900,6 +908,10 @@ impl Value { if let Some(on_readable_stream_available) = locked.on_readable_stream_available.take() { on_readable_stream_available(locked.task.unwrap(), global_this, readable); + } else { + locked + .producer + .readable_stream_available(global_this, &readable); } locked.detach_producer(); @@ -1460,6 +1472,8 @@ impl Value { if let Some(drain) = locked.on_start_streaming.take() { drain_result = drain(locked.task.unwrap()); + } else if let Some(drained) = locked.producer.start_streaming() { + drain_result = drained; } if matches!(drain_result, DrainResult::Aborted) { @@ -1503,6 +1517,10 @@ impl Value { global_this, locked.readable.get().unwrap(), ); + } else { + locked + .producer + .readable_stream_available(global_this, &locked.readable.get().unwrap()); } locked.detach_producer(); diff --git a/src/runtime/webcore/ByteStream.rs b/src/runtime/webcore/ByteStream.rs index f6c4a611b6db..18650abf20ea 100644 --- a/src/runtime/webcore/ByteStream.rs +++ b/src/runtime/webcore/ByteStream.rs @@ -5,6 +5,7 @@ use bun_jsc::strong::Optional as StrongOptional; use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsCell}; use bun_sys::Error as SysError; +use crate::webcore::readable_stream::SourceRef; use crate::webcore::streams::{self, BufferAction, IntoArray}; use crate::webcore::{DrainResult, SinkHandle, blob, readable_stream}; @@ -72,7 +73,7 @@ pub type Source = readable_stream::NewSource; /// can be collected (`SourceHandle::consumer_collected`). #[derive(Default)] pub struct ProducerHold { - source: Cell>>, + source: JsCell>>, parked: Cell, } @@ -88,18 +89,17 @@ pub enum AfterDelivery { } impl ProducerHold { - /// Take the producer ref on the stream's source (JS thread). - /// - /// # Safety - /// `bytes` is the live ByteStream of a stream the caller holds. - pub unsafe fn hold(&self, bytes: *mut ByteStream) { + /// Take the producer ref on `readable`'s source, if it is a byte stream (JS thread; the + /// caller holds the stream). + pub fn hold(&self, readable: &readable_stream::ReadableStream) { self.release(); - // SAFETY: fn contract; the ref keeps the Source alive past this call. - unsafe { - let source = Source::from_context_ptr(bytes); - (*source).increment_count(); - self.source.set(core::ptr::NonNull::new(source)); - } + self.source.set(SourceRef::byte_stream(readable)); + } + + /// [`hold`](Self::hold) for a source the caller has in hand. + pub fn hold_source(&self, source: &Source) { + self.release(); + self.source.set(Some(SourceRef::new(source))); } pub fn is_held(&self) -> bool { @@ -109,22 +109,16 @@ impl ProducerHold { /// The held stream, pinned for the guard's life: a consumer inside `on_data` can cancel the /// producer (which drops the hold), and while parked the wrapper is not rooted. pub fn bytes(&self) -> Option { - let source = self.source.get()?; - // SAFETY: live through our ref; no borrow of the source exists yet. - unsafe { (*source.as_ptr()).increment_count() }; - Some(PinnedBytes(source)) + self.source.get().clone().map(PinnedBytes) } /// Stop being the producer. The source stays pinned by the returned guard, so the caller can /// still deliver a terminal chunk. Touches no JS cell. pub fn take(&self) -> Option { - let source = self.source.take()?; + let source = self.source.replace(None)?; self.parked.set(false); - // SAFETY: still pinned by our ref, which the guard now owns. - unsafe { - (*source.as_ptr()).producer.set(streams::SourceHandle::None); - (*source.as_ptr()).wrapper_unrooted.set(false); - } + source.producer.set(streams::SourceHandle::None); + source.wrapper_unrooted.set(false); Some(PinnedBytes(source)) } @@ -151,9 +145,7 @@ impl ProducerHold { return false; } if let Some(source) = self.source.get() { - // SAFETY: live through our ref. The caller may hold the `&ByteStream` of this very - // source (the chunk it just delivered), which is why this is not a method call. - unsafe { Source::unroot_wrapper(source.as_ptr()) }; + source.unroot_wrapper(); } true } @@ -165,8 +157,7 @@ impl ProducerHold { return false; } if let Some(source) = self.source.get() { - // SAFETY: as in `park`. - unsafe { Source::root_wrapper(source.as_ptr()) }; + source.root_wrapper(); } true } @@ -179,20 +170,12 @@ impl Drop for ProducerHold { } /// A counted ref on a stream's `Source` for the guard's life; derefs to its ByteStream. -pub struct PinnedBytes(core::ptr::NonNull); +pub struct PinnedBytes(SourceRef); impl core::ops::Deref for PinnedBytes { type Target = ByteStream; fn deref(&self) -> &ByteStream { - // SAFETY: pinned by this guard's ref; ByteStream is `&self`-only. - unsafe { &(*self.0.as_ptr()).context } - } -} - -impl Drop for PinnedBytes { - fn drop(&mut self) { - // SAFETY: balances the ref this guard owns. Can free the source. - unsafe { Source::decrement_count(self.0.as_ptr()) }; + &self.0.context } } diff --git a/src/runtime/webcore/ReadableStream.rs b/src/runtime/webcore/ReadableStream.rs index 86f0bc7d40a9..0c3844db2867 100644 --- a/src/runtime/webcore/ReadableStream.rs +++ b/src/runtime/webcore/ReadableStream.rs @@ -4,6 +4,7 @@ use core::ptr::NonNull; use crate::webcore::jsc::SysErrorJsc as _; use crate::webcore::jsc::{self as jsc, CallFrame, JSGlobalObject, JSValue, JsResult}; +use bun_jsc::JsCellRefExt as _; // `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; @@ -849,10 +850,11 @@ pub trait SourceContext: Sized { // With Rust's default repr the field is reordered and the cast reads // adjacent fields as the loader, returning empty bodies. #[repr(C)] +#[derive(bun_ptr::CellRefCounted)] pub struct NewSource { pub context: C, pub cancelled: bool, - pub ref_count: u32, + pub ref_count: Cell, pub pending_err: Option, pub close_handler: Option)>, /// Borrowed opaque context for native `close_handler`s (never @@ -874,7 +876,7 @@ pub struct NewSource { /// only the wrapper's own ref remains. [`Self::finalize`] flips it to /// `Finalized` so [`Self::on_js_close`] reads `None` instead of a /// dead-but-unswept cell. - pub this_jsvalue: jsc::JsRef, + pub this_jsvalue: bun_jsc::JsCell, /// The producer holding a native ref has parked ([`Self::unroot_wrapper`]): /// its ref keeps this allocation, not the wrapper, so an unread stream can /// be collected. Cleared by [`Self::root_wrapper`]. @@ -890,13 +892,13 @@ impl Default for NewSource { Self { context: C::default(), cancelled: false, - ref_count: 1, + ref_count: Cell::new(1), pending_err: None, close_handler: None, close_ctx: None, producer: Cell::new(streams::SourceHandle::None), global_this: None, - this_jsvalue: jsc::JsRef::empty(), + this_jsvalue: bun_jsc::JsCell::new(jsc::JsRef::empty()), wrapper_unrooted: Cell::new(false), is_closed: Cell::new(false), } @@ -1023,11 +1025,66 @@ impl NewSourceCodegen for NewSource { } } +/// A producer's counted reference to a [`NewSource`]: keeps the allocation +/// alive and, unless the producer parked it ([`NewSource::unroot_wrapper`]), +/// the JS wrapper rooted. Released on drop. +pub struct SourceRef(bun_ptr::BackRef>); + +impl SourceRef { + fn retain(source: bun_ptr::BackRef>) -> Self { + source.increment_count(); + Self(source) + } + + /// One more reference on `source` (a live [`NewSource::new`] allocation, which every + /// `&NewSource` handed out by the stream machinery is). + pub fn new(source: &NewSource) -> Self { + Self::retain(bun_ptr::BackRef::new(source)) + } +} + +impl SourceRef { + /// A reference on the `ByteStream` source behind `stream`, if that is what it is. + pub fn byte_stream(stream: &ReadableStream) -> Option { + stream + .ptr + .bytes() + .map(|bytes| Self::retain(bun_ptr::BackRef::new(bytes.parent_const()))) + } +} + +impl Clone for SourceRef { + fn clone(&self) -> Self { + Self::retain(self.0) + } +} + +impl core::ops::Deref for SourceRef { + type Target = NewSource; + #[inline] + fn deref(&self) -> &NewSource { + self.0.get() + } +} + +impl Drop for SourceRef { + fn drop(&mut self) { + self.0.will_release_ref(); + as bun_ptr::CellRefCounted>::deref_nn(self.0.into()); + } +} + // Enforce the layout invariant `from_js`/`Source` rely on. const _: () = assert!(core::mem::offset_of!(NewSource, context) == 0); const _: () = assert!(core::mem::offset_of!(NewSource, context) == 0); const _: () = assert!(core::mem::offset_of!(NewSource, context) == 0); +impl Drop for NewSource { + fn drop(&mut self) { + self.context.deinit_fn(); + } +} + impl NewSource { /// Point the `owner` slot at the GC cell of the peer producing into this /// source (its `producer` backref), so rooting the source roots the @@ -1163,63 +1220,55 @@ impl NewSource { ); } - pub fn increment_count(&mut self) { - self.ref_count += 1; + pub fn increment_count(&self) { + self.ref_(); // A ref beyond the JS wrapper's own is held (in practice a FileReader // `waiting_for_on_reader_done` I/O ref). Root the wrapper so // `on_js_close`, reached from `on_reader_done` off the event loop with // no JS frame on the stack, never reads a dead-but-unswept cell. if !self.wrapper_unrooted.get() { - // SAFETY: `self` is live for the call. - unsafe { Self::upgrade_wrapper(self) }; + self.upgrade_wrapper(); } } - /// # Safety - /// `this` points at a live `NewSource`. - unsafe fn upgrade_wrapper(this: *mut Self) { - // SAFETY: fn contract; field places only, see `unroot_wrapper`. - unsafe { - if let Some(global) = (*this).global_this.as_deref() { - if (*this).this_jsvalue.is_not_empty() { - (*this).this_jsvalue.upgrade(global); + fn upgrade_wrapper(&self) { + if let Some(global) = self.global_this.as_deref() { + self.this_jsvalue.with_mut(|this_jsvalue| { + if this_jsvalue.is_not_empty() { + this_jsvalue.upgrade(global); } - } + }); } } /// The producer keeps its native ref but stops rooting the wrapper: nothing /// is reading, so the stream should be collectable. [`SourceContext::wrapper_finalized`] /// tells the producer if that happens. - /// - /// Takes a raw pointer: the producer reaches this while it holds a `&C` into - /// `this` (the chunk it is delivering to), so only the fields written here - /// are touched, never a `&mut Self` that would cover the context too. - /// - /// # Safety - /// `this` points at a live `NewSource`. - pub unsafe fn unroot_wrapper(this: *mut Self) { - // SAFETY: fn contract. - unsafe { - (*this).wrapper_unrooted.set(true); - (*this).this_jsvalue.downgrade(); - } + pub fn unroot_wrapper(&self) { + self.wrapper_unrooted.set(true); + self.this_jsvalue.with_mut(jsc::JsRef::downgrade); } /// Undo [`Self::unroot_wrapper`]: a consumer is reading again. - /// - /// # Safety - /// As [`Self::unroot_wrapper`]. - pub unsafe fn root_wrapper(this: *mut Self) { - // SAFETY: fn contract. - unsafe { - (*this).wrapper_unrooted.set(false); - if (*this).ref_count > 1 { - Self::upgrade_wrapper(this); - } + pub fn root_wrapper(&self) { + self.wrapper_unrooted.set(false); + if self.ref_count.get() > 1 { + self.upgrade_wrapper(); } } + /// Bookkeeping ahead of releasing one reference: once only the JS wrapper's + /// own ref will remain, drop the Strong root so the wrapper becomes + /// collectable again. Returns the count after the release. + fn will_release_ref(&self) -> u32 { + let rc = self.ref_count.get(); + debug_assert!(rc > 0, "Attempted to decrement ref count below zero"); + if rc == 2 { + self.this_jsvalue.with_mut(jsc::JsRef::downgrade); + } + rc - 1 + } + /// Release one reference. If the count hits zero, runs context teardown and /// **frees the allocation**. /// @@ -1231,33 +1280,10 @@ impl NewSource { /// [`Self::new`] (i.e. `Box::into_raw`). Caller must not dereference `this` /// — nor any interior pointer such as `&mut context` — after this returns. pub unsafe fn decrement_count(this: *mut Self) -> u32 { - // SAFETY: caller contract — `this` is live for the duration of this block. - let remaining = unsafe { - let r = &mut (*this).ref_count; - #[cfg(debug_assertions)] - if *r == 0 { - panic!("Attempted to decrement ref count below zero"); - } - *r -= 1; - *r - }; - if remaining == 1 { - // Only the JS wrapper's own ref remains: drop the Strong root so - // the wrapper becomes collectable again. - // SAFETY: caller contract — `this` is live while remaining > 0. - unsafe { (*this).this_jsvalue.downgrade() }; - } - if remaining == 0 { - // SAFETY: still live; run side-effect teardown while fields are valid. - unsafe { - (*this).context.deinit_fn(); - } - // SAFETY: `this` originated from `Box::into_raw` in `Self::new`. No - // `&mut` borrow of `*this` is live at this point — reclaim and drop, - // which runs `Drop` on `context` and all other fields, then frees. - drop(unsafe { bun_core::heap::take(this) }); - return 0; - } + // SAFETY: caller contract — `this` is live. + let remaining = unsafe { &*this }.will_release_ref(); + // SAFETY: caller contract — `this` came from `Self::new` and is not used again. + unsafe { ::deref(this) }; remaining } @@ -1276,8 +1302,8 @@ impl NewSource { ::to_js(self, global_this) }; out_value.ensure_still_alive(); - if self.this_jsvalue.is_empty() { - self.this_jsvalue = jsc::JsRef::init_weak(out_value); + if self.this_jsvalue.get().is_empty() { + self.this_jsvalue.set(jsc::JsRef::init_weak(out_value)); } from_native(global_this, out_value) } @@ -1512,11 +1538,11 @@ impl NewSource { // the raw refcount via a raw pointer (the call may free `*this`). let this = Box::into_raw(self); // SAFETY: `this` is live — just unwrapped from `Box`. - unsafe { (*this).this_jsvalue.finalize() }; + unsafe { (*this).this_jsvalue.with_mut(jsc::JsRef::finalize) }; // SAFETY: `this` is live; the JS-wrapper +1 (released last) keeps ref_count > 0 // across whatever ref the producer drops in response. unsafe { - if (*this).ref_count > 1 { + if (*this).ref_count.get() > 1 { (*this).context.wrapper_finalized(); } } diff --git a/src/runtime/webcore/Response.rs b/src/runtime/webcore/Response.rs index 79b0121bddab..074b50b7c414 100644 --- a/src/runtime/webcore/Response.rs +++ b/src/runtime/webcore/Response.rs @@ -1,11 +1,8 @@ use core::cell::Cell; -use core::ffi::c_void; use core::mem; -use core::ptr::NonNull; use bun_jsc::JsCell; -use bun_jsc::{AbortSignal, AbortSignalRef, GlobalRef}; -use bun_ptr::RefPtr; +use bun_jsc::{AbortSignal, GlobalRef}; use crate::webcore::jsc::{ BuiltinName, CallFrame, HTTPHeaderName, JSGlobalObject, JSType, JSValue, JsError, JsRef, @@ -24,117 +21,66 @@ use super::{FetchHeaders, ReadableStream, Request}; pub use super::blob::Blob; use bun_ptr::weak_ptr::WeakPtrData; -/// RAII handle to a C++-owned `WebCore::FetchHeaders`. -/// -/// Holds exactly one ref on the C++ intrusive refcount; `Drop` releases it via -/// `WebCore__FetchHeaders__deref`. NOT a `std::rc::Rc` (the payload lives on -/// the C++ heap and is opaque here). -/// -/// Intentionally not `Clone`: the only "share" operation the surface -/// exposes is `clone_this()`, which deep-copies a fresh `FetchHeaders` on the -/// C++ side. Transferring ownership is by-move. -#[repr(transparent)] -pub struct HeadersRef(NonNull); - -impl HeadersRef { - /// Adopt a freshly-created `FetchHeaders*` (refcount already 1). - /// - /// # Safety - /// `ptr` must be a valid `WebCore::FetchHeaders*` and the caller must - /// transfer ownership of one ref. - #[inline] - pub(crate) unsafe fn adopt(ptr: NonNull) -> Self { - Self(ptr) - } - - #[inline] - pub(crate) fn as_ptr(&self) -> *mut FetchHeaders { - self.0.as_ptr() - } - - /// `FetchHeaders.createEmpty()` — fresh C++ allocation, refcount 1. - #[inline] - pub(crate) fn create_empty() -> Self { - // SAFETY: C++ allocates a new FetchHeaders with refcount 1; never null. - unsafe { Self::adopt(FetchHeaders::create_empty()) } - } +pub use bun_jsc::fetch_headers::HeadersRef; - /// `FetchHeaders.createFromUWS(req)` — fresh C++ allocation, refcount 1. - #[inline] - pub(crate) fn create_from_uws(uws_request: *mut core::ffi::c_void) -> Self { - // SAFETY: C++ allocates a new FetchHeaders with refcount 1; never null. - unsafe { Self::adopt(FetchHeaders::create_from_uws(uws_request)) } - } +/// Errors the owning fetch `Response`'s body on abort (Fetch spec "abort a fetch" step 4). +pub(crate) struct BodyAbortListener { + /// Our listener on the signal (and reference on it); dropped with the Response. + registration: Cell>, + /// `Response` owns this, so a ref-counted pointer here would cycle. + response: bun_ptr::ParentRef, + global: GlobalRef, +} - /// `FetchHeaders.createFromJS(global, value)` — may throw, may return null. - #[inline] - pub(crate) fn create_from_js( - global: &JSGlobalObject, - value: JSValue, - ) -> JsResult> { - // SAFETY: C++ returns a +1 ref or null. - Ok(FetchHeaders::create_from_js(global, value)?.map(|p| unsafe { Self::adopt(p) })) - } +/// A counted native reference to a heap [`Response`] (its `ref_count`), +/// released on drop. +pub(crate) struct ResponseRef(bun_ptr::BackRef); - /// `FetchHeaders.cloneThis(global)` — deep copy on the C++ side. - #[inline] - pub(crate) fn clone_this(&self, global: &JSGlobalObject) -> JsResult> { - // SAFETY: C++ returns a +1 ref or null. - Ok(bun_opaque::opaque_deref_mut(self.0.as_ptr()) - .clone_this(global)? - .map(|p| unsafe { Self::adopt(p) })) +impl ResponseRef { + /// One more reference on the heap `Response` behind `response`. + fn retain(response: bun_ptr::ParentRef) -> Self { + response.ref_count.set(response.ref_count.get() + 1); + Self(bun_ptr::BackRef::from( + core::ptr::NonNull::new(response.as_const_ptr().cast_mut()).expect("ParentRef"), + )) } -} -impl core::ops::Deref for HeadersRef { - type Target = FetchHeaders; - #[inline] - fn deref(&self) -> &FetchHeaders { - // `FetchHeaders` is an opaque ZST FFI handle (S008); `self.0` is live - // for the lifetime of `self` — safe `*const → &` via `opaque_deref`. - bun_opaque::opaque_deref(self.0.as_ptr()) + /// Install a [`BodyAbortListener`] so abort reaches this body after `FetchTasklet` has detached. + pub(crate) fn attach_abort_signal(&self, global: &JSGlobalObject, signal: &AbortSignal) { + let listener = bun_ptr::OwnedThis::new(BodyAbortListener { + registration: Cell::new(None), + response: bun_ptr::ParentRef::from(core::ptr::NonNull::from(self.0)), + global: GlobalRef::new(global), + }); + listener + .registration + .set(Some(signal.listen_native(listener.this_ptr().into()))); + self.abort_listener.set(Some(listener)); } } -impl core::ops::DerefMut for HeadersRef { +impl core::ops::Deref for ResponseRef { + type Target = Response; #[inline] - fn deref_mut(&mut self) -> &mut FetchHeaders { - // `FetchHeaders` is an opaque ZST FFI handle (S008); `self.0` is live - // for the lifetime of `self` — safe `*mut → &mut` via `opaque_deref_mut`. - bun_opaque::opaque_deref_mut(self.0.as_ptr()) + fn deref(&self) -> &Response { + self.0.get() } } -impl Drop for HeadersRef { - #[inline] +impl Drop for ResponseRef { fn drop(&mut self) { - // `self.0` is live; releasing our +1 ref via WebCore__FetchHeaders__deref. - // Explicit UFCS to avoid `core::ops::Deref::deref` resolution ambiguity. - // `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref. - FetchHeaders::deref(bun_opaque::opaque_deref_mut(self.0.as_ptr())); + Response::unref(self.0.as_const_ptr().cast_mut()); } } -/// Errors the owning fetch `Response`'s body on abort (Fetch spec "abort a fetch" step 4). -pub(crate) struct BodyAbortListener { - signal: AbortSignalRef, - /// `Response` owns `Box`, so a ref-counted pointer here would cycle. - response: bun_ptr::ParentRef, - global: GlobalRef, -} - -impl BodyAbortListener { - unsafe extern "C" fn on_abort(ctx: *mut c_void, reason: JSValue) { +impl bun_jsc::NativeAbortListener for BodyAbortListener { + fn on_abort(this: bun_ptr::ThisPtr, reason: JSValue) { reason.ensure_still_alive(); - // SAFETY: `ctx` is the `Box` registered in - // `attach_abort_signal`; `clean_native_bindings` removes it before the - // box is dropped, so it is live here. Copy out up front: erroring a - // still-streaming body can re-enter `Response::unref` via - // `FetchTasklet::abandon_response_body` and destroy this box. - let (response, global) = - unsafe { ((*ctx.cast::()).response, (*ctx.cast::()).global) }; - // SAFETY: `response` is live (see above). - let _keepalive = unsafe { RefPtr::init_ref(response.as_mut_ptr()) }; + // Copy out up front: erroring a still-streaming body can re-enter + // `Response::unref` via `FetchTasklet::abandon_response_body` + // and destroy this listener. + let (response, global) = (this.response, this.global); + let _keepalive = ResponseRef::retain(response); if !matches!( response.get_body_value(), BodyValue::Used | BodyValue::Error(_) | BodyValue::Null | BodyValue::Empty @@ -157,14 +103,6 @@ impl BodyAbortListener { } } -impl Drop for BodyAbortListener { - fn drop(&mut self) { - let ctx = core::ptr::from_mut(self).cast::(); - self.signal.clean_native_bindings(ctx); - self.signal.pending_activity_unref(); - } -} - // `jsc.Codegen.JSResponse` — the real bindings, emitted by // `js_class_module!` in `bun_jsc::generated`. pub mod js { @@ -227,7 +165,7 @@ pub struct Response { reported_estimated_size: Cell, /// Fetch's `AbortSignal` listener; survives `FetchTasklet` teardown so a fully-buffered body is still errored. - abort_listener: JsCell>>, + abort_listener: JsCell>>, } impl Default for Response { @@ -490,30 +428,6 @@ impl Response { ::detach_readable_stream(self, global_object) } - /// Install a [`BodyAbortListener`] so abort reaches this body after `FetchTasklet` has detached. - /// - /// SAFETY: `this` must be a live heap `Response` (stored as the listener's [`ParentRef`]). - pub(crate) unsafe fn attach_abort_signal( - this: *mut Response, - global: &JSGlobalObject, - signal: &AbortSignal, - ) { - let signal_ref = signal.ref_(); - signal.pending_activity_ref(); - let mut listener = Box::new(BodyAbortListener { - signal: signal_ref, - // SAFETY: caller contract; `this` is live and owns the box. - response: unsafe { bun_ptr::ParentRef::from_raw_mut(this) }, - global: GlobalRef::new(global), - }); - signal.add_listener( - core::ptr::from_mut(&mut *listener).cast::(), - BodyAbortListener::on_abort, - ); - // SAFETY: caller contract; `this` is live. - unsafe { (*this).abort_listener.set(Some(listener)) }; - } - #[inline] pub(crate) fn set_size_hint(&self, size_hint: super::blob::SizeType) { if let BodyValue::Locked(locked) = self.body.get().value_mut() { @@ -794,6 +708,14 @@ impl Response { unsafe { (*ptr).to_js(global_object) } } + /// Move to the heap and create the JS wrapper (which owns one reference); + /// the returned [`ResponseRef`] is a second, native one. + pub(crate) fn to_js_retained(self, global_object: &JSGlobalObject) -> (JSValue, ResponseRef) { + let ptr = core::ptr::NonNull::from(Box::leak(Box::new(self))); + let native = ResponseRef::retain(bun_ptr::ParentRef::from(ptr)); + (Self::make_maybe_pooled(global_object, ptr.as_ptr()), native) + } + pub(crate) fn clone_value(&self, global_this: &JSGlobalObject) -> JsResult { let body = Body::new(self.clone_body_value_via_cached_stream(global_this)?); // `Body` has NO `Drop`; arm a guard so the diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index a8fcfc002e06..357c91b0f589 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -45,7 +45,7 @@ use bun_core::{String as BunString, Tag as BunStringTag}; use bun_http::{self as http, FetchRedirect, Headers, HeadersExt as _, MimeType}; use bun_http_jsc::method_jsc; use bun_http_types::Method::Method; -use bun_jsc::{HTTPHeaderName, StringJsc as _, SysErrorJsc as _, URLJsc as _}; +use bun_jsc::{GlobalRef, HTTPHeaderName, StringJsc as _, SysErrorJsc as _, URLJsc as _}; use bun_sys::FdExt as _; // `FromJsEnum for FetchRedirect` lives in bun_http_jsc; importing the impl crate // brings the trait impl into scope for `JSValue::get_optional_enum::`. @@ -73,8 +73,8 @@ use bun_s3_signing::{SignOptions, SignResult}; use bun_url::PercentEncoding; use bun_url::URL as ZigURL; +pub use self::fetch_tasklet::FetchTasklet; use self::fetch_tasklet::{FetchOptions, HTTPRequestBody}; -pub use self::fetch_tasklet::{FetchTasklet, FetchTaskletDeinitHop}; // ────────────────────────────────────────────────────────────────────────── // Local extension shims (upstream methods not yet ported / not in scope) @@ -233,32 +233,17 @@ fn bun_fetch_preconnect( } // bun.handleOom(url_str.toOwnedSlice(...)) → to_owned_slice() aborts on OOM. - // `preconnect` takes a `URL<'static>` that borrows a `Box<[u8]>` href and - // assumes ownership when `is_url_owned == true` (it reconstructs the Box - // to free it). Hand the allocation off via `heap::alloc`. - let href_box: Box<[u8]> = url_str.to_owned_slice().into_boxed_slice(); - let href_raw: *mut [u8] = bun_core::heap::into_raw(href_box); - // SAFETY: `href_raw` is a freshly-leaked Box<[u8]>; we either pass ownership - // to `preconnect` (which frees it) or reclaim it on the early-return paths. - let href: &'static [u8] = unsafe { &*href_raw }; - let url = ZigURL::parse(href); - - macro_rules! reclaim_href { - () => { - // SAFETY: paired with the `heap::alloc` above; not yet handed to preconnect. - drop(unsafe { bun_core::heap::take(href_raw) }); - }; - } + let preconnect = + http::async_http::PreparedPreconnect::new(url_str.to_owned_slice().into_boxed_slice()); + let url = preconnect.url(); if !url.is_http() && !url.is_https() && !url.is_s3() { - reclaim_href!(); return Err( global_object.throw_invalid_arguments(format_args!("URL must be HTTP or HTTPS")) ); } if url.hostname.is_empty() { - reclaim_href!(); return Err(global_object .err( jsc::ErrorCode::INVALID_ARG_TYPE, @@ -268,13 +253,10 @@ fn bun_fetch_preconnect( } if !url.has_valid_port() { - reclaim_href!(); return Err(global_object.throw_invalid_arguments(format_args!("Invalid port"))); } - // `preconnect` is a free fn in `bun_http::async_http`. Ownership - // of `href_raw` transfers here (`is_url_owned: true`). - http::async_http::preconnect(url, true); + preconnect.start(); Ok(JSValue::UNDEFINED) } @@ -413,23 +395,12 @@ fn fetch_impl( let mut proxy: Option = None; let mut redirect_type: FetchRedirect = FetchRedirect::Follow; - let signal: Option; + // Our reference on the AbortSignal (set at `'extract_signal`): released on + // every early-return path, or moved into `FetchOptions`. + let mut signal: Option; let mut range: Option = None; let mut unix_socket_path: Box<[u8]> = Box::default(); - // `url_proxy_buffer` gets reassigned while `url`/`proxy` - // still point into it (or into the buffer about to replace it). Detach the - // borrow-checker by parsing through a raw-pointer slice; the caller is - // responsible for keeping the backing allocation alive (it always becomes - // the new `url_proxy_buffer` before the old one is dropped). - macro_rules! parse_url_detached { - ($slice:expr) => {{ - let s: &[u8] = $slice; - // SAFETY: `s` points into a Vec that is immediately adopted as - // `url_proxy_buffer` (or already is it); see note above. - ZigURL::parse(unsafe { bun_ptr::detach_lifetime(s) }) - }}; - } let mut url_type = URLType::Remote; let mut ssl_config: Option = None; @@ -449,24 +420,12 @@ fn fetch_impl( break 'brk None; }; - // kept as raw `*mut Request` because the body re-borrows it - // multiple times across long-lived option/init reads. - let request: Option<*mut Request> = 'brk: { - if first_arg.is_cell() { - if let Some(request_) = first_arg.as_direct::() { - break 'brk Some(request_); - } - } - break 'brk None; + // The JS-owned Request `first_arg` wraps (kept alive by `arguments`). + let request: Option<&Request> = if first_arg.is_cell() { + first_arg.as_direct_class_ref::() + } else { + None }; - // Helper macro: short-lived `&mut Request` reborrow of the optional pointer. - macro_rules! request_mut { - () => { - // SAFETY: `request` was obtained from a live JS-owned Request via - // `as_direct`; each reborrow is non-overlapping at the call site. - request.map(|p| unsafe { &mut *p }) - }; - } // If it's NOT a Request or a subclass of Request, treat the first argument as a URL. let url_str_optional = if first_arg.as_::().is_none() { @@ -493,7 +452,7 @@ fn fetch_impl( break 'extract_url str; } - if let Some(req) = request_mut!() { + if let Some(req) = request { let _ = req.ensure_url(); // bun.handleOom — aborts on OOM break 'extract_url req.url.get().clone(); } @@ -545,7 +504,7 @@ fn fetch_impl( } }; let mut url_proxy_buffer = owned_url.into_href().into_vec(); - let mut url = parse_url_detached!(&url_proxy_buffer[..]); + let mut url = ZigURL::parse(&url_proxy_buffer[..]); if url.is_file() { url_type = URLType::File; } else if url.is_blob() { @@ -562,7 +521,7 @@ fn fetch_impl( } } - if let Some(req) = request_mut!() { + if let Some(req) = request { break 'extract_method Some(req.method); } @@ -777,7 +736,7 @@ fn fetch_impl( // redirect: "follow" | "error" | "manual" | undefined; redirect_type = 'extract_redirect_type: { // First, try to use the Request object's redirect if available - if let Some(req) = request_mut!() { + if let Some(req) = request { redirect_type = req.flags.redirect; } @@ -852,7 +811,7 @@ fn fetch_impl( // proxy: string | { url: string, headers?: Headers } | undefined; let mut proxy_headers: Option = None; // `defer if (proxy_headers) |*hdrs| hdrs.deinit();` → Headers impls Drop. - url_proxy_buffer = 'extract_proxy: { + 'extract_proxy: { let objects_to_try = [ options_object.unwrap_or_default(), request_init_object.unwrap_or_default(), @@ -879,20 +838,21 @@ fn fetch_impl( ), ); } + let url_len = url.href.len(); let mut buffer: Vec = Vec::with_capacity(url_proxy_buffer.len()); buffer.extend_from_slice(&url_proxy_buffer); write!(&mut buffer, "{}", href).expect("write to Vec cannot fail"); - let url_len = url.href.len(); - url = parse_url_detached!(&buffer[0..url_len]); + // allocator.free(url_proxy_buffer) — old Vec dropped on reassign. + url_proxy_buffer = buffer; + url = ZigURL::parse(&url_proxy_buffer[0..url_len]); if url.is_file() { url_type = URLType::File; } else if url.is_blob() { url_type = URLType::Blob; } - proxy = Some(parse_url_detached!(&buffer[url_len..])); - // allocator.free(url_proxy_buffer) — old Vec dropped on reassign. - break 'extract_proxy buffer; + proxy = Some(ZigURL::parse(&url_proxy_buffer[url_len..])); + break 'extract_proxy; } // Handle object format: proxy: { url: "http://proxy.example.com:8080", headers?: Headers } // If the proxy object doesn't have a 'url' property, ignore it. @@ -914,21 +874,21 @@ fn fetch_impl( ), ); } + let url_len = url.href.len(); let mut buffer: Vec = Vec::with_capacity(url_proxy_buffer.len()); buffer.extend_from_slice(&url_proxy_buffer); write!(&mut buffer, "{}", href).expect("write to Vec cannot fail"); - let url_len = url.href.len(); - url = parse_url_detached!(&buffer[0..url_len]); + // allocator.free(url_proxy_buffer) — old Vec dropped on reassign. + url_proxy_buffer = buffer; + url = ZigURL::parse(&url_proxy_buffer[0..url_len]); if url.is_file() { url_type = URLType::File; } else if url.is_blob() { url_type = URLType::Blob; } - proxy = Some(parse_url_detached!(&buffer[url_len..])); - // allocator.free(url_proxy_buffer) — old Vec dropped on reassign. - url_proxy_buffer = buffer; + proxy = Some(ZigURL::parse(&url_proxy_buffer[url_len..])); // Get the headers from the proxy object (optional) if let Some(headers_value) = @@ -951,16 +911,14 @@ fn fetch_impl( } } - break 'extract_proxy url_proxy_buffer; + break 'extract_proxy; } } } } } } - - break 'extract_proxy url_proxy_buffer; - }; + } // signal: AbortSignal | null | undefined; // WebIDL `AbortSignal?` member: present iff not undefined. A present `null` @@ -972,8 +930,8 @@ fn fetch_impl( if signal_.is_null() { break 'extract_signal None; } - if let Some(signal) = AbortSignal::ref_from_js(signal_) { - break 'extract_signal Some(signal); + if let Some(signal__) = AbortSignal::ref_from_js(signal_) { + break 'extract_signal Some(signal__); } let err = ctx.to_type_error( jsc::ErrorCode::INVALID_ARG_TYPE, @@ -988,9 +946,9 @@ fn fetch_impl( } } - if let Some(req) = request_mut!() { + if let Some(req) = request { if let Some(signal_) = req.abort_signal() { - break 'extract_signal Some(signal_.ref_()); + break 'extract_signal Some(signal_.clone()); } break 'extract_signal None; } @@ -1000,8 +958,8 @@ fn fetch_impl( if signal_.is_null() { break 'extract_signal None; } - if let Some(signal) = AbortSignal::ref_from_js(signal_) { - break 'extract_signal Some(signal); + if let Some(signal__) = AbortSignal::ref_from_js(signal_) { + break 'extract_signal Some(signal__); } let err = ctx.to_type_error( jsc::ErrorCode::INVALID_ARG_TYPE, @@ -1040,7 +998,7 @@ fn fetch_impl( } } - if let Some(req) = request_mut!() { + if let Some(req) = request { let body_value = req.get_body_value(); let already_used = match body_value { BodyValue::Used => true, @@ -1149,7 +1107,7 @@ fn fetch_impl( } } - if let Some(req) = request_mut!() { + if let Some(req) = request { if let Some(head) = req.get_fetch_headers_unless_empty() { break 'brk Some(head.as_ptr()); } @@ -1410,7 +1368,7 @@ fn fetch_impl( // Fetch spec step 11: reject synchronously for a pre-aborted signal. Runs // after body/header extraction so Request-constructor errors (GET+body, // already-used body) win and `request.bodyUsed` is set, matching Node. - if let Some(sig) = &signal { + if let Some(sig) = signal.as_deref() { if sig.aborted() { let reason = sig.js_reason(global_this); if let HTTPRequestBody::ReadableStream(stream_ref) = &body { @@ -1698,39 +1656,21 @@ fn fetch_impl( let promise = jsc::JSPromiseStrong::init(global_this); let promise_value = promise.value(); - // `S3StreamWrapper.url` borrows `url_proxy_buffer`; box - // the buffer first (stable heap address) and re-parse so the - // detached-lifetime slices remain valid after the Vec → Box move. - let owned_buffer: Box<[u8]> = core::mem::take(&mut url_proxy_buffer).into_boxed_slice(); let url_len = url.href.len(); - // SAFETY: `owned_buffer` is moved into `s3_stream` alongside the - // re-parsed URL; the slices stay valid for the buffer's lifetime. - let url_static = - ZigURL::parse(unsafe { bun_ptr::detach_lifetime(&owned_buffer[..url_len]) }); - let s3_path = url_static.s3_path(); + let s3_path = url.s3_path(); // Proxy href (if any) lives in the same buffer, immediately after `url`. let proxy_url: Option<&[u8]> = if proxy.is_some() { - // SAFETY: see `url_static` SAFETY note above. - Some(unsafe { bun_ptr::detach_lifetime(&owned_buffer[url_len..]) }) + Some(&url_proxy_buffer[url_len..]) } else { None }; - let s3_stream = Box::new(S3StreamWrapper { - url: url_static, - _url_proxy_buffer: owned_buffer, + let s3_stream = S3StreamWrapper { + url: Box::from(&url_proxy_buffer[..url_len]), promise, - global: global_this, - }); - // Shim: erases both the payload type and the `Result` return when - // coercing to the `fn (S3UploadResult, *mut c_void)` callback shape. - fn s3_stream_wrapper_resolve(result: s3::S3UploadResult<'_>, ctx: *mut libc::c_void) { - // SAFETY: ctx was produced by `heap::alloc(s3_stream)` below; the - // 'static lifetime is a raw-pointer fiction (the pointee's real - // lifetime is managed by the resolve callback itself). - let _ = S3StreamWrapper::resolve(result, ctx.cast::>()); - } + global: GlobalRef::from(global_this), + }; // `dupe()` heap-allocates a fresh intrusive-refcounted copy. // `upload_stream` adopts the ref by value (no extra bump) and the // MultiPartUpload derefs on completion. @@ -1747,10 +1687,12 @@ fn fetch_impl( headers.as_ref().and_then(|h| h.get_content_encoding()), proxy_url, credentials_with_options.request_payer, - Some(s3_stream_wrapper_resolve), - bun_core::heap::into_raw(s3_stream).cast::(), + Some(Box::new(move |result| { + // The outcome reaches the caller through the promise; a + // pending exception from settling it is left for the caller's fold. + let _ = s3_stream.resolve(result); + })), )?; - // url/url_proxy_buffer ownership moved into s3_stream above. return Ok(promise_value); } if method == Method::POST { @@ -1779,21 +1721,20 @@ fn fetch_impl( if let Some(proxy_) = &proxy { // proxy and url are in the same buffer lets replace it - let old_buffer = core::mem::take(&mut url_proxy_buffer); - // `defer allocator.free(old_buffer)` → drop(old_buffer) at end of scope. let mut buffer = vec![0u8; result.url.len() + proxy_.href.len()]; buffer[0..result.url.len()].copy_from_slice(&result.url); buffer[result.url.len()..].copy_from_slice(proxy_.href); + // `defer allocator.free(old_buffer)` → old Vec dropped on reassign. url_proxy_buffer = buffer; - url = parse_url_detached!(&url_proxy_buffer[0..result.url.len()]); - proxy = Some(parse_url_detached!(&url_proxy_buffer[result.url.len()..])); - drop(old_buffer); + url = ZigURL::parse(&url_proxy_buffer[0..result.url.len()]); + proxy = Some(ZigURL::parse(&url_proxy_buffer[result.url.len()..])); } else { + proxy = None; // replace headers and url of the request // allocator.free(url_proxy_buffer) — old Vec dropped on reassign. url_proxy_buffer = core::mem::take(&mut result.url).into(); - url = parse_url_detached!(&url_proxy_buffer[..]); + url = ZigURL::parse(&url_proxy_buffer[..]); // result.url = ""; — fetch now owns this (mem::take above) } @@ -1828,34 +1769,14 @@ fn fetch_impl( let promise_val = promise.value(); - // `FetchOptions.{url,proxy}` are `ZigURL<'static>` borrowing the - // `url_proxy_buffer: Box<[u8]>` stored alongside them — a self-referential - // struct. `Vec::into_boxed_slice` may realloc when `cap > len` (the - // proxy-string path above triggers this), so the existing `url`/`proxy` - // slices may dangle after the conversion. Convert to `Box<[u8]>` first - // (stable heap address), then re-parse the URLs from the boxed buffer. - let url_len = url.href.len(); // fat-pointer len read; no deref + // The tasklet's request re-parses `url` / `proxy` from the buffer it keeps. + let url_len = url.href.len(); let has_proxy = proxy.is_some(); - let url_proxy_boxed: Box<[u8]> = core::mem::take(&mut url_proxy_buffer).into_boxed_slice(); - // SAFETY: `url_proxy_boxed` is moved into `FetchOptions` alongside the URLs - // that borrow it; `FetchTasklet` keeps the buffer alive for as long as the - // URLs are read. Erase the borrow to a raw slice so borrowck doesn't tie - // `url_static` to the local `url_proxy_boxed` binding. - let buf_ptr: *const [u8] = &raw const *url_proxy_boxed; - // SAFETY: `buf_ptr` points into `url_proxy_boxed` which the FetchTasklet - // keeps alive for the lifetime of the parsed URLs (see comment above). - // Explicit `&*` first to satisfy `dangerous_implicit_autorefs` — the - // `Index` call would otherwise create an implicit `&` to `*buf_ptr`. - let buf: &'static [u8] = unsafe { &*buf_ptr }; - let url_static: ZigURL<'static> = ZigURL::parse(&buf[..url_len]); - let proxy_static: Option> = if has_proxy { - Some(ZigURL::parse(&buf[url_len..])) - } else { - None - }; let fetch_options = FetchOptions { method, - url: url_static, + url_proxy_buffer: url_proxy_buffer.into_boxed_slice(), + url_len, + has_proxy, headers: headers.take().unwrap_or_default(), body, disable_keepalive, @@ -1866,10 +1787,8 @@ fn fetch_impl( reject_unauthorized, redirect_type, verbose, - proxy: proxy_static, proxy_headers: proxy_headers.take(), - url_proxy_buffer: url_proxy_boxed, - signal, + signal: signal.take(), ssl_config: ssl_config.take(), upgraded_connection, forced_protocol, @@ -1904,21 +1823,17 @@ fn fetch_impl( // `impl` blocks inside fn bodies for types referenced by external fn pointers. // ────────────────────────────────────────────────────────────────────────── -struct S3StreamWrapper<'a> { +struct S3StreamWrapper { promise: jsc::JSPromiseStrong, - url: ZigURL<'a>, - _url_proxy_buffer: Box<[u8]>, - global: &'a JSGlobalObject, + /// The request URL's href. + url: Box<[u8]>, + global: GlobalRef, } -impl<'a> S3StreamWrapper<'a> { - fn resolve(result: s3::S3UploadResult, self_: *mut Self) -> JsResult<()> { - // SAFETY: self_ was created via heap::alloc in fetch_impl; we reclaim - // ownership here exactly once on the resolve callback. - let mut self_ = unsafe { bun_core::heap::take(self_) }; - let global = self_.global; - // `defer bun.destroy(self)` + `defer free(url_proxy_buffer)` → - // Box and Box<[u8]> Drop at end of scope. +impl S3StreamWrapper { + fn resolve(self, result: s3::S3UploadResult) -> JsResult<()> { + let mut self_ = self; + let global: &JSGlobalObject = &self_.global; match result { s3::S3UploadResult::Success => { let response = Box::new(Response::init( @@ -1928,11 +1843,10 @@ impl<'a> S3StreamWrapper<'a> { ..Default::default() }, Body::new(BodyValue::Empty), - BunString::create_atom_if_possible(self_.url.href), + BunString::create_atom_if_possible(&self_.url), false, )); - // SAFETY: `into_raw` yields a freshly allocated heap `Response`; - // ownership transfers to JSC. + // Ownership of the freshly boxed `Response` transfers to JSC. let response_js = Response::make_maybe_pooled(global, bun_core::heap::into_raw(response)); response_js.ensure_still_alive(); @@ -1950,12 +1864,11 @@ impl<'a> S3StreamWrapper<'a> { bytes: err.message.to_vec(), was_string: true, })), - BunString::create_atom_if_possible(self_.url.href), + BunString::create_atom_if_possible(&self_.url), false, )); - // SAFETY: `into_raw` yields a freshly allocated heap `Response`; - // ownership transfers to JSC. + // Ownership of the freshly boxed `Response` transfers to JSC. let response_js = Response::make_maybe_pooled(global, bun_core::heap::into_raw(response)); response_js.ensure_still_alive(); diff --git a/src/runtime/webcore/fetch/FetchRequestBodySink.rs b/src/runtime/webcore/fetch/FetchRequestBodySink.rs index e020564c83ab..5f7112a095a8 100644 --- a/src/runtime/webcore/fetch/FetchRequestBodySink.rs +++ b/src/runtime/webcore/fetch/FetchRequestBodySink.rs @@ -1,5 +1,8 @@ +use core::cell::Cell; + use bun_collections::ByteVecExt; -use bun_ptr::BackRef; +use bun_jsc::JsCell; +use bun_ptr::{BackRef, Root}; use bun_sys::Error as SysError; use crate::webcore::blob::SizeType as BlobSizeType; @@ -50,34 +53,33 @@ impl<'a> RequestBodyChunk<'a> { /// JSSink streaming a fetch() request body into the HTTP thread's ThreadSafeStreamBuffer /// with chunked framing; one write per drain-ack. pub struct FetchRequestBodySink { - /// Non-owning back-reference; `FetchTasklet` is kept alive by a +1 - /// intrusive ref taken in `start_request_stream` and released exactly once - /// by the `assign_to_stream`-result path (`on_resolve_request_stream` / - /// `on_reject_request_stream` / synchronous branches), which clears this to - /// `None` first. `finalize` releases it as a fallback if that path never - /// ran. - pub task: Option>, - pub source: SourceHandle, - pub high_water_mark: BlobSizeType, + /// The tasklet that owns this sink. While `Some`, the tasklet's + /// `request_stream_ref` is held; the `assign_to_stream`-result path + /// (`on_resolve_request_stream` / `on_reject_request_stream` / synchronous + /// branches) clears this first and then releases that ref, and `finalize` + /// releases it as a fallback if that path never ran. + pub task: Cell>>, + pub source: Cell, + pub high_water_mark: Cell, /// Shared pending drain promise for `write()` and `flush(true)`; resolved /// in `on_drain()`. - pub pending: WritablePending, + pub pending: JsCell, /// Bytes written since last on_drain; >0 guarantees a drain ack is owed. - pub pending_bytes: BlobSizeType, - pub ended: bool, - pub done: bool, + pub pending_bytes: Cell, + pub ended: Cell, + pub done: Cell, } impl Default for FetchRequestBodySink { fn default() -> Self { Self { - task: None, - source: SourceHandle::default(), - high_water_mark: 16384, - pending: WritablePending::default(), - pending_bytes: 0, - ended: false, - done: false, + task: Cell::new(None), + source: Cell::new(SourceHandle::default()), + high_water_mark: Cell::new(16384), + pending: JsCell::new(WritablePending::default()), + pending_bytes: Cell::new(0), + ended: Cell::new(false), + done: Cell::new(false), } } } @@ -85,37 +87,32 @@ impl Default for FetchRequestBodySink { impl FetchRequestBodySink { pub const NAME: &'static str = "FetchRequestBodySink"; - /// Exclusive borrow of the owning `FetchTasklet`, if still attached. - /// - /// SAFETY (invariant): the tasklet is intrusively ref-counted and this - /// sink holds (indirectly, via the `+1` taken in `start_request_stream`) a - /// counted ref while `task` is `Some`; JS-thread-only so no concurrent - /// `&mut` exists. - #[inline] - fn task_mut(&mut self) -> Option<&mut FetchTasklet> { - // SAFETY: see doc comment — exclusive while `&mut self` held. - self.task.as_mut().map(|p| unsafe { p.get_mut() }) + pub fn is_native_source(&self) -> bool { + matches!( + self.source.get(), + SourceHandle::ByteStream(_) | SourceHandle::FileReader(_) + ) } pub fn start(&mut self, stream_start: &Start) -> bun_sys::Result<()> { - if self.ended { + if self.ended.get() { return bun_sys::Result::Ok(()); } if let &Start::ChunkSize(chunk_size) = stream_start { if chunk_size > 0 { - self.high_water_mark = chunk_size; + self.high_water_mark.set(chunk_size); } } - self.source.start(); + self.source.get_mut().start(); bun_sys::Result::Ok(()) } fn write_chunk(&mut self, chunk: RequestBodyChunk<'_>) -> Writable { - if self.ended { + if self.ended.get() { return Writable::Done; } - let high_water_mark = self.high_water_mark; - let result = match self.task_mut() { + let high_water_mark = self.high_water_mark.get(); + let result = match self.task.get() { Some(task) => task.write_request_data(chunk, high_water_mark as usize), None => return Writable::Done, }; @@ -124,11 +121,9 @@ impl FetchRequestBodySink { Writable::Backpressure(len) => (len, true), other => return other, }; - self.pending_bytes = self.pending_bytes.saturating_add(len); - if matches!( - self.source, - SourceHandle::ByteStream(_) | SourceHandle::FileReader(_) - ) { + self.pending_bytes + .set(self.pending_bytes.get().saturating_add(len)); + if self.is_native_source() { // Native sources are push-driven by a macrotask (on_body_received); // only park them when the cross-thread buffer is actually over HWM. return if backed_up { @@ -141,9 +136,10 @@ impl FetchRequestBodySink { // on each so the event loop reaches the I/O poll before resuming; batching // sync writes here lets the HTTP thread re-enqueue the drain ack before // the microtask loop yields and starves uWS callbacks and timers. - self.pending.consumed = len; - self.pending.result = Writable::Owned(len); - Writable::Pending(core::ptr::from_mut(&mut self.pending)) + let pending = self.pending.as_mut(); + pending.consumed = len; + pending.result = Writable::Owned(len); + Writable::Pending(core::ptr::from_mut(pending)) } pub fn write(&mut self, data: &StreamResult) -> Writable { @@ -174,23 +170,24 @@ impl FetchRequestBodySink { wait: bool, ) -> bun_sys::Result { use crate::webcore::streams::PendingState; - if self.pending.state == PendingState::Pending { + let pending = self.pending.as_mut(); + if pending.state == PendingState::Pending { return bun_sys::Result::Ok( - JSPromise::opaque_ref(self.pending.promise(global_this)).to_js(), + JSPromise::opaque_ref(pending.promise(global_this)).to_js(), ); } - if self.done || self.ended { + if self.done.get() || self.ended.get() { return bun_sys::Result::Ok(JSPromise::resolved_promise_value( global_this, JSValue::js_number(0.0), )); } - if wait && self.pending_bytes > 0 { + if wait && self.pending_bytes.get() > 0 { // Bytes were scheduled to the HTTP thread since the last drain ack, // so an `on_drain` is guaranteed to arrive and resolve this. - self.pending.result = Writable::Owned(self.pending_bytes); + pending.result = Writable::Owned(self.pending_bytes.get()); return bun_sys::Result::Ok( - JSPromise::opaque_ref(self.pending.promise(global_this)).to_js(), + JSPromise::opaque_ref(pending.promise(global_this)).to_js(), ); } bun_sys::Result::Ok(JSPromise::resolved_promise_value( @@ -208,25 +205,21 @@ impl FetchRequestBodySink { /// `StreamError` so a JS-valued upstream error (e.g. fetch reset) reaches /// `write_end_request(Some(js))` instead of being silently dropped to EOF. pub fn end_from_stream(&mut self, err: Option) { - if self.ended { + if self.ended.get() { return; } - self.ended = true; - if matches!( - self.source, - SourceHandle::ByteStream(_) | SourceHandle::FileReader(_) - ) { + self.ended.set(true); + if self.is_native_source() { // Native source drove this call and already cleared its own `sink` // field; detach (not cancel) so we don't re-enter the source while // it is still on the stack (FileReader.on_reader_error ref-leak). - self.source.clear(); - if let Some(mut task) = self.task.take() { + self.source.set(SourceHandle::None); + if let Some(task) = self.task.take() { let err_js = err.map(|e| e.to_js(&task.global_this)); - // SAFETY: the `+1` taken in `start_request_stream` keeps the - // tasklet live while `task` was `Some`; `write_end_request` is - // the balancing release and may free `*self` via `clear_sink`, - // so do not touch `self` afterwards. - unsafe { task.get_mut() }.write_end_request(err_js); + // Releases the tasklet's `request_stream_ref`, which may free the + // tasklet and, through `clear_sink`, `*self`: do not touch `self` + // afterwards. + FetchTasklet::write_end_request(task.this_ptr(), err_js); } return; } @@ -235,7 +228,7 @@ impl FetchRequestBodySink { Some(StreamError::Error(e)) => Some(e), _ => None, }; - self.source.close(sys_err); + self.source.get_mut().close(sys_err); } pub fn end_from_js(&mut self, _global_this: &JSGlobalObject) -> bun_sys::Result { @@ -246,29 +239,40 @@ impl FetchRequestBodySink { /// # Safety /// `this` must be live and must not be used after the call: the tasklet /// owns this allocation, and if the ref released here was its last one, - /// its `deinit` → `clear_sink` frees `*this`. + /// dropping it (`clear_sink`) frees `*this`. pub unsafe fn finalize(this: *mut Self) { // SAFETY: caller contract; `this` is not touched again after this line. let task = unsafe { (*this).task.take() }; if let Some(task) = task { - // Balances the `ref_()` taken in `start_request_stream` when the - // assign_to_stream-result handler never ran to release it. - FetchTasklet::deref(task.as_ptr()); + // Releases `request_stream_ref` when the assign_to_stream-result + // handler never ran to release it. + FetchTasklet::release_request_stream_ref(task.this_ptr()); } } + /// Settle the pending write/flush promise with `Done` and fire the source's + /// close (which cancels the upstream reader). + pub(crate) fn cancel(&self) { + self.pending + .with_mut(|pending| pending.result = Writable::Done); + self.pending.with_mut(WritablePending::run); + let mut source = self.source.get(); + source.close(None); + } + /// HTTP-thread drain ack: resolves the pending write/flush promise and wakes source.ready(). - pub fn on_drain(&mut self, _global_this: &JSGlobalObject) { + pub fn on_drain(&self, _global_this: &JSGlobalObject) { bun_core::scoped_log!(FetchRequestBodySinkLog, "onDrain"); - self.pending_bytes = 0; - self.pending.run(); - self.source.ready(None, None); + self.pending_bytes.set(0); + self.pending.with_mut(WritablePending::run); + let mut source = self.source.get(); + source.ready(None, None); } pub fn memory_cost(&self) -> usize { - if let Some(task) = self.task.as_ref() { - if let Some(buf) = task.stream_buffer_mut() { - return buf.lock().size(); + if let Some(task) = self.task.get() { + if let Some(buffer) = task.request_body_buffer() { + return buffer.lock().size(); } } 0 @@ -292,6 +296,6 @@ impl crate::webcore::sink::JsSinkType for FetchRequestBodySink { Self::end_from_js(self, global) } fn source(&mut self) -> Option<&mut SourceHandle> { - Some(&mut self.source) + Some(self.source.get_mut()) } } diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index fd72c2b4d30b..d652747861fc 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -1,155 +1,414 @@ -use core::ffi::c_void; +use core::cell::Cell; use core::ptr::NonNull; -use core::sync::atomic::{AtomicBool, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; +use std::sync::Arc; -use bun_boringssl as boringssl; use bun_cares_sys::c_ares_draft as c_ares; use bun_core::{MutableString, String as BunString}; -use bun_event_loop::{ - ConcurrentTask::{AutoDeinit, ConcurrentTask}, - Task, Taskable, -}; +use bun_event_loop::task_tag; +use bun_event_loop::{ConcurrentTask::ConcurrentTask, ReusableConcurrentTask, Task, TaskHop}; use bun_http as http; use bun_http::Method; use bun_http::{ AsyncHTTP, CertificateInfo, FetchRedirect, HTTPClientResult, HTTPResponseMetadata, Headers, - Signals, ThreadSafeStreamBuffer, + InFlight, OwnedRequest, ThreadSafeStreamBuffer, }; use bun_io::KeepAlive; use bun_jsc::bun_string_jsc; use bun_jsc::debugger::AsyncTaskTracker; -use bun_jsc::{self as jsc, GlobalRef, JSGlobalObject, JSValue, JsCell, JsResult, StrongOptional}; -use bun_ptr::RefPtr; +use bun_jsc::{ + self as jsc, AbortListenerRegistration, AbortSignalRef, GlobalRef, JSGlobalObject, JSValue, + JsCell, JsResult, StrongOptional, +}; +use bun_ptr::{BackRef, OwnedThis, RefPtr, SelfRoot, ThisPtr}; use bun_sys::FdExt; -use bun_threading::Mutex; +use bun_threading::Guarded; use bun_url::URL as ZigURL; use crate::api::bun_x509 as X509; use crate::webcore::blob::{Any as AnyBlob, Blob, SizeType as BlobSizeType, Store as BlobStore}; use crate::webcore::body::{self, Body, Value as BodyValue, ValueError as BodyValueError}; +use crate::webcore::byte_stream::{AfterDelivery, ProducerHold}; use crate::webcore::fetch::fetch_request_body_sink::{FetchRequestBodySink, RequestBodyChunk}; use crate::webcore::readable_stream::{ReadableStream, Strong as ReadableStreamStrong}; -use crate::webcore::response::HeadersRef; +use crate::webcore::response::{HeadersRef, ResponseRef}; use crate::webcore::sink::JSSink; use crate::webcore::streams::{SourceHandle, StreamError, StreamResult, Writable}; -use crate::webcore::{AbortSignal, DrainResult, FetchHeaders, InternalBlob, Response, SinkHandle}; -use bun_jsc::AbortSignalRef; - -// `bun_event_loop::JsResult` (cycle-broken erased error) — used by -// ConcurrentTask callbacks at the tier-3 layer. -type ElJsResult = bun_event_loop::JsResult; - -use http::signals::BODY_HIGH_WATER_MARK; - -use boringssl::c::{X509_free, d2i_X509}; - -// ConcurrentTask::from() needs `Taskable`; tag is declared in bun_event_loop -// but the impl lives next to the type (cycle-break). -/// The "last ref dropped on the HTTP thread → deinit on the JS thread" hop: -/// same pointer, its own tag, so teardown can tell it from a progress update. -#[repr(transparent)] -pub struct FetchTaskletDeinitHop(FetchTasklet); -impl Taskable for FetchTaskletDeinitHop { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::FetchTaskletDeinit; - /// The last ref dropped on the HTTP thread while we were tearing down: - /// deinit here, on the JS thread with the heap alive, as the hop intended. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract. - unsafe { Self::run(this) } +use crate::webcore::{ByteStream, DrainResult, InternalBlob, Response, SinkHandle}; + +bun_output::declare_scope!(FetchTasklet, visible); + +/// Upper bound on the Content-Length-driven `reserve_exact` in `on_result()`. +const SCHEDULED_PRERESERVE_MAX: usize = 256 * 1024 * 1024; + +use http::signals::{BODY_HIGH_WATER_MARK, BodyReceiveMode}; + +/// What the request out on the HTTP thread borrows: kept, untouched, until the +/// HTTP thread hands the request back (`InFlight`). +pub(crate) struct FetchRequestStorage { + /// url + proxy href, back to back. + url_proxy_buffer: Box<[u8]>, + url_len: usize, + headers_buf: Vec, + unix_socket_path: Box<[u8]>, + /// The in-memory request body (`HTTPRequestBody::AnyBlob`); empty otherwise. + body: AnyBlob, +} + +impl FetchRequestStorage { + fn url(&self) -> ZigURL<'_> { + ZigURL::parse(&self.url_proxy_buffer[..self.url_len]) } } -impl FetchTaskletDeinitHop { - /// # Safety - /// `this` is the tasklet the hop was created from, ref_count == 0, JS thread. - pub(crate) unsafe fn run(this: *mut Self) { - // SAFETY: fn contract — sole owner. - drop(unsafe { bun_core::heap::take(this.cast::()) }); + +/// Response-side state the HTTP thread writes (`FetchShared::on_result`) and the +/// JS thread consumes (`FetchTasklet::on_progress_update`). +#[derive(Default)] +pub(crate) struct SharedState { + pub(crate) result: HTTPClientResult<'static>, + pub(crate) metadata: Option, + /// buffer used to stream response to JS + pub(crate) scheduled_response_buffer: MutableString, + /// The HTTP thread is done with the fetch (terminal result delivered): the + /// progress update that sees this releases `http_ref` too. Fetches with a + /// streaming request body post a `HandBackHop` instead (see `on_result`). + handed_back: bool, + /// `process.exit()` interrupted the request on the HTTP thread + /// (`release_at_shutdown`): nothing more will arrive, so the next progress + /// update only releases the tasklet. + released_at_shutdown: bool, +} + +/// The part of a fetch shared with the HTTP thread. `bun_http` holds it (as the +/// request's result handler) until the terminal result; the tasklet for its +/// whole life. +pub(crate) struct FetchShared { + pub(crate) state: Guarded, + pub(crate) signal_store: http::signals::Store, + /// A progress update is queued on the JS thread and has not run yet; + /// `on_result` sets it (compare-exchange) and `on_progress_update` clears it + /// under `state`'s lock. + has_schedule_callback: AtomicBool, + /// The latest result's `is_http2` (the streaming request body goes out + /// unframed): read by `write_request_data` for every request-body chunk + /// without taking `state`'s lock, which `on_progress_update` may be holding + /// on the same thread while it pumps that body. + is_http2: AtomicBool, + /// The tasklet's address, which the JS-thread hops carry; set once in + /// `queue`. The tasklet keeps itself alive for them through `progress_ref` + /// / `http_ref`. + tasklet: AtomicPtr, + /// The progress-update hop's queue node (at most one is queued at a time: + /// `has_schedule_callback`). + progress_task: ReusableConcurrentTask, + /// The hand-back hop's queue node (posted once, if `posts_drain_hops`). + hand_back_task: ReusableConcurrentTask, + /// Request-body drain hops are posted (a streaming request body), so the + /// hand-back must be its own hop, queued after them. + posts_drain_hops: bool, + /// How the HTTP thread posts to the JS thread, and what makes the VM wait + /// for it; handed back at the terminal result / `release_at_shutdown`. + ticket: jsc::InFlightTicket, +} + +impl FetchShared { + fn hop(&self, tag: bun_event_loop::TaskTag) -> Task { + Task::new(tag, self.tasklet.load(Ordering::Relaxed).cast::<()>()) + } + + /// Post the progress-update hop unless one is queued already. + fn post_progress_update(&self) { + if self + .has_schedule_callback + .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) + .is_err() + { + return; + } + let task = self.hop(task_tag::FetchTasklet); + // `has_schedule_callback` was clear, so the previous hop has run and its + // node was consumed; the heap node is only a fallback. + let node = self + .progress_task + .arm(task) + .unwrap_or_else(|| ConcurrentTask::create(task)); + self.ticket.post(node); } } -impl Taskable for FetchTasklet { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::FetchTasklet; - /// A progress hop the HTTP thread posted: it carries the +1 that - /// `on_progress_update` would have dropped. The HTTP thread's own +1 is - /// released only after its last touch of the tasklet, so a 1→0 here means - /// it is done with it, and this runs on the JS thread with the heap alive. - unsafe fn release_unrun(this: *mut Self) { - FetchTasklet::deref(this); +impl http::HTTPClientResultHandler for FetchShared { + /// HTTP thread: fold `result` into the shared state and, unless one is + /// already queued, post a progress update to the JS thread. The terminal + /// result also hands the tasklet back: through `handed_back`, which the + /// progress update that sees it acts on, or — when request-body drain hops + /// may be queued — as a `HandBackHop` queued after them. + fn on_result(&self, mut result: HTTPClientResult<'_>) { + let is_done = !result.has_more; + let mut state = self.state.lock(); + + bun_output::scoped_log!( + FetchTasklet, + "callback success={} receive_mode={:?} has_more={} bytes={}", + result.is_success(), + self.signal_store.body_receive_mode(), + result.has_more, + result.body.len() + ); + + let prev_metadata = state.result.metadata.take(); + let prev_cert_info = state.result.certificate_info.take(); + let prev_can_stream = state.result.can_stream; + // `result.body` borrows the HTTP thread's scratch buffer on non-terminal + // callbacks; the terminal callback carries the bytes in `body_owned` + // instead. Capture both before `into_owned` drops the borrowed view. + let body: &[u8] = result.body; + let body_owned: Vec = core::mem::take(&mut result.body_owned); + state.result = result.into_owned(); + // can_stream is a one-shot signal to start the request body stream; don't let a + // later coalesced result clobber it before the JS thread sees it. + state.result.can_stream = state.result.can_stream || prev_can_stream; + self.is_http2 + .store(state.result.is_http2, Ordering::Relaxed); + + // Preserve pending certificate info if it was provided in the previous update. + if state.result.certificate_info.is_none() { + if let Some(cert_info) = prev_cert_info { + state.result.certificate_info = Some(cert_info); + } + } + + // metadata should be provided only once + if let Some(metadata) = state.result.metadata.take().or(prev_metadata) { + bun_output::scoped_log!(FetchTasklet, "added callback metadata"); + if state.metadata.is_none() { + state.metadata = Some(metadata); + } + + state.result.metadata = None; + } + + let success = state.result.is_success(); + + if self.signal_store.body_receive_mode() == BodyReceiveMode::Abandoned { + if state.scheduled_response_buffer.list.capacity() > 0 { + state.scheduled_response_buffer = MutableString::default(); + } + if success && state.result.has_more { + return; + } + } else if success { + let has_more = state.result.has_more; + let body_size = state.result.body_size; + let scheduled = &mut state.scheduled_response_buffer; + if body.is_empty() && !body_owned.is_empty() && scheduled.list.is_empty() { + scheduled.list = body_owned; + } else { + // Grow to Content-Length once so the per-packet append below + // doesn't leave the ~2x doubling over-capacity that the + // ArrayBuffer would adopt. Only for a consumer that wants the whole body. + if self.signal_store.body_receive_mode() == BodyReceiveMode::BufferAll { + if let http::BodySize::ContentLength(n) = body_size { + if n > scheduled.list.capacity() { + let additional = n + .min(SCHEDULED_PRERESERVE_MAX) + .saturating_sub(scheduled.list.len()); + let _ = scheduled.list.try_reserve_exact(additional); + } + } + } + let chunk = if body.is_empty() { + body_owned.as_slice() + } else { + body + }; + if !chunk.is_empty() { + bun_core::handle_oom(scheduled.write(chunk)); + } + } + // The other half of this rule is `FetchTasklet::after_body_chunk_delivered`. + if has_more && scheduled.list.len() >= BODY_HIGH_WATER_MARK { + self.signal_store.pause_receive(); + } + } + + if is_done { + self.hand_back(&mut state); + } + self.post_progress_update(); + drop(state); + if is_done { + // The HTTP thread is done with this fetch. + self.ticket.hand_back(); + } + } + + /// Called from `dealloc_in_flight_for_exit` on the HTTP thread for each + /// request still in flight when `process.exit()` interrupts it. The + /// terminal `on_result` will never run, so post what it would have: the + /// hand-back, and — unless one is already queued — a last progress update, + /// which `released_at_shutdown` turns into just the release of the JS + /// side's reference. A queued update's VM releases it from its queue if it + /// never runs. + /// + /// Only reachable for a request whose VM has *not* torn down (a worker + /// still running when the main thread exits): a VM's teardown waits for its + /// fetches' tickets — i.e. for their terminal result — before the exiting + /// main thread parks the HTTP thread. + fn release_at_shutdown(&self) { + { + let mut state = self.state.lock(); + // No JS-thread drain will reclaim it. + state.scheduled_response_buffer = MutableString::default(); + state.released_at_shutdown = true; + self.hand_back(&mut state); + } + self.post_progress_update(); + // The HTTP thread is done with this fetch. + self.ticket.hand_back(); } } -bun_output::declare_scope!(FetchTasklet, visible); +impl FetchShared { + /// HTTP thread, `state` locked, nothing more to deliver: let the JS thread + /// release `http_ref`. + fn hand_back(&self, state: &mut SharedState) { + if self.posts_drain_hops { + // Its own hop: FIFO after every `RequestBodyDrainHop` this thread + // posted, which is what keeps the tasklet alive for those. + let task = self.hop(task_tag::FetchTaskletHandBack); + let node = self + .hand_back_task + .arm(task) + .unwrap_or_else(|| ConcurrentTask::create(task)); + self.ticket.post(node); + } else { + state.handed_back = true; + } + } +} -/// Upper bound on the Content-Length-driven `reserve_exact` in `callback()`. -const SCHEDULED_PRERESERVE_MAX: usize = 256 * 1024 * 1024; +impl http::DrainHandler for FetchShared { + /// HTTP thread, with the request body buffer locked: it drained. Hop to the + /// JS thread to resume the request body stream. Carries no reference of its + /// own: the later `HandBackHop` from this thread is what releases `http_ref`. + fn on_drain(&self) { + self.ticket.post(ConcurrentTask::create( + self.hop(task_tag::FetchTaskletRequestDataDrain), + )); + } +} -use http::signals::BodyReceiveMode; +/// `task_tag::FetchTasklet`: a progress update the HTTP thread posted. +pub struct ProgressHop; +impl TaskHop for ProgressHop { + type Target = FetchTasklet; + const TAG: bun_event_loop::TaskTag = task_tag::FetchTasklet; + fn run(this: ThisPtr) -> JsResult<()> { + FetchTasklet::on_progress_update(this) + } + /// The VM is tearing down (its wait for the ticket is over, so the fetch was + /// handed back): release what the update would have. + fn release_unrun(this: ThisPtr) { + let handed_back = this.shared.state.lock().handed_back; + if handed_back { + FetchTasklet::release(this, |t| &t.http_ref); + } + FetchTasklet::release(this, |t| &t.progress_ref); + } +} -#[derive(bun_ptr::ThreadSafeRefCounted)] +/// `task_tag::FetchTaskletHandBack`: the HTTP thread is done with a fetch that +/// streamed its request body (posted after its last touch of anything the +/// tasklet can reach). +pub struct HandBackHop; +impl TaskHop for HandBackHop { + type Target = FetchTasklet; + const TAG: bun_event_loop::TaskTag = task_tag::FetchTaskletHandBack; + fn run(this: ThisPtr) -> JsResult<()> { + FetchTasklet::release(this, |t| &t.http_ref); + Ok(()) + } + fn release_unrun(this: ThisPtr) { + FetchTasklet::release(this, |t| &t.http_ref); + } +} + +/// `task_tag::FetchTaskletRequestDataDrain`: the streaming request body buffer +/// drained; resume the stream feeding it. +pub struct RequestBodyDrainHop; +impl TaskHop for RequestBodyDrainHop { + type Target = FetchTasklet; + const TAG: bun_event_loop::TaskTag = task_tag::FetchTaskletRequestDataDrain; + fn run(this: ThisPtr) -> JsResult<()> { + FetchTasklet::resume_request_data_stream(this); + Ok(()) + } + /// Nothing held: the tasklet outlives this hop through `http_ref`. + fn release_unrun(_this: ThisPtr) {} +} + +/// The per-`fetch()` state machine. Lives on the JS thread; the HTTP thread +/// only sees [`FetchShared`]. Reference-counted: `progress_ref` (released by +/// the terminal progress update), `http_ref` (released when the HTTP thread +/// hands the request back) and `request_stream_ref` (held while a request body +/// stream is wired to the sink) are the references queued work holds on it. +#[derive(bun_ptr::CellRefCounted)] pub struct FetchTasklet { - // Heap-allocated `FetchRequestBodySink` (a `JSSink`). FetchTasklet owns the - // allocation from `start_request_stream` until `clear_sink`; the JS - // controller holds only a detachable back-pointer into it. - pub sink: Option>, - // Self-referential: borrows from `request_body` / `request_headers` owned - // by sibling fields, so the lifetime is erased to `'static`. - pub(crate) http: Option>>, - pub(crate) result: HTTPClientResult<'static>, - pub(crate) metadata: Option, - /// Held while the request is out on the HTTP thread (`queue` until its - /// final callback / `release_at_shutdown`): how that thread posts progress - /// and deinit tasks, and what makes the VM wait for it. JS-thread code uses - /// the VM through `global_this` instead and never touches this. - pub(crate) http_ticket: Option, + ref_count: Cell, + /// `&self` paths reach the `ThisPtr`-taking ones through this. + self_ref: SelfRoot, + pub(crate) shared: Arc, + /// The request out on (or back from) the HTTP thread; taken back on drop. + request: JsCell>>, + method: Method, + /// The request-body `JSSink`, from `start_request_stream` until + /// `clear_sink`; the JS controller holds only a detachable pointer into it. + pub(crate) sink: JsCell>>, pub global_this: GlobalRef, - pub(crate) request_body: HTTPRequestBody, - /// This side's ref; the HTTP thread holds the other of the two initial refs. - pub(crate) request_body_streaming_buffer: Option>, - - /// buffer used to stream response to JS - pub(crate) scheduled_response_buffer: MutableString, + /// `Sendfile` / `ReadableStream` request bodies (an in-memory body lives in + /// the request storage). + pub(crate) request_body: JsCell, + /// Our reference on the request-body buffer shared with the HTTP thread; + /// released in `clear_sink`. + request_body_streaming_buffer: JsCell>>, /// response weak ref we need this to track the response JS lifetime - pub(crate) response: jsc::Weak, + pub(crate) response: JsCell>, /// native response ref if we still need it when JS is discarted - pub(crate) native_response: JsCell>>, + pub(crate) native_response: JsCell>, /// The response body stream while this tasklet is its producer. - pub(crate) response_stream: crate::webcore::byte_stream::ProducerHold, - pub(crate) request_headers: Headers, - pub(crate) promise: jsc::JSPromiseStrong, - pub(crate) concurrent_task: ConcurrentTask, - /// `JsCell`: the ByteStream's drain signal reaches `on_stream_drained` through a shared ref. + pub(crate) response_stream: ProducerHold, + pub(crate) promise: JsCell, pub poll_ref: JsCell, - /// For Http Client requests - /// when Content-Length is provided this represents the whole size of the request - /// If chunked encoded this will represent the total received size (ignoring the chunk headers) - /// If is not chunked encoded and Content-Length is not provided this will be unknown - pub(crate) body_size: http::BodySize, - - /// This is url + proxy memory buffer and is owned by FetchTasklet - /// We always clone url and proxy (if informed) - pub(crate) url_proxy_buffer: Box<[u8]>, - - pub(crate) signal: Option, - pub(crate) signals: Signals, - pub(crate) signal_store: http::signals::Store, - pub(crate) has_schedule_callback: AtomicBool, - + /// Our `abort` listener on the request's signal (and our handle on the + /// signal); dropped once the abort has been consumed or at teardown. + pub(crate) signal: JsCell>, // must be stored because AbortSignal stores reason weakly - pub(crate) abort_reason: StrongOptional, - + pub(crate) abort_reason: JsCell, // custom checkServerIdentity - pub(crate) check_server_identity: StrongOptional, + pub(crate) check_server_identity: JsCell, pub(crate) reject_unauthorized: bool, pub(crate) upgraded_connection: bool, - pub(crate) unix_socket_path: Box<[u8]>, - pub(crate) is_waiting_body: bool, - pub(crate) is_waiting_abort: bool, - pub(crate) is_waiting_request_stream_start: bool, - pub(crate) mutex: Mutex, - + /// The user set Content-Length without Transfer-Encoding: a streamed + /// request body goes out unframed (`skip_chunked_framing`). + unframed_by_headers: bool, + pub(crate) is_waiting_body: Cell, + pub(crate) is_waiting_abort: Cell, + pub(crate) is_waiting_request_stream_start: Cell, pub(crate) tracker: AsyncTaskTracker, - - pub(crate) ref_count: bun_ptr::ThreadSafeRefCount, + /// The JS side's reference: released by the terminal progress update (or + /// by its VM releasing that update unrun). + progress_ref: JsCell>>, + /// The `fetch()` promise's settlement, run from its own event-loop task + /// (`PromiseSettleHop`, which holds `settle_ref`). + pending_settle: JsCell>, + settle_ref: JsCell>>, + /// The reference held while the request is out on the HTTP thread: + /// released once it hands the request back (`SharedState::handed_back`, + /// or `HandBackHop` when the request body is streamed). + http_ref: JsCell>>, + /// Held while a request body stream is wired to `sink` (`start_request_stream`); + /// released by `write_end_request`, or by the sink's `finalize` if that never ran. + request_stream_ref: JsCell>>, } // Boxing `AnyBlob` is not viable: the `AnyBlob` arm is constructed/matched in @@ -277,59 +536,20 @@ impl HTTPRequestBody { } } -impl Drop for FetchTasklet { - fn drop(&mut self) { - bun_output::scoped_log!(FetchTasklet, "deinit"); - self.ref_count.assert_no_refs(); - // JS thread: no longer something the VM must abort at teardown. - crate::jsc_hooks::ActiveHandle::Fetch(NonNull::from(&mut *self)).unregister(); - self.clear_data(); - } +/// What `on_progress_update` does once the shared state is unlocked. +enum AfterProgress { + /// Nothing (a later update finishes the fetch). + Pending, + /// The fetch is over: release the JS side's reference. + Release, + /// The fetch is over: cancel a still-running request body stream, release + /// the event loop, and release the JS side's reference. + Finish, } impl FetchTasklet { - const HOLDS_TICKET: &str = "fetch on the HTTP thread holds a ticket"; - - // ───── raw-ptr field accessors (centralised unsafe) ─────────────────── - // - // `signal` / `sink` / `native_response` are intrusive-refcounted heap - // objects that this tasklet holds one strong ref on while the field is - // `Some`. They are never reborrowed through any other path on the JS - // thread, so a single `&` / `&mut` derived here is the sole live borrow. - - /// Recover `&mut Self` from a type-erased `*mut c_void` callback context. - /// - /// INVARIANT: every callback that stores a `FetchTasklet*` as `ctx` (the - /// readable-stream available/start-streaming hooks and the ByteStream - /// cancel handler) holds one strong ref on the tasklet for the lifetime - /// of the registration, and fires only on the JS thread — so the returned - /// `&mut` is the sole live borrow. - #[inline] - fn from_ctx<'a>(ctx: NonNull) -> &'a mut Self { - // SAFETY: see INVARIANT above. - unsafe { bun_ptr::callback_ctx::(ctx.as_ptr()) } - } - - /// Recover `&mut Self` from a `*mut FetchTasklet` callback arg. - /// - /// INVARIANT: every `*mut FetchTasklet` threaded through the HTTP-thread - /// callback (`callback`), the drain hook (`on_write_request_data_drain` / - /// `resume_request_data_stream`), and the JS-thread enqueue - /// (`queue` → `node`) was produced by `heap::into_raw(Box)` - /// in `get()` and is kept alive by the intrusive `ref_count` until - /// `deinit`. Access on either thread is serialised: HTTP-thread writes - /// happen under `mutex.lock()` and JS-thread access is single-threaded. - #[inline] - fn from_raw_mut<'a>(this: *mut FetchTasklet) -> &'a mut Self { - // SAFETY: see INVARIANT above. - unsafe { &mut *this } - } - /// Shared variant of [`from_raw_mut`] for paths that only read atomics - /// (`ref_count`, `is_shutting_down`) before deciding whether to upgrade. - #[inline] - fn from_raw_ref<'a>(this: *mut FetchTasklet) -> &'a Self { - // SAFETY: see [`from_raw_mut`] INVARIANT. - unsafe { &*this } + fn this_ptr(&self) -> ThisPtr { + self.self_ref.this_ptr(self) } /// Wrap a borrowed body chunk in a `StreamResult::Temporary*` for @@ -351,145 +571,85 @@ impl FetchTasklet { } } - /// `Some(&AbortSignal)` while we hold a strong ref on the C++-owned - /// `WebCore::AbortSignal*` (taken in `queue`, released in - /// `clear_abort_signal`). - #[inline] - fn abort_signal(&self) -> Option<&AbortSignal> { - self.signal.as_deref() - } - /// True iff an attached AbortSignal has fired. #[inline] pub(crate) fn signal_aborted(&self) -> bool { - self.abort_signal().is_some_and(|s| s.aborted()) + self.signal + .get() + .as_ref() + .is_some_and(|s| s.signal().aborted()) } - /// Mutable access to the request-body sink while `self.sink` is `Some` - /// (owned allocation from `start_request_stream` until `clear_sink`). + /// The request-body sink while attached (`start_request_stream` until `clear_sink`). #[inline] - pub(crate) fn sink_mut(&mut self) -> Option<&mut FetchRequestBodySink> { - // SAFETY: see block comment above. JS-thread-only. - self.sink.map(|p| unsafe { &mut *p.as_ptr() }) + pub(crate) fn sink(&self) -> Option<&FetchRequestBodySink> { + self.sink.get().as_deref() } - /// Mutable access to the request-body streaming buffer while `Some` (this - /// side holds one of the two initial intrusive refs from - /// `ThreadSafeStreamBuffer::new`; released in `clear_sink`). Detached - /// lifetime so the borrow does not conflict with disjoint `&mut self` - /// access at call sites — the buffer lives in a separate heap allocation - /// shared with the HTTP thread (mutex-guarded internally). + /// The request-body buffer shared with the HTTP thread, while attached. #[inline] - pub(crate) fn stream_buffer_mut<'r>(&self) -> Option<&'r mut ThreadSafeStreamBuffer> { - // SAFETY: see doc comment: the counted ref keeps the pointee live, and the - // mutex inside `ThreadSafeStreamBuffer` serialises every cross-thread - // access (`buffer` and the drain callback alike). - self.request_body_streaming_buffer - .as_ref() - .map(|p| unsafe { &mut *p.as_ptr() }) - } - - fn ref_(&self) { - // SAFETY: `self` is live; `ref_` only touches the interior-mutable - // atomic counter. - unsafe { bun_ptr::ThreadSafeRefCount::::ref_(core::ptr::from_ref(self).cast_mut()) }; - } - - /// # Safety - /// Caller holds a ref; `this` must be a live heap allocation from `get()`. - // Forwards `this` to ThreadSafeRefCount without dereferencing; signature must stay - // `*mut` because the call may drop the last ref and free the allocation, so a `&mut` - // here would be UB. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub(crate) fn deref(this: *mut FetchTasklet) { - // SAFETY: caller contract. - unsafe { bun_ptr::ThreadSafeRefCount::::deref(this) }; - } - - /// # Safety - /// Caller holds a ref; `this` must be a live heap allocation from `get()`. - // Forwards `this` to ThreadSafeRefCount/dealloc without dereferencing; signature must - // stay `*mut` because the call may drop the last ref and free the allocation. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn deref_from_thread(this: *mut FetchTasklet, ticket: &jsc::Ticket) { - // SAFETY: caller contract. - if !unsafe { bun_ptr::ThreadSafeRefCount::::release(this) } { - return; - } - // Last ref dropped on the HTTP thread: deinit must run on the JS thread - // (it drops JSC Strong/Weak handles), so hop there — as a task with its - // own tag, so a VM that is tearing down releases it from its queue. - ticket.post(ConcurrentTask::create(bun_event_loop::Task::init( - this.cast::(), - ))); - } - - /// HTTP thread, final callback: the fetch is back. Move the ticket out - /// (nothing here touches the tasklet after the ref drop) and drop this - /// thread's ref through it. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn hand_back(this: *mut FetchTasklet) { - // SAFETY: caller contract; the field is HTTP-thread-only. - let ticket = unsafe { (*this).http_ticket.take() }.expect(Self::HOLDS_TICKET); - Self::deref_from_thread(this, &ticket); - } - - fn clear_sink(&mut self) { - if let Some(sink_ptr) = self.sink.take() { - // SAFETY: FetchTasklet owns the heap allocation from - // `start_request_stream`; the JS controller's back-pointer is - // cleared via `detach` below before drop; sole owner. - let mut sink = unsafe { bun_core::heap::take(sink_ptr.as_ptr()) }; - // Prevent the sink's `finalize()` / `Drop` from double-releasing the - // FetchTasklet ref — `write_end_request` is the canonical release. - sink.task = None; + pub(crate) fn request_body_buffer(&self) -> Option<&ThreadSafeStreamBuffer> { + self.request_body_streaming_buffer.get().as_deref() + } + + fn async_http_id(&self) -> Option { + self.request.get().as_ref().map(InFlight::async_http_id) + } + + fn request_storage(&self) -> Option<&FetchRequestStorage> { + self.request.get().as_ref().map(InFlight::storage) + } + + fn clear_sink(&self) { + if let Some(sink) = self.sink.replace(None) { + // `write_end_request` is the canonical release of `request_stream_ref`; + // keep the sink's `finalize()` from repeating it. + sink.task.set(None); // `detach` may fire the controller's onClose; every terminal path // here has already cleared it, so this just nulls m_sinkPtr. - JSSink::::detach(&mut sink.source, &self.global_this); + let mut source = sink.source.replace(SourceHandle::None); + JSSink::::detach(&mut source, &self.global_this); + drop(sink); } - if let Some(buffer) = self.request_body_streaming_buffer.take() { + if let Some(buffer) = self.request_body_streaming_buffer.replace(None) { // The HTTP thread may still be using its ref; `clear_drain_callback` // synchronises with it through the buffer's mutex. - // SAFETY: kept live by `buffer`. - unsafe { (*buffer.as_ptr()).clear_drain_callback() }; + buffer.clear_drain_callback(); } } - fn clear_data(&mut self) { + fn clear_data(&self) { bun_output::scoped_log!(FetchTasklet, "clearData "); - // `http.client` borrows `url_proxy_buffer` / `unix_socket_path` / `request_headers`. - self.http = None; - if !self.url_proxy_buffer.is_empty() { - self.url_proxy_buffer = Box::default(); - } - - self.unix_socket_path = Box::default(); - - if let Some(certificate) = self.result.certificate_info.take() { - drop(certificate); - } - - // Drop on assignment runs the cleanup. MultiArrayList has no `clear()`. - self.request_headers = Headers::default(); - - if let Some(metadata) = self.metadata.take() { - drop(metadata); + { + let mut state = self.shared.state.lock(); + if let Some(certificate) = state.result.certificate_info.take() { + drop(certificate); + } + if let Some(metadata) = state.metadata.take() { + drop(metadata); + } + state.scheduled_response_buffer = MutableString::default(); } - self.response.clear(); + self.detach_response_body_producer(); + self.response.get().clear(); self.native_response.set(None); self.clear_stream_handlers(); - self.scheduled_response_buffer = MutableString::default(); // Always detach request_body regardless of type. // When request_body is a ReadableStream, startRequestStream() hands // the stream off to `assign_to_stream`, so FetchTasklet's reference // becomes redundant and must be released to avoid leaks. - self.request_body.detach(); + self.request_body.with_mut(HTTPRequestBody::detach); + // The HTTP thread handed the request back before `http_ref` was released. + if let Some(Ok(request)) = self.request.replace(None).map(InFlight::reclaim) { + let mut storage = request.into_storage(); + storage.body.detach(); + } - self.abort_reason.deinit(); - self.check_server_identity.deinit(); + self.abort_reason.with_mut(StrongOptional::deinit); + self.check_server_identity.with_mut(StrongOptional::deinit); self.clear_abort_signal(); // Clear the sink only after the requested ended otherwise we would potentialy lose the last chunk self.clear_sink(); @@ -497,72 +657,21 @@ impl FetchTasklet { /// VM teardown's stop phase (JS thread): abort the transport. The HTTP /// thread then fails the request promptly — started or still queued — and - /// hands the tasklet back through its final callback, which teardown - /// waits for before the handle closes. - /// - /// # Safety - /// `this` is live (registered ⇒ not yet deinit'd); JS thread. - pub(crate) unsafe fn stop_for_vm_teardown(this: *mut FetchTasklet) { - // SAFETY: fn contract. - unsafe { (*this).abort_task() }; - } - - /// `HTTPClientResultCallback::release_at_shutdown` for `FetchTasklet`. - /// Called from `dealloc_in_flight_for_exit` on the HTTP thread for each - /// request still in `in_flight` when `process.exit()` interrupts it. - /// `queue()` left two refs (initial +1 and `node_ref.ref_()`); the final - /// `callback`'s deref and `on_progress_update`'s JS-side deref will never - /// run, so this must balance both — but only when no `on_progress_update` - /// is already parked in the parent's concurrent queue. - /// - /// The `has_schedule_callback` flag distinguishes the two states: - /// * `false` — nothing queued. Drop both refs here; the last one hops - /// `deinit` to the JS thread, which teardown runs from its queue release. - /// * `true` — a non-final `on_progress_update` is queued (this entry is - /// still in `in_flight`, so the *final* `callback` hasn't run). That - /// queued node owns the JS-side ref and its VM releases it from its - /// queue; dropping it here too would leave the queued node pointing at - /// a freed `FetchTasklet`. Drop only the HTTP-side ref. - /// - /// Only reachable for a request whose VM has *not* torn down (a worker - /// still running when the main thread exits): a VM's teardown waits for its - /// fetches' tickets — i.e. for their final callback — before the exiting - /// main thread parks the HTTP thread. `has_schedule_callback` is written by - /// the HTTP-thread `callback` and the JS-thread `on_progress_update` under - /// its own compare-exchange discipline, which this load relies on. - /// - /// SAFETY: `this` is the live `*mut FetchTasklet` registered as - /// `result_callback.ctx` in `get()`; HTTP-thread-only at this point. - unsafe fn release_at_shutdown(this: *mut ()) { - let this = this.cast::(); - // Free the body-bytes buffer the same way the `is_shutting_down` - // branch in `callback` does (no JS-thread drain will reclaim it). - // SAFETY: caller contract — `this` is live and HTTP-thread-exclusive. - let queued_progress_update = - unsafe { (*this).has_schedule_callback.load(Ordering::Acquire) }; - // SAFETY: caller contract — `this` is live and HTTP-thread-exclusive. - let ticket = unsafe { - (*this).scheduled_response_buffer = MutableString::default(); - (*this).http_ticket.take() - } - .expect(Self::HOLDS_TICKET); - FetchTasklet::deref_from_thread(this, &ticket); - if !queued_progress_update { - FetchTasklet::deref_from_thread(this, &ticket); - } - // The HTTP thread is done with this fetch. - drop(ticket); + /// hands the tasklet back, which teardown waits for before the handle closes. + pub(crate) fn stop_for_vm_teardown(this: ThisPtr) { + this.abort_task(); } - fn get_current_response(&self) -> Option<*mut Response> { + /// The response's native `Response`, while we still hold it or its JS wrapper is alive. + fn current_response(&self) -> Option<&Response> { // we need a body to resolve the promise when buffering - if let Some(response) = self.native_response.get().as_ref() { - return Some(response.as_ptr()); + if let Some(response) = self.native_response.get().as_deref() { + return Some(response); } // if we did not have a direct reference we check if the Weak ref is still alive - if let Some(response_js) = self.response.get() { - if let Some(response) = response_js.as_::() { + if let Some(response_js) = self.response.get().get() { + if let Some(response) = response_js.as_class_ref::() { return Some(response); } } @@ -570,28 +679,13 @@ impl FetchTasklet { None } - /// `&mut`-yielding form of [`get_current_response`]. - /// - /// INVARIANT: when `Some`, the pointer is either `native_response` (one - /// strong native ref held by the tasklet until `unref` in cleanup) or the - /// `JSValue::as_::()` deref of a live JS handle pinned by - /// `self.response`. The `Response` is a separate JSC-cell allocation - /// disjoint from `FetchTasklet`, so the returned `&mut` does not overlap - /// any `&mut self` the caller may take afterwards (hence the unbounded - /// `'a`). JS-thread-only; no concurrent `&mut` exists. - #[inline] - fn current_response_mut<'a>(&self) -> Option<&'a mut Response> { - // SAFETY: see INVARIANT above. - self.get_current_response().map(|r| unsafe { &mut *r }) - } - - fn start_request_stream(&mut self) -> JsResult<()> { - self.is_waiting_request_stream_start = false; + fn start_request_stream(&self) -> JsResult<()> { + self.is_waiting_request_stream_start.set(false); debug_assert!(matches!( - self.request_body, + self.request_body.get(), HTTPRequestBody::ReadableStream(_) )); - let HTTPRequestBody::ReadableStream(ref stream_ref) = self.request_body else { + let HTTPRequestBody::ReadableStream(stream_ref) = self.request_body.get() else { return Ok(()); }; let Some(stream) = stream_ref.get() else { @@ -602,11 +696,12 @@ impl FetchTasklet { } let global_this = self.global_this; - // +1 on the tasklet; balanced exactly once by `write_end_request` on the + // Balanced exactly once by `write_end_request` on the // assign_to_stream-result side (on_resolve/on_reject or the synchronous // Fulfilled/Rejected/undefined branches below), or by the sink's // `finalize` as a fallback if that path never runs. - self.ref_(); + self.request_stream_ref + .set(Some(RefPtr::from_this(self.this_ptr()))); if stream.is_locked(&global_this) || stream.is_disturbed(&global_this) { let err = jsc::SystemError { @@ -618,45 +713,43 @@ impl FetchTasklet { }; let err_instance = err.to_error_instance(&global_this); err_instance.ensure_still_alive(); - self.write_end_request(Some(err_instance)); + self.write_end_request_impl(Some(err_instance)); return Ok(()); } - let self_ptr = std::ptr::from_mut::(self); - // `self_ptr` is the live heap tasklet; the +1 above keeps it alive - // until `write_end_request`/`finalize` clears `task`. - let sink: &mut FetchRequestBodySink = Box::leak(Box::new(FetchRequestBodySink { - task: Some(bun_ptr::BackRef::new_mut(self)), - high_water_mark: 16384, + let sink_owner = OwnedThis::new(FetchRequestBodySink { + task: Cell::new(Some(self.self_ref.backref(self))), + high_water_mark: Cell::new(16384), ..Default::default() - })); - let sink_handle = SinkHandle::FetchRequestBody(bun_ptr::BackRef::new_mut(sink)); - self.sink = Some(core::ptr::NonNull::from(&mut *sink)); + }); + let sink_this = sink_owner.this_ptr(); + let sink_handle = SinkHandle::FetchRequestBody(sink_this.backref_mut()); + self.sink.set(Some(sink_owner)); + let sink: &FetchRequestBodySink = sink_this.get(); // Native ByteStream/FileReader fast-path: wire the SinkHandle // directly, skipping the JS pump. match stream.wire_native_sink(&global_this, sink_handle, JSValue::UNDEFINED, |src| { - sink.source = src; + sink.source.set(src); }) { crate::webcore::readable_stream::NativeWireResult::Wired => return Ok(()), crate::webcore::readable_stream::NativeWireResult::EndedInline(err) => { // The source finished inside the wire attempt, so leave the // sink in the state `end_from_stream` leaves it: ended, with // the source and task detached. `write_end_request` below is - // the single balancing release of the `+1` taken above; a + // the single balancing release of `request_stream_ref`; a // sink left `ended == false` here would make the terminal // `cancel_request_body_sink` treat it as a live native sink - // and release that ref a second time, freeing the tasklet - // while it is still in use. - sink.ended = true; - sink.source.clear(); - sink.task = None; + // and release it a second time. + sink.ended.set(true); + sink.source.set(SourceHandle::None); + sink.task.set(None); let err_js = err.map(|err| { let err_js = err.to_js(&global_this); err_js.ensure_still_alive(); err_js }); - self.write_end_request(err_js); + self.write_end_request_impl(err_js); return Ok(()); } crate::webcore::readable_stream::NativeWireResult::NotNative => {} @@ -665,12 +758,12 @@ impl FetchTasklet { let assignment_result = JSSink::::assign_to_stream( &global_this, stream.value, - core::ptr::NonNull::from(&mut *sink), + NonNull::from(sink_this), ); assignment_result.ensure_still_alive(); if let Some(err) = assignment_result.to_error() { - self.write_end_request(Some(err)); + self.write_end_request_impl(Some(err)); self.clear_sink(); return Ok(()); } @@ -681,20 +774,20 @@ impl FetchTasklet { bun_jsc::js_promise::Status::Pending => { assignment_result.then( &global_this, - self_ptr, - on_resolve_request_stream_shim, - on_reject_request_stream_shim, + self.this_ptr().as_ptr(), + crate::generated_host_exports::Bun__FetchTasklet__onResolveRequestStream, + crate::generated_host_exports::Bun__FetchTasklet__onRejectRequestStream, ); } bun_jsc::js_promise::Status::Fulfilled => { - sink.task = None; - self.write_end_request(None); + sink.task.set(None); + self.write_end_request_impl(None); } bun_jsc::js_promise::Status::Rejected => { promise.set_handled(global_this.vm()); let result = promise.result(global_this.vm()); - sink.task = None; - self.write_end_request(Some(result)); + sink.task.set(None); + self.write_end_request_impl(Some(result)); } } return Ok(()); @@ -703,47 +796,50 @@ impl FetchTasklet { // undefined/null: the stream drained synchronously inside // assignToStream. `end()` no longer calls `write_end_request`, so this - // path always balances the `+1` itself. - sink.task = None; - self.write_end_request(None); + // path always releases `request_stream_ref` itself. + sink.task.set(None); + self.write_end_request_impl(None); Ok(()) } - fn on_body_received(&mut self) -> JsResult<()> { - let success = self.result.is_success(); + fn on_body_received(&self, state: &mut SharedState) -> JsResult<()> { + let success = state.result.is_success(); let global_this = self.global_this; - // reset the buffer if we are streaming or if we are not waiting for bufferig anymore - let buffer_reset = core::cell::Cell::new(true); bun_output::scoped_log!( FetchTasklet, "onBodyReceived success={} has_more={}", success, - self.result.has_more + state.result.has_more ); - // The reset must run on `?` failure paths too. - // Capture a raw ptr so the defer can reset on every exit (incl. `?`) without holding a - // long-lived &mut borrow of self. - let scheduled_buf: *mut MutableString = &raw mut self.scheduled_response_buffer; - scopeguard::defer! { - if buffer_reset.get() { - // SAFETY: `self` outlives this defer (it's a local in this fn) and no other - // borrow of scheduled_response_buffer is live at scope exit / `?` unwind. - let list = unsafe { &mut (*scheduled_buf).list }; - if list.capacity() > http::DECODED_BODY_RETAIN_CAP { - *list = Vec::new(); - } else { - list.clear(); - } + // reset the buffer if we are streaming or if we are not waiting for bufferig anymore + // (on every exit, `?` failures included) + let mut buffer_reset = true; + let r = self.on_body_received_inner(state, success, global_this, &mut buffer_reset); + if buffer_reset { + let list = &mut state.scheduled_response_buffer.list; + if list.capacity() > http::DECODED_BODY_RETAIN_CAP { + *list = Vec::new(); + } else { + list.clear(); } } + r + } + fn on_body_received_inner( + &self, + state: &mut SharedState, + success: bool, + global_this: GlobalRef, + buffer_reset: &mut bool, + ) -> JsResult<()> { if !success { // `ValueError` // has no `Drop` (it's reset-in-place, see Body.rs), so the Strong installed by // `to_js` would leak on the sink-cancel / no-response / `?` exits. Hold it in a // scopeguard and defuse via `into_inner` when ownership is transferred to // `to_error_instance`. - let mut err = scopeguard::guard(self.on_reject(), |mut e| e.reset()); + let mut err = scopeguard::guard(self.on_reject(state), |mut e| e.reset()); let mut js_err = JSValue::ZERO; // if we are streaming update with error if let Some(bytes) = self.response_stream.take() { @@ -760,12 +856,12 @@ impl FetchTasklet { // leave a buffered `arrayBuffer()`/`text()` promise pending // forever when a fetch with an in-flight streaming request body // was aborted mid-response. - if self.sink_mut().is_some() && js_err.is_empty() { + if self.sink().is_some() && js_err.is_empty() { js_err = err.to_js(&global_this); js_err.ensure_still_alive(); } // if we are buffering resolve the promise - if let Some(response) = self.current_response_mut() { + if let Some(response) = self.current_response() { // body value now owns the error let err = scopeguard::ScopeGuard::into_inner(err); let body = response.get_body_value(); @@ -781,21 +877,21 @@ impl FetchTasklet { } // body can be marked as used but we still need to pipe the data - if !self.result.has_more { + if !state.result.has_more { // Unhook before the final delivery so it cannot signal a producer that is done; // release after it so the bytes land in memory we still pin. if let Some(bytes) = self.response_stream.take() { bun_output::scoped_log!(FetchTasklet, "onBodyReceived response_stream done"); - bytes.size_hint.set(self.get_size_hint()); - buffer_reset.set(false); - let chunk = self.scheduled_response_buffer.list.as_slice(); + bytes.size_hint.set(Self::get_size_hint(state)); + *buffer_reset = false; + let chunk = state.scheduled_response_buffer.list.as_slice(); bytes.on_data(Self::temporary_chunk(chunk, true)); return Ok(()); } } else if let Some(bytes) = self.response_stream.bytes() { bun_output::scoped_log!(FetchTasklet, "onBodyReceived response_stream"); - bytes.size_hint.set(self.get_size_hint()); - let chunk = self.scheduled_response_buffer.list.as_slice(); + bytes.size_hint.set(Self::get_size_hint(state)); + let chunk = state.scheduled_response_buffer.list.as_slice(); bytes.on_data(Self::temporary_chunk(chunk, false)); if self.response_stream.is_held() { self.after_body_chunk_delivered(&bytes); @@ -803,9 +899,9 @@ impl FetchTasklet { return Ok(()); } - if let Some(response) = self.current_response_mut() { + if let Some(response) = self.current_response() { bun_output::scoped_log!(FetchTasklet, "onBodyReceived Current Response"); - let size_hint = self.get_size_hint(); + let size_hint = Self::get_size_hint(state); response.set_size_hint(size_hint); if let Some(readable) = response.get_body_readable_stream() { bun_output::scoped_log!( @@ -813,9 +909,9 @@ impl FetchTasklet { "onBodyReceived CurrentResponse BodyReadableStream" ); if let Some(bytes) = readable.ptr.bytes() { - let chunk = self.scheduled_response_buffer.list.as_slice(); + let chunk = state.scheduled_response_buffer.list.as_slice(); - if self.result.has_more { + if state.result.has_more { bytes.on_data(Self::temporary_chunk(chunk, false)); } else { readable.value.ensure_still_alive(); @@ -827,24 +923,21 @@ impl FetchTasklet { } } - // raw ptr: `body` and `get_fetch_headers()` are disjoint fields but borrowck can't see through the accessors. - let body: *mut BodyValue = response.get_body_value(); // `BodyAbortListener::on_abort` may have set `Error` while this - // callback was queued; checked before `buffer_reset.set(false)` so - // the defer still drops the bytes. - // SAFETY: just obtained from live `response`. - if !matches!(unsafe { &*body }, BodyValue::Locked(_)) { + // callback was queued; checked before `buffer_reset = false` so + // the bytes are still dropped. + if !matches!(response.get_body_value(), BodyValue::Locked(_)) { return Ok(()); } // we will reach here when not streaming, this is also the only case we dont wanna to reset the buffer - buffer_reset.set(false); - if !self.result.has_more { + *buffer_reset = false; + if !state.result.has_more { let scheduled_response_buffer = - core::mem::take(&mut self.scheduled_response_buffer.list); + core::mem::take(&mut state.scheduled_response_buffer.list); + let body = response.get_body_value(); // done resolve body let old = core::mem::replace( - // SAFETY: just obtained from live `response`; uniquely accessed here. - unsafe { &mut *body }, + body, BodyValue::InternalBlob(InternalBlob { bytes: scheduled_response_buffer, was_string: false, @@ -853,26 +946,18 @@ impl FetchTasklet { bun_output::scoped_log!( FetchTasklet, "onBodyReceived body_value length={}", - // SAFETY: see above. - match unsafe { &*body } { + match &*body { BodyValue::InternalBlob(b) => b.bytes.len(), _ => 0, } ); - self.scheduled_response_buffer = MutableString::default(); + state.scheduled_response_buffer = MutableString::default(); if matches!(old, BodyValue::Locked(_)) { bun_output::scoped_log!(FetchTasklet, "onBodyReceived old.resolve"); let mut old = old; - // BodyValue::resolve takes `Option>` (opaque C++ handle - // mutated via FFI); the inherent `get_fetch_headers` returns `Option<&_>`, so - // erase the borrow into a raw NonNull. Disjoint from `body` (response.init vs - // response.body) and outlives this block. - let headers = response.get_fetch_headers().map(core::ptr::NonNull::from); - // SAFETY: `body` points into `response.body`, disjoint from `headers` - // (response.init); both live for this block. - let body = unsafe { &mut *body }; + let headers = response.get_fetch_headers().map(NonNull::from); BodyValue::resolve(&mut old, body, &self.global_this, headers)?; } } @@ -880,60 +965,87 @@ impl FetchTasklet { Ok(()) } - pub(crate) fn on_progress_update(&mut self) -> JsResult<()> { + /// A progress update the HTTP thread posted (`FetchShared::on_result`). + pub(crate) fn on_progress_update(this: ThisPtr) -> JsResult<()> { jsc::mark_binding!(); bun_output::scoped_log!(FetchTasklet, "onProgressUpdate"); - self.mutex.lock(); - self.has_schedule_callback.store(false, Ordering::Relaxed); - let is_done = !self.result.has_more; + // The references released below are never the last while this runs. + let _guard = RefPtr::from_this(this); + let mut state = this.shared.state.lock(); + this.shared + .has_schedule_callback + .store(false, Ordering::Relaxed); + let handed_back = state.handed_back; + let (after, result) = if state.released_at_shutdown { + // `process.exit()` took the request off the HTTP thread: nothing + // more arrives and there is nothing to deliver; just let go. + (AfterProgress::Release, Ok(())) + } else { + let is_done = !state.result.has_more; + this.progress_update_locked(&mut state, is_done) + }; + drop(state); + match after { + AfterProgress::Pending => {} + AfterProgress::Release => Self::release(this, |t| &t.progress_ref), + AfterProgress::Finish => { + // The HTTP response has been fully received. If the request body + // is still being uploaded, the HTTP layer will never drain/resume + // it again — cancel the sink so the JS side releases the reader; + // the pump-promise settlement drops `request_stream_ref`. + this.cancel_request_body_sink(JSValue::UNDEFINED); + this.poll_ref + .with_mut(|poll_ref| poll_ref.unref(bun_io::js_vm_ctx())); + Self::release(this, |t| &t.progress_ref); + } + } + if handed_back { + Self::release(this, |t| &t.http_ref); + } + result + } + /// Release the reference in `slot`, if held. May free the tasklet. + fn release( + this: ThisPtr, + slot: fn(&FetchTasklet) -> &JsCell>>, + ) { + drop(slot(this.get()).replace(None)); + } + + fn progress_update_locked( + &self, + state: &mut SharedState, + is_done: bool, + ) -> (AfterProgress, JsResult<()>) { + let done_or = |after: AfterProgress| { + if is_done { + after + } else { + AfterProgress::Pending + } + }; let vm = self.global_this.bun_vm(); // teardown forbade script: we cannot touch JS if !vm.script_allowed() { // The certificate will never be checked; release the parked // HTTP-thread socket instead of leaving it occupying an active // request slot until the idle timeout. - if self.result.certificate_info.take().is_some() { - if let Some(http_) = self.http.as_mut() { - http::http_thread().schedule_shutdown(http_); + if state.result.certificate_info.take().is_some() { + if let Some(id) = self.async_http_id() { + http::http_thread().schedule_shutdown_by_id(id); } } - self.mutex.unlock(); - if is_done { - // SAFETY: `self` is the live heap tasklet; we hold a ref. - FetchTasklet::deref(std::ptr::from_mut(self)); - } - return Ok(()); + return (done_or(AfterProgress::Release), Ok(())); } let global_this = self.global_this; - // explicit cleanup at each return (a closure keeps borrowck happy) - let cleanup = |this: &mut FetchTasklet| { - this.mutex.unlock(); - // if we are not done we wait until the next call - if is_done { - // The HTTP response has been fully received. If the request body - // is still being uploaded, the HTTP layer will never drain/resume - // it again — cancel the sink so the JS side releases the reader; - // the pump-promise settlement drops the `startRequestStream` ref. - this.cancel_request_body_sink(JSValue::UNDEFINED); - this.poll_ref - .with_mut(|poll_ref| poll_ref.unref(bun_io::js_vm_ctx())); - // SAFETY: `this` is the live heap tasklet; we hold a ref. - FetchTasklet::deref(std::ptr::from_mut(this)); - } - }; - if self.is_waiting_request_stream_start && self.result.can_stream { + if self.is_waiting_request_stream_start.get() && state.result.can_stream { // start streaming if let Err(err) = self.start_request_stream() { // The VM is being stopped: leave like the `!script_allowed()` gate above does. - self.mutex.unlock(); - if is_done { - // SAFETY: `self` is the live heap tasklet; we hold a ref. - FetchTasklet::deref(std::ptr::from_mut(self)); - } - return Err(err); + return (done_or(AfterProgress::Release), Err(err)); } // Makes wpt-h2 number-chunk test deterministic. // `assign_to_stream` kicks off `await reader.read()`; an invalid @@ -958,22 +1070,22 @@ impl FetchTasklet { // The JSC-only drain is `&self`, runs just promise reactions (sufficient // for the queued `endSink(err)` to land in `write_end_request` → // `abort_reason`), and leaves the Bun event loop untouched. - if self.metadata.is_some() && !self.is_waiting_body { + if state.metadata.is_some() && !self.is_waiting_body.get() { vm.jsc_vm().drain_microtasks(); } } // if we already respond the metadata and still need to process the body - if self.is_waiting_body { + if self.is_waiting_body.get() { // `scheduled_response_buffer` has two readers that both drain-and-reset: - // this path (onBodyReceived) and `onStartStreamingHTTPResponseBodyCallback`, + // this path (onBodyReceived) and `on_start_streaming_http_response_body`, // which runs once when JS first touches `res.body` and hands any already- // buffered bytes to the new ByteStream synchronously. // // That creates a stale-task race: - // 1. HTTP thread `callback()` writes N bytes to the buffer and enqueues - // this onProgressUpdate task (under mutex). + // 1. HTTP thread `on_result()` writes N bytes to the buffer and enqueues + // this onProgressUpdate task (under the `state` lock). // 2. Main thread: JS touches `res.body` -> `onStartStreaming` drains those - // N bytes and resets the buffer (under mutex). + // N bytes and resets the buffer (under the `state` lock). // 3. This task runs and finds the buffer empty. // // The task cannot be un-enqueued in step 2, and at schedule time (step 1) @@ -987,16 +1099,14 @@ impl FetchTasklet { // early-returned on `kPendingRead`) is never cleared, `_read()` is never // called again, and `pipeline(Readable.fromWeb(res.body), ...)` stalls // forever — eventually spinning at 100% CPU once `poll_ref` unrefs. - if self.scheduled_response_buffer.list.is_empty() - && self.result.has_more - && self.result.is_success() + if state.scheduled_response_buffer.list.is_empty() + && state.result.has_more + && state.result.is_success() { - cleanup(self); - return Ok(()); + return (done_or(AfterProgress::Finish), Ok(())); } - let r = self.on_body_received(); - cleanup(self); - return r; + let r = self.on_body_received(state); + return (done_or(AfterProgress::Finish), r); } // Run the user-supplied `checkServerIdentity` callback as soon as the // certificate arrives. The HTTP thread parks the connection after the @@ -1006,45 +1116,43 @@ impl FetchTasklet { // first progress update carries only the certificate (no metadata, no // failure) and would otherwise be dropped, leaving the socket parked // until the idle timeout. - if let Some(certificate_info) = self.result.certificate_info.take() { + if let Some(certificate_info) = state.result.certificate_info.take() { // we receive some error - if self.reject_unauthorized && !self.check_server_identity(&certificate_info) { + if self.reject_unauthorized && !self.check_server_identity(state, &certificate_info) { bun_output::scoped_log!(FetchTasklet, "onProgressUpdate: aborted due certError"); drop(certificate_info); // `check_server_identity` already set abort_reason / aborted / // result.fail and scheduled the shutdown of the parked // socket; all that is left is rejecting the promise. - let promise_value = self.promise.value_or_empty(); + let promise_value = self.promise.get().value_or_empty(); if promise_value.is_empty_or_undefined_or_null() { bun_output::scoped_log!( FetchTasklet, "onProgressUpdate: promise_value is null" ); - self.promise = jsc::JSPromiseStrong::empty(); - cleanup(self); - return Ok(()); + self.promise.set(jsc::JSPromiseStrong::empty()); + return (done_or(AfterProgress::Finish), Ok(())); } // we need to abort the request let promise = promise_value.as_any_promise().unwrap(); let tracker = self.tracker; - let mut result = self.on_reject(); + let mut result = self.on_reject(state); promise_value.ensure_still_alive(); let r = promise.reject_with_async_stack(&global_this, result.to_js(&global_this)); result.reset(); tracker.did_dispatch(&global_this); - self.promise = jsc::JSPromiseStrong::empty(); - cleanup(self); - return r; + self.promise.set(jsc::JSPromiseStrong::empty()); + return (done_or(AfterProgress::Finish), r); } drop(certificate_info); // checkServerIdentity passed: un-park the HTTP-thread connection // so the request is finally written to the now-verified peer. If // the connection already closed/failed the resume is a no-op // (keyed through the abort tracker). - if let Some(http_) = self.http.as_mut() { - http::http_thread().schedule_cert_check_resume(http_); + if let Some(id) = self.async_http_id() { + http::http_thread().schedule_cert_check_resume(id); } // Fall through. The common case (certificate-only update) returns // at the metadata-less early return below; the #27275 coalesced @@ -1054,38 +1162,35 @@ impl FetchTasklet { // — falls through to the reject logic with `result.fail` set. } - if self.metadata.is_none() && self.result.is_success() { - cleanup(self); - return Ok(()); + if state.metadata.is_none() && state.result.is_success() { + return (done_or(AfterProgress::Finish), Ok(())); } // if we abort because of cert error // we wait the Http Client because we already have the response // we just need to deinit - if self.is_waiting_abort { - cleanup(self); - return Ok(()); + if self.is_waiting_abort.get() { + return (done_or(AfterProgress::Finish), Ok(())); } - let promise_value = self.promise.value_or_empty(); + let promise_value = self.promise.get().value_or_empty(); if promise_value.is_empty_or_undefined_or_null() { bun_output::scoped_log!(FetchTasklet, "onProgressUpdate: promise_value is null"); - self.promise = jsc::JSPromiseStrong::empty(); - cleanup(self); - return Ok(()); + self.promise.set(jsc::JSPromiseStrong::empty()); + return (done_or(AfterProgress::Finish), Ok(())); } // WHATWG fetch: once the response head is available the promise // resolves; post-head failures (body decompression etc.) surface on // the body reader regardless of whether head+body arrived in one read. - let success = self.result.is_success() || self.metadata.is_some(); + let success = state.result.is_success() || state.metadata.is_some(); // Paired with the microtask drain after // startRequestStream above: the request-body sink may have set `abort_reason` // via writeEndRequest while the HTTP result is still a success — server HEADERS // raced ahead of the scheduled shutdown. Reject with that reason instead of // resolving a 200 Response. Makes wpt-h2 number-chunk test deterministic. - if success && self.abort_reason.has() { + if success && self.abort_reason.get().has() { let promise = promise_value.as_any_promise().unwrap(); let tracker = self.tracker; // get_abort_error consumes abort_reason and clears the signal handler. @@ -1094,25 +1199,20 @@ impl FetchTasklet { let r = promise.reject_with_async_stack(&global_this, err.to_js(&global_this)); err.reset(); tracker.did_dispatch(&global_this); - self.promise = jsc::JSPromiseStrong::empty(); - cleanup(self); - return r; + self.promise.set(jsc::JSPromiseStrong::empty()); + return (done_or(AfterProgress::Finish), r); } let tracker = self.tracker; tracker.will_dispatch(&global_this); - let dispatch_cleanup = |_this: &mut FetchTasklet| { - bun_output::scoped_log!(FetchTasklet, "onProgressUpdate: promise_value is not null"); - tracker.did_dispatch(&global_this); - }; let result = if success { - let resolved = self.on_resolve(); + let resolved = self.on_resolve(state); // Cancel the request-body sink last (as on_body_received does): // closing the sink signal runs the user's cancel callback // synchronously, so the body error must already be stored. - if self.result.fail.is_some() && self.sink_mut().is_some() { - let mut err = self.on_reject(); + if state.result.fail.is_some() && self.sink().is_some() { + let mut err = self.on_reject(state); let err_js = err.to_js(&global_this); err_js.ensure_still_alive(); self.cancel_request_body_sink(err_js); @@ -1121,7 +1221,7 @@ impl FetchTasklet { StrongOptional::create(resolved, &global_this) } else { // in this case we wanna a jsc.Strong.Optional so we just convert it - let mut value = self.on_reject(); + let mut value = self.on_reject(state); let err_js = value.to_js(&global_this); self.cancel_request_body_sink(err_js); // `to_js` leaves `value` in the `JSValue(Strong)` state (Body.rs:547). Move @@ -1135,52 +1235,45 @@ impl FetchTasklet { promise_value.ensure_still_alive(); - let holder = Box::new(FetchTaskletPromiseSettle { + self.pending_settle.set(Some(FetchTaskletPromiseSettle { held: result, // we need the promise to be alive until the task is done - promise: self.promise.take(), + promise: self.promise.with_mut(jsc::JSPromiseStrong::take), global_object: global_this, success, - }); - // SAFETY: `vm.event_loop()` is the live JS-thread loop. - unsafe { - (*vm.event_loop()).enqueue_task(Task::from_boxed(holder)); - } - - dispatch_cleanup(self); - cleanup(self); - Ok(()) - } - - fn check_server_identity(&mut self, certificate_info: &CertificateInfo) -> bool { - if let Some(check_server_identity) = self.check_server_identity.get() { + })); + self.settle_ref + .set(Some(RefPtr::from_this(self.this_ptr()))); + vm.event_loop_mut() + .enqueue_task(PromiseSettleHop::task(self.this_ptr())); + + bun_output::scoped_log!(FetchTasklet, "onProgressUpdate: promise_value is not null"); + tracker.did_dispatch(&global_this); + (done_or(AfterProgress::Finish), Ok(())) + } + + fn check_server_identity( + &self, + state: &mut SharedState, + certificate_info: &CertificateInfo, + ) -> bool { + if let Some(check_server_identity) = self.check_server_identity.get().get() { check_server_identity.ensure_still_alive(); if !certificate_info.cert.is_empty() { - let cert = &certificate_info.cert; - let mut cert_ptr = cert.as_ptr(); - // SAFETY: cert is a valid DER buffer; d2i_X509 reads up to cert.len() bytes - let x509 = unsafe { - d2i_X509( - core::ptr::null_mut(), - &raw mut cert_ptr, - core::ffi::c_long::try_from(cert.len()).expect("int cast"), - ) - }; - if !x509.is_null() { + if let Some(mut x509) = + bun_boringssl_sys::OwnedX509::from_der(&certificate_info.cert) + { let global_object = self.global_this; - // SAFETY: `x` is the non-null `X509*` returned by `d2i_X509` above; this - // guard is its sole owner and frees it exactly once on scope exit. - let _x509_guard = scopeguard::guard(x509, |x| unsafe { X509_free(x) }); - // SAFETY: x509 is non-null, freshly parsed; freed by guard above. - let js_cert = match X509::to_js(unsafe { &mut *x509 }, &global_object) { + let js_cert = match X509::to_js(x509.as_mut(), &global_object) { Ok(v) => v, Err(e) => { let check_result = global_object.take_exception(e); // mark to wait until deinit - self.is_waiting_abort = self.result.has_more; - self.abort_reason.set(&global_object, check_result); + self.is_waiting_abort.set(state.result.has_more); + self.abort_reason + .with_mut(|r| r.set(&global_object, check_result)); self.abort_task(); - self.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); + state.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); return false; } }; @@ -1191,10 +1284,11 @@ impl FetchTasklet { Ok(v) => v, Err(e) => { let hostname_err_result = global_object.take_exception(e); - self.is_waiting_abort = self.result.has_more; - self.abort_reason.set(&global_object, hostname_err_result); + self.is_waiting_abort.set(state.result.has_more); + self.abort_reason + .with_mut(|r| r.set(&global_object, hostname_err_result)); self.abort_task(); - self.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); + state.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); return false; } }; @@ -1212,10 +1306,11 @@ impl FetchTasklet { // > Returns object [...] on failure if check_result.is_any_error() { // mark to wait until deinit - self.is_waiting_abort = self.result.has_more; - self.abort_reason.set(&global_object, check_result); + self.is_waiting_abort.set(state.result.has_more); + self.abort_reason + .with_mut(|r| r.set(&global_object, check_result)); self.abort_task(); - self.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); + state.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); return false; } @@ -1227,62 +1322,60 @@ impl FetchTasklet { } // Empty or unparseable certificate bytes: every false return must have // scheduled the parked socket's shutdown, like the paths above. - if let Some(http_) = self.http.as_mut() { - http::http_thread().schedule_shutdown(http_); + if let Some(id) = self.async_http_id() { + http::http_thread().schedule_shutdown_by_id(id); } - self.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); + state.result.fail = Some(http::Error::ERR_TLS_CERT_ALTNAME_INVALID); false } - fn get_abort_error(&mut self) -> Option { - if self.abort_reason.has() { - let out = core::mem::replace(&mut self.abort_reason, StrongOptional::empty()); + fn get_abort_error(&self) -> Option { + if self.abort_reason.get().has() { + let out = self.abort_reason.replace(StrongOptional::empty()); self.clear_abort_signal(); return Some(BodyValueError::JSValue(out)); } - if let Some(signal) = self.abort_signal() { - if let Some(reason) = signal.reason_if_aborted(&self.global_this) { - // `AbortReason::to_body_value_error` lives in bun_jsc but - // would forward-depend on bun_runtime; reconstruct the trivial - // mapping at the call site (per AbortSignal.rs note). - let out = match reason { - jsc::abort_signal::AbortReason::Common(r) => BodyValueError::AbortReason(r), - jsc::abort_signal::AbortReason::Js(v) => { - BodyValueError::JSValue(StrongOptional::create(v, &self.global_this)) - } - }; - self.clear_abort_signal(); - return Some(out); - } + let reason = self + .signal + .get() + .as_ref() + .and_then(|s| s.signal().reason_if_aborted(&self.global_this)); + if let Some(reason) = reason { + // `AbortReason::to_body_value_error` lives in bun_jsc but + // would forward-depend on bun_runtime; reconstruct the trivial + // mapping at the call site (per AbortSignal.rs note). + let out = match reason { + jsc::abort_signal::AbortReason::Common(r) => BodyValueError::AbortReason(r), + jsc::abort_signal::AbortReason::Js(v) => { + BodyValueError::JSValue(StrongOptional::create(v, &self.global_this)) + } + }; + self.clear_abort_signal(); + return Some(out); } None } - fn clear_abort_signal(&mut self) { - let Some(signal) = self.signal.take() else { - return; - }; - // Order matters: cleanNativeBindings first, then pending_activity_unref - // and (dropping `signal`) unref. - signal.clean_native_bindings(std::ptr::from_mut(self).cast::()); - signal.pending_activity_unref(); + /// Drop our listener (and with it our reference on the signal and its pending activity). + fn clear_abort_signal(&self) { + self.signal.set(None); } - fn on_reject(&mut self) -> BodyValueError { - debug_assert!(self.result.fail.is_some()); + fn on_reject(&self, state: &SharedState) -> BodyValueError { + debug_assert!(state.result.fail.is_some()); bun_output::scoped_log!(FetchTasklet, "onReject"); if let Some(err) = self.get_abort_error() { return err; } - if let Some(reason) = self.result.abort_reason() { + if let Some(reason) = state.result.abort_reason() { return BodyValueError::AbortReason(reason); } - let fail = self.result.fail.unwrap(); + let fail = state.result.fail.unwrap(); if fail == http::Error::RequestBodyNotReusable { return BodyValueError::TypeError(BunString::static_( @@ -1290,11 +1383,11 @@ impl FetchTasklet { )); } - // some times we don't have metadata so we also check http.url - let path = if let Some(metadata) = &self.metadata { + // some times we don't have metadata so we also check the request's url + let path = if let Some(metadata) = &state.metadata { BunString::clone_utf8(metadata.url.slice()) - } else if let Some(http_) = &self.http { - BunString::clone_utf8(http_.url.href) + } else if let Some(storage) = self.request_storage() { + BunString::clone_utf8(storage.url().href) } else { BunString::EMPTY }; @@ -1305,12 +1398,11 @@ impl FetchTasklet { // raw getaddrinfo(3) code and is nonzero on this path, so `init_eai` // is always `Some`. if fail == http::Error::DNSResolveFailed { - if let Some(dns_err) = c_ares::Error::init_eai(self.result.dns_error) { + if let Some(dns_err) = c_ares::Error::init_eai(state.result.dns_error) { // `dns_hostname` is the owned copy of the exact name the // connect resolved (proxy or post-redirect target), captured - // on the HTTP thread; never reconstruct it from `self.http`, - // whose post-redirect URL slices are freed by then. - let hostname: &[u8] = self.result.dns_hostname.as_deref().unwrap_or(b""); + // on the HTTP thread. + let hostname: &[u8] = state.result.dns_hostname.as_deref().unwrap_or(b""); let mut err = crate::dns_jsc::cares_jsc::system_error_with_syscall_and_hostname( dns_err, b"getaddrinfo", @@ -1560,43 +1652,41 @@ impl FetchTasklet { BodyValueError::SystemTypeError(fetch_error) } - fn on_readable_stream_available( - ctx: NonNull, + /// The response body's `ByteStream` now exists: become its producer. + pub(crate) fn on_readable_stream_available( + &self, _global_this: &JSGlobalObject, - readable: ReadableStream, + readable: &ReadableStream, ) { - let this = Self::from_ctx(ctx); - if let crate::webcore::readable_stream::Source::Bytes(bytes) = readable.ptr { - // SAFETY: the caller holds the stream, which owns the live ByteStream. JS thread. - unsafe { this.response_stream.hold(bytes) }; - } else { - this.response_stream.release(); - } + self.response_stream.hold(readable); } - fn on_start_streaming_http_response_body_callback(ctx: NonNull) -> DrainResult { - let this = Self::from_ctx(ctx); - if this.signal_store.aborted.load(Ordering::Relaxed) { + /// The response body is being realised as a `ByteStream`: hand over what is + /// already buffered and start streaming the rest. + pub(crate) fn on_start_streaming_http_response_body(&self) -> DrainResult { + if self.shared.signal_store.aborted.load(Ordering::Relaxed) { return DrainResult::Aborted; } // A body consumer is attaching; keep the process alive until the // body finishes (undone in `on_progress_update` when `is_done`), or // until the stream parks unread (`park_body_stream`). - this.poll_ref + self.poll_ref .with_mut(|poll_ref| poll_ref.ref_(bun_io::js_vm_ctx())); - this.mutex.lock(); - let size_hint = this.get_size_hint() as usize; - let drained = core::mem::take(&mut this.scheduled_response_buffer.list); - this.mutex.unlock(); + let (size_hint, drained) = { + let mut state = self.shared.state.lock(); + let size_hint = Self::get_size_hint(&state) as usize; + let drained = core::mem::take(&mut state.scheduled_response_buffer.list); + (size_hint, drained) + }; // After the take, not before: a chunk the HTTP thread appends (and pauses for) in // between would otherwise reach the stream with its task finding the buffer empty, and // nothing left to undo that pause. Unconditional: also flushes body bytes the client // holds that arrived with no follow-up read (`drain_response_body`). - this.signal_store.unpause_receive(); - this.schedule_receive_resume(); + self.shared.signal_store.unpause_receive(); + self.schedule_receive_resume(); if drained.is_empty() { DrainResult::EstimatedSize(size_hint) @@ -1608,8 +1698,8 @@ impl FetchTasklet { } } - fn get_size_hint(&self) -> BlobSizeType { - match self.body_size { + fn get_size_hint(state: &SharedState) -> BlobSizeType { + match state.result.body_size { http::BodySize::ContentLength(n) => n as BlobSizeType, http::BodySize::TotalReceived(n) => n as BlobSizeType, http::BodySize::Unknown => 0, @@ -1622,7 +1712,7 @@ impl FetchTasklet { } /// reader.cancel() / body.cancel(): the server has to see the close (Node, Deno and browsers - /// abort too). `&self` because a failed sink write reaches here from inside `on_body_received`. + /// abort too). A failed sink write reaches here from inside `on_body_received`. pub(crate) fn on_stream_cancelled(&self) { self.abort_task(); self.abandon_response_body(); @@ -1641,7 +1731,7 @@ impl FetchTasklet { } fn resume_receive(&self) { - if self.signal_store.unpause_receive() { + if self.shared.signal_store.unpause_receive() { self.schedule_receive_resume(); } } @@ -1651,9 +1741,8 @@ impl FetchTasklet { self.unpark_body_stream(); } - /// The other half of this rule is in `callback` (HTTP thread). - fn after_body_chunk_delivered(&self, bytes: &crate::webcore::ByteStream) { - use crate::webcore::byte_stream::{AfterDelivery, ProducerHold}; + /// The other half of this rule is in `FetchShared::on_result` (HTTP thread). + fn after_body_chunk_delivered(&self, bytes: &ByteStream) { bun_output::scoped_log!( FetchTasklet, "afterBodyChunkDelivered buffered={}", @@ -1661,9 +1750,9 @@ impl FetchTasklet { ); match ProducerHold::after_delivery(bytes) { AfterDelivery::Resume => self.resume_receive(), - AfterDelivery::Pause => self.signal_store.pause_receive(), + AfterDelivery::Pause => self.shared.signal_store.pause_receive(), AfterDelivery::Park => { - self.signal_store.pause_receive(); + self.shared.signal_store.pause_receive(); self.park_body_stream(); } } @@ -1685,48 +1774,43 @@ impl FetchTasklet { } } - fn on_start_buffering_callback(ctx: NonNull) { - let this = Self::from_ctx(ctx); - this.poll_ref + /// A consumer wants the whole response body buffered. + pub(crate) fn on_start_buffering(&self) { + self.poll_ref .with_mut(|poll_ref| poll_ref.ref_(bun_io::js_vm_ctx())); - if this.signal_store.receive_all() { - this.schedule_receive_resume(); + if self.shared.signal_store.receive_all() { + self.schedule_receive_resume(); } } fn schedule_receive_resume(&self) { - if let Some(http_) = self.http.as_ref() { - http::http_thread().schedule_receive_resume(http_.async_http_id); + if let Some(id) = self.async_http_id() { + http::http_thread().schedule_receive_resume(id); } } - fn to_body_value(&mut self) -> BodyValue { + fn to_body_value(&self, state: &mut SharedState) -> BodyValue { if let Some(err) = self.get_abort_error() { return BodyValue::Error(err); } - if self.result.fail.is_some() { + if state.result.fail.is_some() { // Head received but body failed in the same callback; surface on // the body so this matches the split-read `on_body_received` path. - return BodyValue::Error(self.on_reject()); + return BodyValue::Error(self.on_reject(state)); } - if self.is_waiting_body { + if self.is_waiting_body.get() { let mut pending = body::PendingValue::new(&self.global_this); - pending.size_hint = self.get_size_hint(); - pending.task = Some(NonNull::from(&mut *self).cast::()); - pending.on_start_streaming = - Some(FetchTasklet::on_start_streaming_http_response_body_callback); - pending.on_readable_stream_available = Some(FetchTasklet::on_readable_stream_available); - pending.on_start_buffering = Some(FetchTasklet::on_start_buffering_callback); - pending.producer = SourceHandle::FetchResponseBody(bun_ptr::BackRef::new_mut(self)); + pending.size_hint = Self::get_size_hint(state); + pending.producer = SourceHandle::FetchResponseBody(BackRef::new(self)); return BodyValue::Locked(pending); } - let scheduled_response_buffer = core::mem::take(&mut self.scheduled_response_buffer); + let scheduled_response_buffer = core::mem::take(&mut state.scheduled_response_buffer); let response = BodyValue::InternalBlob(InternalBlob { bytes: scheduled_response_buffer.list, was_string: false, }); - self.scheduled_response_buffer = MutableString::default(); + state.scheduled_response_buffer = MutableString::default(); response } @@ -1736,30 +1820,27 @@ impl FetchTasklet { /// requested upgrade, and the upgraded connection is then the body. fn response_body_is_null(&self, status_code: u16) -> bool { (crate::server::http_status_text::is_null_body(status_code) && status_code != 101) - || self - .http - .as_deref() - .is_some_and(|http_| http_.method() == Method::HEAD) + || self.method == Method::HEAD } /// Content the server frames anyway (a 205 with content) is dropped and the connection /// closed. `is_waiting_body` stays false: nothing may reach this body. - fn null_body_value(&mut self) -> BodyValue { - self.scheduled_response_buffer = MutableString::default(); - if self.result.has_more { + fn null_body_value(&self, state: &mut SharedState) -> BodyValue { + state.scheduled_response_buffer = MutableString::default(); + if state.result.has_more { self.abandon_response_body(); } BodyValue::Null } - fn to_response(&mut self) -> Response { + fn to_response(&self, state: &mut SharedState) -> Response { bun_output::scoped_log!(FetchTasklet, "toResponse"); - debug_assert!(self.metadata.is_some()); + debug_assert!(state.metadata.is_some()); // at this point we always should have metadata - let metadata = self.metadata.as_ref().unwrap(); + let metadata = state.metadata.as_ref().unwrap(); let http_response = &metadata.response; - // reshaped for borrowck — capture metadata fields before to_body_value() takes &mut self - let headers = FetchHeaders::create_from_pico_headers(http_response.headers.list); + // reshaped for borrowck — capture metadata fields before to_body_value() takes &mut state + let headers = HeadersRef::create_from_pico_headers(http_response.headers.list); let status_code = http_response.status_code as u16; // Fast path: when the wire reason phrase matches the canonical text for // this status code, store a StaticEncodedSlice and skip the WTF allocation. @@ -1771,17 +1852,16 @@ impl FetchTasklet { None => BunString::clone_utf8(http_response.status), }; let url = BunString::clone_utf8(metadata.url.slice()); - let redirected = self.result.redirected; + let redirected = state.result.redirected; let body = if self.response_body_is_null(status_code) { - self.null_body_value() + self.null_body_value(state) } else { - self.is_waiting_body = self.result.has_more; - self.to_body_value() + self.is_waiting_body.set(state.result.has_more); + self.to_body_value(state) }; Response::init( crate::webcore::response::Init { - // SAFETY: create_from_pico_headers returns a fresh refcount=1 FetchHeaders*. - headers: Some(unsafe { HeadersRef::adopt(headers) }), + headers: Some(headers), status_code, status_text, ..Default::default() @@ -1792,329 +1872,301 @@ impl FetchTasklet { ) } + /// Stop being the producer of a Response body that was never realised as a + /// stream, so nothing signals this tasklet through it afterwards. + fn detach_response_body_producer(&self) { + if let Some(response) = self.current_response() { + if let BodyValue::Locked(locked) = response.get_body_value() { + if matches!(locked.producer, SourceHandle::FetchResponseBody(_)) { + locked.producer = SourceHandle::None; + } + } + } + } + /// Nothing will read the rest of the body: abort the transport, let go of the loop and of the - /// response; `callback` drops whatever still arrives. Safe inside a GC sweep + /// response; `FetchShared::on_result` drops whatever still arrives. Safe inside a GC sweep /// (`on_response_finalize`, `on_body_stream_collected`): no JS cell is touched; the - /// request-body sink is left for `clear_sink()` in `deinit()`. + /// request-body sink is left for `clear_sink()` at teardown. fn abandon_response_body(&self) { bun_output::scoped_log!(FetchTasklet, "abandonResponseBody"); - self.signal_store.abandon(); + self.shared.signal_store.abandon(); self.abort_transport(); self.poll_ref .with_mut(|poll_ref| poll_ref.unref(bun_io::js_vm_ctx())); self.clear_stream_handlers(); - self.response.clear(); + self.detach_response_body_producer(); + self.response.get().clear(); self.native_response.set(None); } - fn on_resolve(&mut self) -> JSValue { + fn on_resolve(&self, state: &mut SharedState) -> JSValue { bun_output::scoped_log!(FetchTasklet, "onResolve"); - let response = bun_core::heap::into_raw(Box::new(self.to_response())); + let response = self.to_response(state); // The fetch() promise is about to resolve; from here the paused // transport should not by itself keep the event loop alive. The body - // consumer hooks (`on_start_streaming_http_response_body_callback`, - // `on_start_buffering_callback`) re-ref if the caller reads the body. - if self.is_waiting_body { + // consumer hooks (`on_start_streaming_http_response_body`, + // `on_start_buffering`) re-ref if the caller reads the body. + if self.is_waiting_body.get() { self.poll_ref .with_mut(|poll_ref| poll_ref.unref(bun_io::js_vm_ctx())); } - // SAFETY: response is a freshly allocated Response; makeMaybePooled takes ownership semantics on the JS side let global_this = self.global_this; - // SAFETY: `response` is freshly allocated above; ownership transfers to JSC. - let response_js = Response::make_maybe_pooled(&global_this, response); + // The JS wrapper owns the allocation; `native_response` is our reference, so + // the body can still be settled if JS drops the Response. + let (response_js, native_response) = response.to_js_retained(&global_this); response_js.ensure_still_alive(); - self.response = jsc::Weak::::create( + self.response.set(jsc::Weak::::create( response_js, &global_this, jsc::WeakRefType::FetchResponse, - self, - ); - // SAFETY: `response` is the live heap allocation owned by JSC after - // `make_maybe_pooled`. - self.native_response - .set(Some(unsafe { RefPtr::init_ref(response) })); + BackRef::new(self), + )); // Response-owned listener so abort still errors the body after this tasklet detaches its own. - if let Some(signal) = self.abort_signal() { - // SAFETY: `response` is the live heap allocation owned by JSC. - unsafe { Response::attach_abort_signal(response, &global_this, signal) }; + if let Some(signal) = self.signal.get().as_ref() { + native_response.attach_abort_signal(&global_this, signal.signal()); } + self.native_response.set(Some(native_response)); response_js } - fn get( + /// Build the tasklet and its request, start the request on the HTTP thread. + pub(crate) fn queue( global_this: &JSGlobalObject, fetch_options: FetchOptions, promise: jsc::JSPromiseStrong, - ) -> crate::Result<*mut FetchTasklet> { - let mut fetch_tasklet = Box::new(FetchTasklet { - sink: None, - // `AsyncHTTP` has no `Default`/zero-init; defer the Box until - // `AsyncHTTP::init` produces the value. - http: None, - result: HTTPClientResult::default(), - metadata: None, - http_ticket: None, - global_this: GlobalRef::from(global_this), - request_body: fetch_options.body, - request_body_streaming_buffer: None, - scheduled_response_buffer: MutableString::default(), - response: jsc::Weak::default(), - native_response: JsCell::new(None), - response_stream: Default::default(), - request_headers: fetch_options.headers, - promise, - concurrent_task: ConcurrentTask::default(), - poll_ref: JsCell::new(KeepAlive::default()), - body_size: http::BodySize::Unknown, - url_proxy_buffer: fetch_options.url_proxy_buffer, - signal: fetch_options.signal, - signals: Signals::default(), + ) -> crate::Result<()> { + http::http_thread::init(&http::http_thread::InitOpts::default()); + jsc::mark_binding!(); + + let FetchOptions { + method, + headers, + body, + disable_timeout, + idle_timeout_seconds, + disable_keepalive, + disable_decompression, + max_redirects, + reject_unauthorized, + url_proxy_buffer, + url_len, + has_proxy, + verbose, + redirect_type, + proxy_headers, + signal, + check_server_identity, + unix_socket_path, + ssl_config, + upgraded_connection, + forced_protocol, + is_node_http_client, + compress, + } = fetch_options; + + let is_stream = matches!(body, HTTPRequestBody::ReadableStream(_)); + // Out on the HTTP thread from `start` below until it hands the request + // back: the VM waits for it (the ticket) and aborts it at teardown (registry). + let shared = Arc::new(FetchShared { + state: Guarded::new(SharedState::default()), signal_store: http::signals::Store::default(), has_schedule_callback: AtomicBool::new(false), - abort_reason: StrongOptional::empty(), - check_server_identity: fetch_options.check_server_identity, - reject_unauthorized: fetch_options.reject_unauthorized, - upgraded_connection: fetch_options.upgraded_connection, - unix_socket_path: fetch_options.unix_socket_path, - is_waiting_body: false, - is_waiting_abort: false, - is_waiting_request_stream_start: false, - mutex: Mutex::new(), - // SAFETY: jsc_vm derived from FFI ptr above; AsyncTaskTracker::init only - // bumps a counter on the VM. - tracker: AsyncTaskTracker::init(global_this.bun_vm().as_mut()), - ref_count: bun_ptr::ThreadSafeRefCount::init(), + is_http2: AtomicBool::new(false), + tasklet: AtomicPtr::new(core::ptr::null_mut()), + progress_task: ReusableConcurrentTask::default(), + hand_back_task: ReusableConcurrentTask::default(), + posts_drain_hops: is_stream, + ticket: global_this.bun_vm().ticket().in_flight(), }); + let mut signals = shared.signal_store.to_with_backpressure(); + if check_server_identity.has() && reject_unauthorized { + shared + .signal_store + .cert_errors + .store(true, Ordering::Relaxed); + } else { + signals.cert_errors = None; + } + // we want to return after headers are received + shared + .signal_store + .header_progress + .store(true, Ordering::Relaxed); - fetch_tasklet.signals = fetch_tasklet.signal_store.to_with_backpressure(); - - fetch_tasklet.tracker.did_schedule(global_this); - - // `body` is *moved* through `FetchOptions` into `request_body` (no - // shallow alias, no post-queue detach), so the RefPtr already carries - // the caller's +1 — bumping it again here leaked one ref per - // Blob-backed body (issue: fetch-leak fixture #5 RSS growth). - // `clear_data() → request_body.detach()` releases it. + let (in_memory_body, request_body) = match body { + HTTPRequestBody::AnyBlob(blob) => (blob, HTTPRequestBody::default()), + other => (AnyBlob::Blob(Blob::default()), other), + }; + // `body` was *moved* through `FetchOptions` (no shallow alias, no + // post-queue detach), so the `RefPtr` already carries the caller's +1; + // `clear_data()` releases it with the request storage. - let url = fetch_options.url; let env = global_this.bun_vm().as_mut().transpiler.env_mut(); // Capture the proxy env so the HTTP thread can re-resolve per redirect // hop (`HTTPClient::reevaluate_proxy_for_redirect`). `ProxySettings` // owns copies of the env values, so a later `process.env.HTTP_PROXY = // ...` on the JS thread cannot invalidate them mid-request. - let proxy_settings: Option> = - if let Some(proxy_opt) = &fetch_options.proxy { - if !proxy_opt.is_empty() { - http::ProxySettings::from_explicit(proxy_opt.href, env) - } else { - // proxy: "" means explicitly no proxy (direct connection) - None - } + let proxy_settings: Option> = if has_proxy { + let proxy = ZigURL::parse(&url_proxy_buffer[url_len..]); + if !proxy.is_empty() { + http::ProxySettings::from_explicit(proxy.href, env) } else { - http::ProxySettings::from_env(env) - }; - // Hop-0 proxy borrows the boxed `ProxySettings` heap storage, which is - // moved into `AsyncHTTP::init` below and lives on `client` for the - // lifetime of the request. - let proxy: Option = proxy_settings.as_deref().and_then(|s| { - let href: *const [u8] = s.resolve(&url)?; - // SAFETY: see block comment above. - Some(ZigURL::parse(unsafe { &*href })) - }); - - if fetch_tasklet.check_server_identity.has() && fetch_tasklet.reject_unauthorized { - fetch_tasklet - .signal_store - .cert_errors - .store(true, Ordering::Relaxed); + // proxy: "" means explicitly no proxy (direct connection) + None + } } else { - fetch_tasklet.signals.cert_errors = None; - } - - let fetch_tasklet_ptr = bun_core::heap::into_raw(fetch_tasklet); - // SAFETY: just allocated; exclusive access until returned - let fetch_tasklet = unsafe { &mut *fetch_tasklet_ptr }; - - // This task gets queued on the HTTP thread. - // `AsyncHTTP::init` takes several `&'static [u8]` borrows - // (headers_buf, request_body, unix_socket_path) that point into - // FetchTasklet-owned storage. The tasklet is heap-pinned via - // `heap::alloc`, so erase the borrow lifetimes through raw pointers. - // SAFETY: `fetch_tasklet_ptr` is a stable heap allocation that outlives - // the AsyncHTTP (dropped together in `deinit`); the slices below borrow - // its `request_headers.buf`, `request_body`, and `unix_socket_path` - // fields which are not reallocated for the lifetime of the request. - // SAFETY (`Interned::assume` — Population B, holder-backed): - // `fetch_tasklet_ptr` is a `heap::alloc`'d `FetchTasklet` whose - // `request_headers.buf` / `request_body` / - // `unix_socket_path` fields are not reallocated for the request's - // lifetime, and the tasklet is freed in `deinit` only after the owned - // `AsyncHTTP` is dropped. NOT process-lifetime — these should become - // `RawSlice` once `AsyncHTTP::init` accepts holder-lifetime slices; - // `assume` names the owner so the widen is grep-able until then. - let headers_buf: &'static [u8] = - unsafe { bun_ptr::Interned::assume(fetch_tasklet.request_headers.buf.as_slice()) } - .as_bytes(); - // SAFETY: see `Interned::assume` note above — same heap-pinned `FetchTasklet` owner. - let request_body_slice: &'static [u8] = - unsafe { bun_ptr::Interned::assume(fetch_tasklet.request_body.slice()) }.as_bytes(); - // SAFETY: see block note above — same `FetchTasklet` owner. - let unix_socket_path: &'static [u8] = - unsafe { bun_ptr::Interned::assume(&fetch_tasklet.unix_socket_path) }.as_bytes(); - // `MultiArrayList` owns its - // allocation, so clone; AsyncHTTP::init clones again for the client. - let header_entries = bun_core::handle_oom(fetch_tasklet.request_headers.entries.clone()); - // `url` is moved into `AsyncHTTP::init`; capture the one - // post-move query (`is_http()`, debug-assert only) up front. - let url_is_http = url.is_http(); - - fetch_tasklet.http = Some(Box::new(AsyncHTTP::init( - fetch_options.method, - url, - header_entries, + http::ProxySettings::from_env(env) + }; + let unframed_by_headers = + headers.get(b"content-length").is_some() && headers.get(b"transfer-encoding").is_none(); + let Headers { + entries: header_entries, + buf: headers_buf, + } = headers; + let storage = FetchRequestStorage { + url_proxy_buffer, + url_len, headers_buf, - request_body_slice, - // handles response events (on headers, on body, etc.) - http::HTTPClientResultCallback::new_with_release::( - fetch_tasklet_ptr, - // SAFETY: `new_with_release` guarantees the pointer/lifetime - // contract `callback` documents. - FetchTasklet::callback, - FetchTasklet::release_at_shutdown, - ), - fetch_options.redirect_type, - http::async_http::Options { - http_proxy: proxy, - proxy_settings, - proxy_headers: fetch_options.proxy_headers, - signals: Some(fetch_tasklet.signals), - unix_socket_path: Some(unix_socket_path), - disable_timeout: Some(fetch_options.disable_timeout), - idle_timeout_seconds: fetch_options.idle_timeout_seconds, - disable_keepalive: Some(fetch_options.disable_keepalive), - disable_decompression: Some(fetch_options.disable_decompression), - max_redirects: fetch_options.max_redirects, - reject_unauthorized: Some(fetch_options.reject_unauthorized), - verbose: Some(fetch_options.verbose), - tls_props: fetch_options.ssl_config, - compress: fetch_options.compress, - }, - ))); - // enable streaming the write side - let is_stream = matches!( - fetch_tasklet.request_body, - HTTPRequestBody::ReadableStream(_) - ); - let http_client = fetch_tasklet.http.as_mut().unwrap(); - http_client.client.flags.is_streaming_request_body = is_stream; - http_client.client.flags.forced_protocol = fetch_options.forced_protocol; - http_client.client.flags.is_node_http_client = fetch_options.is_node_http_client; - fetch_tasklet.is_waiting_request_stream_start = is_stream; - if is_stream { - // Intrusive `ref_count` starts at 2 (one for the main thread, one for the HTTP - // thread), so the same raw pointer can be handed to both sides. - let buffer = ThreadSafeStreamBuffer::new(ThreadSafeStreamBuffer::default()); - // SAFETY: fresh heap allocation from `ThreadSafeStreamBuffer::new` (heap::alloc); - // exclusively owned here until shared below. - unsafe { - (*buffer).set_drain_callback::( - FetchTasklet::on_write_request_data_drain, - fetch_tasklet_ptr, - ); - } - // SAFETY: adopts one of the two initial refs. - fetch_tasklet.request_body_streaming_buffer = Some(unsafe { RefPtr::from_raw(buffer) }); - fetch_tasklet.http.as_mut().unwrap().request_body = - http::HTTPRequestBody::Stream(http::http_request_body::Stream { - buffer: core::ptr::NonNull::new(buffer), - ended: false, - }); - } - // TODO is this necessary? the http client already sets the redirect type, - // so manually setting it here seems redundant - if fetch_options.redirect_type != FetchRedirect::Follow { - fetch_tasklet - .http - .as_mut() - .unwrap() - .client - .remaining_redirect_count = 0; - } + unix_socket_path, + body: in_memory_body, + }; - // we want to return after headers are received - fetch_tasklet - .signal_store - .header_progress - .store(true, Ordering::Relaxed); + let this = RefPtr::new_cyclic(|self_ref| FetchTasklet { + ref_count: Cell::new(1), + self_ref, + shared: Arc::clone(&shared), + request: JsCell::new(None), + method, + sink: JsCell::new(None), + global_this: GlobalRef::from(global_this), + request_body: JsCell::new(request_body), + request_body_streaming_buffer: JsCell::new(None), + response: JsCell::new(jsc::Weak::default()), + native_response: JsCell::new(None), + response_stream: ProducerHold::default(), + promise: JsCell::new(promise), + poll_ref: JsCell::new(KeepAlive::default()), + signal: JsCell::new(None), + abort_reason: JsCell::new(StrongOptional::empty()), + check_server_identity: JsCell::new(check_server_identity), + reject_unauthorized, + upgraded_connection, + unframed_by_headers, + is_waiting_body: Cell::new(false), + is_waiting_abort: Cell::new(false), + is_waiting_request_stream_start: Cell::new(is_stream), + tracker: AsyncTaskTracker::init(global_this.bun_vm().as_mut()), + progress_ref: JsCell::new(None), + pending_settle: JsCell::new(None), + settle_ref: JsCell::new(None), + http_ref: JsCell::new(None), + request_stream_ref: JsCell::new(None), + }); + let this_ptr = this.this_ptr(); + // The reference `RefPtr::new_cyclic` created is the JS side's. + this_ptr.progress_ref.set(Some(this)); + let this = this_ptr; + shared.tasklet.store(this.as_ptr(), Ordering::Relaxed); + + this.tracker.did_schedule(global_this); + + let request_body_buffer = if is_stream { + let handler: Arc = Arc::::clone(&shared); + let buffer = ThreadSafeStreamBuffer::create(handler); + let attached = http::http_request_body::Stream::attach(&buffer); + this.request_body_streaming_buffer.set(Some(buffer)); + Some(attached) + } else { + None + }; + let sendfile = match this.request_body.get() { + HTTPRequestBody::Sendfile(sendfile) => { + debug_assert!(!has_proxy); + Some(*sendfile) + } + _ => None, + }; - if let HTTPRequestBody::Sendfile(sendfile) = &fetch_tasklet.request_body { - debug_assert!(url_is_http); - debug_assert!(fetch_options.proxy.is_none()); - fetch_tasklet.http.as_mut().unwrap().request_body = - http::HTTPRequestBody::Sendfile(*sendfile); - } + let handler_arc: Arc = Arc::clone(&shared); + let mut request = OwnedRequest::new(storage, |storage| { + let url = storage.url(); + debug_assert!(sendfile.is_none() || url.is_http()); + AsyncHTTP::init( + method, + url, + header_entries, + storage.headers_buf.as_slice(), + storage.body.slice(), + // handles response events (on headers, on body, etc.) + http::HTTPClientResultCallback::from_handler(handler_arc), + redirect_type, + http::async_http::Options { + // Hop 0's proxy is resolved from `proxy_settings` too. + http_proxy: None, + proxy_settings, + proxy_headers, + signals: Some(signals), + unix_socket_path: Some(&storage.unix_socket_path), + disable_timeout: Some(disable_timeout), + idle_timeout_seconds, + disable_keepalive: Some(disable_keepalive), + disable_decompression: Some(disable_decompression), + max_redirects, + reject_unauthorized: Some(reject_unauthorized), + verbose: Some(verbose), + tls_props: ssl_config, + compress, + }, + ) + }); + request.with_http_mut(|http_client| { + http_client.client.flags.is_streaming_request_body = is_stream; + http_client.client.flags.forced_protocol = forced_protocol; + http_client.client.flags.is_node_http_client = is_node_http_client; + if let Some(stream) = request_body_buffer { + http_client.request_body = http::HTTPRequestBody::Stream(stream); + } + // TODO is this necessary? the http client already sets the redirect type, + // so manually setting it here seems redundant + if redirect_type != FetchRedirect::Follow { + http_client.client.remaining_redirect_count = 0; + } + if let Some(sendfile) = sendfile { + http_client.request_body = http::HTTPRequestBody::Sendfile(sendfile); + } + }); - if let Some(signal) = &fetch_tasklet.signal { - signal.pending_activity_ref(); - signal.add_listener(fetch_tasklet_ptr.cast::(), Self::__abort_listener_c); + if let Some(signal) = signal { + this.signal.set(Some( + signal.listen_native(this.self_ref.backref(this.get())), + )); } - Ok(fetch_tasklet_ptr) - } - #[bun_uws::uws_callback] - pub(crate) fn abort_listener(&mut self, reason: JSValue) { - bun_output::scoped_log!(FetchTasklet, "abortListener"); - let this = self; - reason.ensure_still_alive(); - this.abort_reason.set(&this.global_this, reason); - this.abort_task(); - if this.sink_mut().is_some() { - this.cancel_request_body_sink(reason); - return; - } - // Abort fired before the HTTP thread asked for the body, so the - // ReadableStream was never wired into a sink. Cancel it directly so - // the underlying source's cancel(reason) callback still observes the - // signal's reason (https://fetch.spec.whatwg.org/#abort-fetch step 5). - if this.is_waiting_request_stream_start { - if let HTTPRequestBody::ReadableStream(stream_ref) = &this.request_body { - this.is_waiting_request_stream_start = false; - if let Some(stream) = stream_ref.get() { - crate::dispatch::fold(stream.cancel_with_reason(&this.global_this, reason)); - } - } - } - } + this.poll_ref + .with_mut(|poll_ref| poll_ref.ref_(bun_io::js_vm_ctx())); + // The HTTP thread's reference, from `start` until it hands the request back. + this.http_ref.set(Some(RefPtr::from_this(this))); + crate::jsc_hooks::ActiveHandle::Fetch(NonNull::from(this)).register(); + let mut batch = bun_threading::thread_pool::Batch::default(); + this.request.set(Some(request.start(&mut batch))); + http::HTTPThread::schedule(batch); - /// This is ALWAYS called from the http thread and we cannot touch the buffer here because is locked - fn on_write_request_data_drain(this: *mut FetchTasklet) { - let this_ref = Self::from_raw_ref(this); - // ref until the main thread callback is called - this_ref.ref_(); - // `from_callback` heap-allocates a fresh `ConcurrentTaskItem`. - let task = ConcurrentTask::from_callback(this, FetchTasklet::resume_request_data_stream); - this_ref - .http_ticket - .as_ref() - .expect(Self::HOLDS_TICKET) - .post(task); + Ok(()) } - /// This is ALWAYS called from the main thread - // ConcurrentTask::from_callback expects `fn(*mut T) -> bun_event_loop::JsResult<()>`. - fn resume_request_data_stream(this: *mut FetchTasklet) -> ElJsResult<()> { - let this_ref = Self::from_raw_mut(this); + /// This is ALWAYS called from the main thread: the HTTP thread drained the + /// request body buffer (`FetchShared::on_drain`). + pub(crate) fn resume_request_data_stream(this: ThisPtr) { bun_output::scoped_log!(FetchTasklet, "resumeRequestDataStream"); - if !this_ref.signal_aborted() { - let global_this = this_ref.global_this; - if let Some(sink) = this_ref.sink_mut() { + // The stream this resumes may end and release `request_stream_ref`. + let _guard = RefPtr::from_this(this); + if !this.signal_aborted() { + let global_this = this.global_this; + if let Some(sink) = this.sink() { sink.on_drain(&global_this); } } - // deref when done because we ref inside onWriteRequestDataDrain - // SAFETY: `this` is the live heap tasklet; we hold a ref. - FetchTasklet::deref(this); - Ok(()) } /// Whether the request body should skip chunked transfer encoding framing. @@ -2122,16 +2174,15 @@ impl FetchTasklet { /// set Content-Length without setting Transfer-Encoding. pub(crate) fn skip_chunked_framing(&self) -> bool { self.upgraded_connection - || self.result.is_http2 - || (self.request_headers.get(b"content-length").is_some() - && self.request_headers.get(b"transfer-encoding").is_none()) + || self.shared.is_http2.load(Ordering::Relaxed) + || self.unframed_by_headers } /// Called from `FetchRequestBodySink::write_*`; `high_water_mark` is the /// sink's configured HWM so the backpressure threshold tracks /// `start({ highWaterMark })`. pub(crate) fn write_request_data( - &mut self, + &self, data: RequestBodyChunk<'_>, high_water_mark: usize, ) -> Writable { @@ -2150,11 +2201,11 @@ impl FetchTasklet { return Writable::Owned(0); } let len = utf8_len as BlobSizeType; - let Some(thread_safe_stream_buffer) = self.stream_buffer_mut() else { + let Some(thread_safe_stream_buffer) = self.request_body_buffer() else { return Writable::Done; }; // Mutex guards `buffer` against the HTTP thread; released when - // `stream_buffer` drops. Borrow is detached from `self` (see accessor). + // `stream_buffer` drops. let mut stream_buffer = thread_safe_stream_buffer.lock(); // dont have backpressure so we will schedule the data to be written @@ -2183,43 +2234,54 @@ impl FetchTasklet { } else { Writable::Owned(len) }; + drop(stream_buffer); if needs_schedule { // wakeup the http thread to write the data - http::http_thread().schedule_request_write( - self.http.as_mut().unwrap(), - http::http_thread::WriteMessageType::Data, - ); + if let Some(id) = self.async_http_id() { + http::http_thread() + .schedule_request_write(id, http::http_thread::WriteMessageType::Data); + } } // pause the stream if we hit the high water mark result } - pub(crate) fn write_end_request(&mut self, err: Option) { + /// The request body stream is done (`err`: why, if it failed): flush the + /// terminator and release `request_stream_ref`. May free the tasklet. + pub(crate) fn write_end_request(this: ThisPtr, err: Option) { + let _guard = RefPtr::from_this(this); + this.write_end_request_impl(err); + } + + /// [`write_end_request`](Self::write_end_request) for callers that already + /// hold a reference across the call. + fn write_end_request_impl(&self, err: Option) { bun_output::scoped_log!(FetchTasklet, "writeEndRequest hasError? {}", err.is_some()); - let this_ptr = std::ptr::from_mut(self); + self.end_request(err); + drop(self.request_stream_ref.replace(None)); + } + + fn end_request(&self, err: Option) { if let Some(js_error) = err { - if self.signal_store.aborted.load(Ordering::Relaxed) || self.abort_reason.has() { - // SAFETY: `this_ptr` derived from live `&mut self`; we hold a ref. - FetchTasklet::deref(this_ptr); + if self.shared.signal_store.aborted.load(Ordering::Relaxed) + || self.abort_reason.get().has() + { return; } if !js_error.is_undefined_or_null() { - self.abort_reason.set(&self.global_this, js_error); + self.abort_reason + .with_mut(|r| r.set(&self.global_this, js_error)); } self.abort_task(); } else { - if self.signal_store.aborted.load(Ordering::Relaxed) { - // SAFETY: `this_ptr` derived from live `&mut self`; we hold a ref. - FetchTasklet::deref(this_ptr); + if self.shared.signal_store.aborted.load(Ordering::Relaxed) { return; } if !self.skip_chunked_framing() { // Using chunked transfer encoding, send the terminating chunk - let Some(thread_safe_stream_buffer) = self.stream_buffer_mut() else { - // SAFETY: `this_ptr` derived from live `&mut self`; we hold a ref. - FetchTasklet::deref(this_ptr); + let Some(thread_safe_stream_buffer) = self.request_body_buffer() else { return; }; // Mutex guards `buffer` against the HTTP thread; released when @@ -2228,13 +2290,17 @@ impl FetchTasklet { .lock() .write(http::END_OF_CHUNKED_HTTP1_1_ENCODING_RESPONSE_BODY); // OOM/capacity: fire-and-forget } - if let Some(http_) = self.http.as_mut() { + if let Some(id) = self.async_http_id() { http::http_thread() - .schedule_request_write(http_, http::http_thread::WriteMessageType::End); + .schedule_request_write(id, http::http_thread::WriteMessageType::End); } } - // SAFETY: `this_ptr` derived from live `&mut self`; we hold a ref. - FetchTasklet::deref(this_ptr); + } + + /// The sink's fallback release of `request_stream_ref` (its `finalize`). + /// May free the tasklet. + pub(crate) fn release_request_stream_ref(this: ThisPtr) { + Self::release(this, |t| &t.request_stream_ref); } fn abort_task(&self) { @@ -2246,11 +2312,16 @@ impl FetchTasklet { /// Idempotent: an AbortSignal, VM teardown and `abandon_response_body` can all reach here for /// the same fetch. Only the first enqueues a shutdown. No JS. fn abort_transport(&self) -> bool { - if self.signal_store.aborted.swap(true, Ordering::Relaxed) { + if self + .shared + .signal_store + .aborted + .swap(true, Ordering::Relaxed) + { return false; } - if let Some(http_) = self.http.as_deref() { - http::http_thread().schedule_shutdown(http_); + if let Some(id) = self.async_http_id() { + http::http_thread().schedule_shutdown_by_id(id); } true } @@ -2262,318 +2333,130 @@ impl FetchTasklet { /// `assign_to_stream` pump-promise settlement (on_resolve/on_reject); on /// the native ByteStream path there is no pump promise, so balance it /// here. No-op when no sink or already ended. - pub(crate) fn cancel_request_body_sink(&mut self, reason: JSValue) { - let Some(sink) = self.sink_mut() else { + pub(crate) fn cancel_request_body_sink(&self, reason: JSValue) { + let Some(sink) = self.sink() else { return; }; - if sink.ended { + if sink.ended.get() { return; } - sink.ended = true; - sink.done = true; - let is_native = matches!( - sink.source, - SourceHandle::ByteStream(_) | SourceHandle::FileReader(_) - ); - if !reason.is_empty_or_undefined_or_null() && !self.abort_reason.has() { + sink.ended.set(true); + sink.done.set(true); + let is_native = sink.is_native_source(); + if !reason.is_empty_or_undefined_or_null() && !self.abort_reason.get().has() { let global_this = self.global_this; - self.abort_reason.set(&global_this, reason); + self.abort_reason.with_mut(|r| r.set(&global_this, reason)); } self.abort_task(); - if let Some(sink) = self.sink_mut() { - sink.pending.result = Writable::Done; - sink.pending.run(); - sink.source.close(None); + if let Some(sink) = self.sink() { + sink.cancel(); if is_native { - sink.task = None; + sink.task.set(None); } } if is_native { - // No pump promise exists to balance the `+1` from - // `start_request_stream`; `aborted` is set above so - // `write_end_request(Some(_))` is just the balancing deref. - self.write_end_request(Some(reason)); + // No pump promise exists to release `request_stream_ref`; `aborted` + // is set above so this is just that release. + self.write_end_request_impl(Some(reason)); } } +} - pub(crate) fn queue( - global: &JSGlobalObject, - fetch_options: FetchOptions, - promise: jsc::JSPromiseStrong, - ) -> crate::Result<*mut FetchTasklet> { - http::http_thread::init(&http::http_thread::InitOpts::default()); - let node = Self::get(global, fetch_options, promise)?; - - let node_ref = Self::from_raw_mut(node); - let mut batch = bun_threading::thread_pool::Batch::default(); - node_ref.http.as_mut().unwrap().schedule(&mut batch); - node_ref - .poll_ref - .with_mut(|poll_ref| poll_ref.ref_(bun_io::js_vm_ctx())); - - // increment ref so we can keep it alive until the http client is done - node_ref.ref_(); - // Out on the HTTP thread from here until its final callback: the VM - // aborts it at teardown (registry) and waits for it (the ticket). - node_ref.http_ticket = Some(global.bun_vm().ticket()); - crate::jsc_hooks::ActiveHandle::Fetch(NonNull::new(node).expect("tasklet")).register(); - http::HTTPThread::schedule(batch); - - Ok(node) +/// `WeakRefType::FetchResponse`'s finalize callback: the Response's JS wrapper +/// (`response`) was collected. Inside a GC sweep: decide from native state only. +// HOST_EXPORT(Bun__FetchResponse_finalize, c) +pub fn on_response_finalize(this: &crate::webcore::fetch::FetchTasklet) { + bun_output::scoped_log!(FetchTasklet, "onResponseFinalize"); + let Some(response) = this.native_response.get().as_deref() else { + return; + }; + let BodyValue::Locked(locked) = response.get_body_value() else { + // The body arrived or failed; nothing is underway. + return; + }; + // What can outlive the Response and still take the body: its stream (whose own collection + // is `on_body_stream_collected`), or a whole-body consumer (`.text()` and friends hold a + // promise, `Bun.write` an `on_receive_value`). + let outlived = this.response_stream.is_held() + || locked.on_receive_value.is_some() + || locked + .promise + .is_some_and(|promise| !promise.is_empty_or_undefined_or_null()); + if !outlived { + this.abandon_response_body(); } +} - /// Called from HTTP thread. Handles HTTP events received from socket. - /// - /// # Safety - /// `task` must be a live heap-allocated `FetchTasklet` with the - /// HTTP-thread ref still held; `async_http` must point to the HTTP - /// thread's live `AsyncHTTP` for the duration of the call. - // Signature is fixed by `HTTPClientResultCallback`; `task` may be freed by the - // trailing `deref_from_thread`, so it cannot become `&mut`. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn callback( - task: *mut FetchTasklet, - async_http: *mut AsyncHTTP<'static>, - mut result: HTTPClientResult, - ) { - // at this point only this thread is accessing result to is no race condition - let is_done = !result.has_more; - let task_ref = Self::from_raw_mut(task); - - task_ref.mutex.lock(); - // we need to unlock before task.deref(); - // explicit unlock + deref at end instead of nested defers. - // Sync HTTP-thread state back into the JS-side instance via an - // explicit field-subset copy (`AsyncHTTP` is not `Copy`: - // `HTTPClient: Drop`, owned Vecs); see `AsyncHTTP::sync_progress_from` - // for the field list. - // SAFETY: `async_http` is the HTTP-thread copy passed by `on_async_http_callback`; - // it is alive for the duration of this call and not mutated concurrently (HTTP - // thread is blocked in the callback). - task_ref - .http - .as_mut() - .unwrap() - .sync_progress_from(unsafe { &*async_http }); - - bun_output::scoped_log!( - FetchTasklet, - "callback success={} receive_mode={:?} has_more={} bytes={}", - result.is_success(), - task_ref.signal_store.body_receive_mode(), - result.has_more, - result.body.len() - ); - - let prev_metadata = task_ref.result.metadata.take(); - let prev_cert_info = task_ref.result.certificate_info.take(); - let prev_can_stream = task_ref.result.can_stream; - // `result.body` borrows the HTTP thread's scratch buffer on non-terminal - // callbacks; the terminal callback carries the bytes in `body_owned` - // instead. Capture both before `detach_lifetime` clears them in the - // stored copy. - let body: &[u8] = result.body; - let body_owned: Vec = core::mem::take(&mut result.body_owned); - // SAFETY: lifetime erasure for non-body fields; `body` is stored as - // `&'static []` so no borrow escapes. - task_ref.result = unsafe { result.detach_lifetime() }; - // can_stream is a one-shot signal to start the request body stream; don't let a - // later coalesced result clobber it before the JS thread sees it. - task_ref.result.can_stream = task_ref.result.can_stream || prev_can_stream; - - // Preserve pending certificate info if it was preovided in the previous update. - if task_ref.result.certificate_info.is_none() { - if let Some(cert_info) = prev_cert_info { - task_ref.result.certificate_info = Some(cert_info); - } - } - - // metadata should be provided only once - if let Some(metadata) = task_ref.result.metadata.take().or(prev_metadata) { - bun_output::scoped_log!(FetchTasklet, "added callback metadata"); - if task_ref.metadata.is_none() { - task_ref.metadata = Some(metadata); - } - - task_ref.result.metadata = None; - } - - task_ref.body_size = task_ref.result.body_size; - - let success = task_ref.result.is_success(); - - if task_ref.signal_store.body_receive_mode() == BodyReceiveMode::Abandoned { - if task_ref.scheduled_response_buffer.list.capacity() > 0 { - task_ref.scheduled_response_buffer = MutableString::default(); - } - if success && task_ref.result.has_more { - task_ref.mutex.unlock(); - return; - } - } else if success { - let scheduled = &mut task_ref.scheduled_response_buffer; - if body.is_empty() && !body_owned.is_empty() && scheduled.list.is_empty() { - scheduled.list = body_owned; - } else { - // Grow to Content-Length once so the per-packet append below - // doesn't leave the ~2x doubling over-capacity that the - // ArrayBuffer would adopt. Only for a consumer that wants the whole body. - if task_ref.signal_store.body_receive_mode() == BodyReceiveMode::BufferAll { - if let http::BodySize::ContentLength(n) = task_ref.body_size { - if n > scheduled.list.capacity() { - let additional = n - .min(SCHEDULED_PRERESERVE_MAX) - .saturating_sub(scheduled.list.len()); - let _ = scheduled.list.try_reserve_exact(additional); - } - } - } - let chunk = if body.is_empty() { - body_owned.as_slice() - } else { - body - }; - if !chunk.is_empty() { - bun_core::handle_oom(scheduled.write(chunk)); - } - } - if task_ref.result.has_more - && task_ref.scheduled_response_buffer.list.len() >= BODY_HIGH_WATER_MARK - { - task_ref.signal_store.pause_receive(); - } +impl jsc::NativeAbortListener for FetchTasklet { + fn on_abort(this: ThisPtr, reason: JSValue) { + bun_output::scoped_log!(FetchTasklet, "abortListener"); + // Cancelling the sink may release `request_stream_ref`. + let _guard = RefPtr::from_this(this); + reason.ensure_still_alive(); + this.abort_reason + .with_mut(|r| r.set(&this.global_this, reason)); + this.abort_task(); + if this.sink().is_some() { + this.cancel_request_body_sink(reason); + return; } - - if let Err(has_schedule_callback) = task_ref.has_schedule_callback.compare_exchange( - false, - true, - Ordering::Acquire, - Ordering::Relaxed, - ) { - if has_schedule_callback { - task_ref.mutex.unlock(); - if is_done { - FetchTasklet::hand_back(task); + // Abort fired before the HTTP thread asked for the body, so the + // ReadableStream was never wired into a sink. Cancel it directly so + // the underlying source's cancel(reason) callback still observes the + // signal's reason (https://fetch.spec.whatwg.org/#abort-fetch step 5). + if this.is_waiting_request_stream_start.get() { + if let HTTPRequestBody::ReadableStream(stream_ref) = this.request_body.get() { + this.is_waiting_request_stream_start.set(false); + if let Some(stream) = stream_ref.get() { + crate::dispatch::fold(stream.cancel_with_reason(&this.global_this, reason)); } - return; } } - // will deinit when done with the http client (when is_done = true) - let ct = core::ptr::NonNull::from( - task_ref - .concurrent_task - .from(task, AutoDeinit::ManualDeinit), - ); - // `ct` is the inline `concurrent_task` field of the heap tasklet; the - // queue takes ownership of its `next` link. This thread's ref keeps the - // tasklet (and the ticket in it) alive across the post. - task_ref - .http_ticket - .as_ref() - .expect(Self::HOLDS_TICKET) - .post(ct); + } +} - task_ref.mutex.unlock(); - // we are done with the http client so we can deref our side - // this is a atomic operation and will enqueue a task to deinit on the main thread - if is_done { - FetchTasklet::hand_back(task); - } +impl Drop for FetchTasklet { + fn drop(&mut self) { + bun_output::scoped_log!(FetchTasklet, "deinit"); + // JS thread: no longer something the VM must abort at teardown. + crate::jsc_hooks::ActiveHandle::Fetch(NonNull::from(&*self)).unregister(); + self.clear_data(); } } -fn on_resolve_request_stream( +/// The `assign_to_stream` pump settled: the request body stream is done. +// HOST_EXPORT(Bun__FetchTasklet__onResolveRequestStream) +pub fn on_resolve_request_stream( + this: ThisPtr, _global_this: &JSGlobalObject, - callframe: &bun_jsc::CallFrame, + _callframe: &bun_jsc::CallFrame, ) -> JsResult { - let args = callframe.arguments(); - let this: *mut FetchTasklet = args[args.len() - 1].as_promise_ptr::(); - // SAFETY: `as_promise_ptr` recovers the `*mut FetchTasklet` stashed by - // `start_request_stream`; the `ref_()` there keeps it alive, balanced by - // `write_end_request` below. Clear `sink.task` first so the sink's - // `finalize()` fallback does not release a second time. - unsafe { - if let Some(sink) = (*this).sink_mut() { - sink.task = None; - } - (*this).write_end_request(None); + let _guard = RefPtr::from_this(this); + // Clear `sink.task` first so the sink's `finalize()` fallback does not + // release a second time. + if let Some(sink) = this.sink() { + sink.task.set(None); } + this.write_end_request_impl(None); Ok(JSValue::UNDEFINED) } -fn on_reject_request_stream( +/// The `assign_to_stream` pump rejected: the request body stream failed. +// HOST_EXPORT(Bun__FetchTasklet__onRejectRequestStream) +pub fn on_reject_request_stream( + this: ThisPtr, _global_this: &JSGlobalObject, callframe: &bun_jsc::CallFrame, ) -> JsResult { - let args = callframe.arguments(); - let this: *mut FetchTasklet = args[args.len() - 1].as_promise_ptr::(); - let err = args[0]; - // SAFETY: `as_promise_ptr` recovers the `*mut FetchTasklet` stashed by - // `start_request_stream`; the `ref_()` there keeps it alive, balanced by - // `write_end_request` below. Clear `sink.task` first so the sink's - // `finalize()` fallback does not release a second time. - unsafe { - if let Some(sink) = (*this).sink_mut() { - sink.task = None; - } - (*this).write_end_request(Some(err)); + let err = callframe.argument(0); + let _guard = RefPtr::from_this(this); + if let Some(sink) = this.sink() { + sink.task.set(None); } + this.write_end_request_impl(Some(err)); Ok(JSValue::UNDEFINED) } -// Exported as function symbols so `Zig::GlobalObject::promiseHandlerID`'s -// address comparison matches; see `Bun__FileSink__onResolveStream` for why a -// `static` fn-ptr export would fail. -bun_jsc::jsc_host_abi! { - #[unsafe(export_name = "Bun__FetchTasklet__onResolveRequestStream")] - unsafe fn on_resolve_request_stream_shim( - g: *mut JSGlobalObject, - cf: *mut bun_jsc::CallFrame, - ) -> JSValue { - match on_resolve_request_stream(bun_opaque::opaque_deref(g), bun_opaque::opaque_deref(cf)) { - Ok(v) => v, - Err(_) => JSValue::ZERO, - } - } -} -bun_jsc::jsc_host_abi! { - #[unsafe(export_name = "Bun__FetchTasklet__onRejectRequestStream")] - unsafe fn on_reject_request_stream_shim( - g: *mut JSGlobalObject, - cf: *mut bun_jsc::CallFrame, - ) -> JSValue { - match on_reject_request_stream(bun_opaque::opaque_deref(g), bun_opaque::opaque_deref(cf)) { - Ok(v) => v, - Err(_) => JSValue::ZERO, - } - } -} - -impl FetchTasklet { - #[bun_uws::uws_callback(export = "Bun__FetchResponse_finalize", no_catch)] - pub(crate) fn on_response_finalize(&mut self) { - bun_output::scoped_log!(FetchTasklet, "onResponseFinalize"); - let Some(response) = self.native_response.get().as_deref() else { - return; - }; - let BodyValue::Locked(locked) = response.get_body_value() else { - // The body arrived or failed; nothing is underway. - return; - }; - // What can outlive the Response and still take the body: its stream (whose own collection - // is `on_body_stream_collected`), or a whole-body consumer (`.text()` and friends hold a - // promise, `Bun.write` an `on_receive_value`). - let outlived = self.response_stream.is_held() - || locked.on_receive_value.is_some() - || locked - .promise - .is_some_and(|promise| !promise.is_empty_or_undefined_or_null()); - if !outlived { - self.abandon_response_body(); - } - } -} - pub struct FetchOptions { pub method: Method, pub(crate) headers: Headers, @@ -2585,12 +2468,13 @@ pub struct FetchOptions { pub(crate) disable_decompression: bool, pub(crate) max_redirects: Option, pub(crate) reject_unauthorized: bool, - pub(crate) url: ZigURL<'static>, + /// url + proxy href, back to back; `url_len` splits them. + pub(crate) url_proxy_buffer: Box<[u8]>, + pub(crate) url_len: usize, + pub(crate) has_proxy: bool, pub(crate) verbose: http::HTTPVerboseLevel, pub(crate) redirect_type: FetchRedirect, - pub(crate) proxy: Option>, pub(crate) proxy_headers: Option, - pub(crate) url_proxy_buffer: Box<[u8]>, pub(crate) signal: Option, pub(crate) check_server_identity: StrongOptional, pub(crate) unix_socket_path: Box<[u8]>, @@ -2601,6 +2485,7 @@ pub struct FetchOptions { pub(crate) compress: Option, } +/// Settles the `fetch()` promise from its own event-loop task. pub(crate) struct FetchTaskletPromiseSettle { held: StrongOptional, promise: jsc::JSPromiseStrong, @@ -2608,9 +2493,30 @@ pub(crate) struct FetchTaskletPromiseSettle { success: bool, } +/// `task_tag::FetchTaskletPromiseSettle`: settle the `fetch()` promise +/// (`FetchTasklet::pending_settle`). +pub struct PromiseSettleHop; +impl TaskHop for PromiseSettleHop { + type Target = FetchTasklet; + const TAG: bun_event_loop::TaskTag = task_tag::FetchTaskletPromiseSettle; + fn run(this: ThisPtr) -> JsResult<()> { + let settle = this.pending_settle.replace(None); + let result = match settle { + Some(settle) => settle.run(), + None => Ok(()), + }; + FetchTasklet::release(this, |t| &t.settle_ref); + result + } + /// Drop the held value and promise handle without settling. + fn release_unrun(this: ThisPtr) { + this.pending_settle.set(None); + FetchTasklet::release(this, |t| &t.settle_ref); + } +} + impl FetchTaskletPromiseSettle { - #[allow(clippy::boxed_local, reason = "reclaim point for the boxed task")] - pub(crate) fn run(mut self: Box) -> JsResult<()> { + fn run(mut self) -> JsResult<()> { let prom = self.promise.value_or_empty().as_any_promise().unwrap(); let res = self.held.swap(); res.ensure_still_alive(); @@ -2624,12 +2530,3 @@ impl FetchTaskletPromiseSettle { r } } - -impl bun_event_loop::Taskable for FetchTaskletPromiseSettle { - const TAG: bun_event_loop::TaskTag = bun_event_loop::task_tag::FetchTaskletPromiseSettle; - /// Drop the held value and promise handle without settling. - unsafe fn release_unrun(this: *mut Self) { - // SAFETY: fn contract — the box the completion queued. - drop(unsafe { bun_core::heap::take(this) }); - } -} diff --git a/src/runtime/webcore/s3/client.rs b/src/runtime/webcore/s3/client.rs index d3755e134596..9e9323b324e3 100644 --- a/src/runtime/webcore/s3/client.rs +++ b/src/runtime/webcore/s3/client.rs @@ -569,8 +569,8 @@ pub struct S3UploadStreamWrapper { pub sink: Option>, pub task: RefPtr, pub(crate) end_promise: bun_jsc::JSPromiseStrong, - pub callback: Option, - pub(crate) callback_context: *mut c_void, + /// Told the upload's outcome once it settles. + pub on_done: Option)>>, /// this is owned by the task not by the wrapper pub path: bun_ptr::RawSlice, /// Roots the source ReadableStream, and the JS pump reachable only through it, until this wrapper drops. @@ -726,8 +726,8 @@ impl S3UploadStreamWrapper { } } - if let Some(callback) = self_.callback { - callback(result, self_.callback_context); + if let Some(on_done) = self_.on_done.take() { + on_done(result); } settled } @@ -808,8 +808,7 @@ pub(crate) fn upload_stream( content_encoding: Option<&[u8]>, proxy: Option<&[u8]>, request_payer: bool, - callback: Option, - callback_context: *mut c_void, + on_done: Option)>>, ) -> JsResult { let proxy_url = proxy.unwrap_or(b""); if readable_stream.is_disturbed(global_this) { @@ -953,8 +952,7 @@ pub(crate) fn upload_stream( bun_core::heap::into_raw(Box::new(S3UploadStreamWrapper { ref_count: Cell::new(2), // +1 for the stream pump (released by the .then shim / handle_*_stream) sink: None, - callback, - callback_context, + on_done, path: bun_ptr::RawSlice::new(&task.path), // SAFETY: adopts one of `task_ptr`'s two initial refs. task: unsafe { RefPtr::from_raw(task_ptr) }, @@ -1479,8 +1477,8 @@ pub(crate) fn readable_stream( global: global_static, task: Cell::new(core::ptr::null_mut()), }); - // SAFETY: `reader` is the live source made above; `wrapper` the live heap allocation. - unsafe { (*wrapper).stream.hold(&raw mut reader_mut.context) }; + // SAFETY: `wrapper` is the live heap allocation made above. + unsafe { (*wrapper).stream.hold_source(reader_mut) }; reader_mut .producer diff --git a/src/runtime/webcore/s3/simple_request.rs b/src/runtime/webcore/s3/simple_request.rs index a05b3e529091..2295911d9b24 100644 --- a/src/runtime/webcore/s3/simple_request.rs +++ b/src/runtime/webcore/s3/simple_request.rs @@ -389,10 +389,7 @@ impl S3HttpSimpleTask { ) { let previous_metadata = self.result.metadata.take(); result.body_into(&mut self.response_buffer.list); - // SAFETY: `result.body` (the only borrowed field) points at `self.response_buffer`, - // which lives for the task's lifetime — extending to `'static` here is sound for - // self-reference. - self.result = unsafe { result.detach_lifetime() }; + self.result = result.into_owned(); if self.result.metadata.is_none() { self.result.metadata = previous_metadata; } diff --git a/src/runtime/webcore/streams.rs b/src/runtime/webcore/streams.rs index e50885ccdc22..3f7a9c937c48 100644 --- a/src/runtime/webcore/streams.rs +++ b/src/runtime/webcore/streams.rs @@ -866,7 +866,7 @@ pub enum SourceHandle { /// `Subprocess<'a>`; the pointed-at allocation outlives this handle. Subprocess(BackRef>), ShellWritable(BackRef), - FetchResponseBody(BackRef), + FetchResponseBody(BackRef), ServerRequestBody(crate::server::AnyRequestContext), S3DownloadBody(BackRef), HTMLRewriter(BackRef), @@ -965,6 +965,66 @@ impl SourceHandle { } } + /// `Body::PendingValue` producer hook: a consumer wants the whole body + /// buffered. `true` if this producer handles it. + pub fn start_buffering(&self) -> bool { + match *self { + SourceHandle::FetchResponseBody(p) => { + p.on_start_buffering(); + true + } + SourceHandle::None + | SourceHandle::JSController(_) + | SourceHandle::ServerRequestBody(_) + | SourceHandle::ByteStream(_) + | SourceHandle::FileReader(_) + | SourceHandle::Subprocess(_) + | SourceHandle::ShellWritable(_) + | SourceHandle::S3DownloadBody(_) + | SourceHandle::HTMLRewriter(_) + | SourceHandle::TestingCancelOnDrain(_) => false, + } + } + + /// `Body::PendingValue` producer hook: the body is being realised as a + /// `ByteStream`; hand over whatever is already buffered. + pub fn start_streaming(&self) -> Option { + match *self { + SourceHandle::FetchResponseBody(p) => Some(p.on_start_streaming_http_response_body()), + SourceHandle::None + | SourceHandle::JSController(_) + | SourceHandle::ServerRequestBody(_) + | SourceHandle::ByteStream(_) + | SourceHandle::FileReader(_) + | SourceHandle::Subprocess(_) + | SourceHandle::ShellWritable(_) + | SourceHandle::S3DownloadBody(_) + | SourceHandle::HTMLRewriter(_) + | SourceHandle::TestingCancelOnDrain(_) => None, + } + } + + /// `Body::PendingValue` producer hook: the body's `ByteStream` now exists. + pub fn readable_stream_available( + &self, + global: &JSGlobalObject, + readable: &crate::webcore::ReadableStream, + ) { + match *self { + SourceHandle::FetchResponseBody(p) => p.on_readable_stream_available(global, readable), + SourceHandle::None + | SourceHandle::JSController(_) + | SourceHandle::ServerRequestBody(_) + | SourceHandle::ByteStream(_) + | SourceHandle::FileReader(_) + | SourceHandle::Subprocess(_) + | SourceHandle::ShellWritable(_) + | SourceHandle::S3DownloadBody(_) + | SourceHandle::HTMLRewriter(_) + | SourceHandle::TestingCancelOnDrain(_) => {} + } + } + pub fn start(&mut self) { match *self { SourceHandle::FetchResponseBody(p) => p.on_start(), From e4e316e1f62c593d22467ff0be3e918f5c3bded7 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 07:28:08 +0000 Subject: [PATCH 05/11] NetworkTask: comment names into_owned --- src/install/NetworkTask.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index 3cad4a76afd1..6b8131c7ca52 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -310,7 +310,7 @@ impl NetworkTask { } // Stash this callback's body bytes into our own accumulation buffer - // before `detach_lifetime` clears `result.body` to `&[]`. Covers the + // before `into_owned` drops `result.body`. Covers the // non-streaming manifest path and the tarball fall-through above. if result.metadata.is_some() { // First callback of a fresh attempt on the non-streaming path — From 5c2d6d989ac0389759ce5a79b4c9d1735289c1c0 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 08:14:52 +0000 Subject: [PATCH 06/11] fetch: post the progress hop before the hand-back hop; release an unrun progress hop's slot so the terminal one still lands; Bun.write streams fetch bodies again --- src/runtime/webcore/Blob.rs | 7 +++- src/runtime/webcore/fetch/FetchTasklet.rs | 46 ++++++++++++++--------- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 848b21469d30..cda6916cf570 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -5002,7 +5002,12 @@ pub(crate) fn write_file_internal( let BodyValue::Locked(locked) = (unsafe { &mut *body_value }) else { unreachable!() }; - locked.readable.has() || locked.on_start_streaming.is_some() + locked.readable.has() + || locked.on_start_streaming.is_some() + || matches!( + locked.producer, + crate::webcore::streams::SourceHandle::FetchResponseBody(_) + ) }; if streamable { // SAFETY: exclusive borrow scoped to the call (may run JS). diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index d652747861fc..68eea7165e85 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -70,8 +70,9 @@ pub(crate) struct SharedState { /// buffer used to stream response to JS pub(crate) scheduled_response_buffer: MutableString, /// The HTTP thread is done with the fetch (terminal result delivered): the - /// progress update that sees this releases `http_ref` too. Fetches with a - /// streaming request body post a `HandBackHop` instead (see `on_result`). + /// next progress hop to be consumed (run, or released unrun at teardown) + /// releases `http_ref` too. Fetches with a streaming request body post a + /// `HandBackHop` instead (see `on_result`). handed_back: bool, /// `process.exit()` interrupted the request on the HTTP thread /// (`release_at_shutdown`): nothing more will arrive, so the next progress @@ -141,7 +142,7 @@ impl http::HTTPClientResultHandler for FetchShared { /// already queued, post a progress update to the JS thread. The terminal /// result also hands the tasklet back: through `handed_back`, which the /// progress update that sees it acts on, or — when request-body drain hops - /// may be queued — as a `HandBackHop` queued after them. + /// may be queued — as a `HandBackHop` queued after them and after that update. fn on_result(&self, mut result: HTTPClientResult<'_>) { let is_done = !result.has_more; let mut state = self.state.lock(); @@ -231,10 +232,10 @@ impl http::HTTPClientResultHandler for FetchShared { } } + self.post_progress_update(); if is_done { self.hand_back(&mut state); } - self.post_progress_update(); drop(state); if is_done { // The HTTP thread is done with this fetch. @@ -244,11 +245,11 @@ impl http::HTTPClientResultHandler for FetchShared { /// Called from `dealloc_in_flight_for_exit` on the HTTP thread for each /// request still in flight when `process.exit()` interrupts it. The - /// terminal `on_result` will never run, so post what it would have: the - /// hand-back, and — unless one is already queued — a last progress update, - /// which `released_at_shutdown` turns into just the release of the JS - /// side's reference. A queued update's VM releases it from its queue if it - /// never runs. + /// terminal `on_result` will never run, so post what it would have: unless + /// one is already queued, a last progress update — which + /// `released_at_shutdown` turns into just the release of the JS side's + /// reference — and then the hand-back. A queued update's VM releases it + /// from its queue if it never runs. /// /// Only reachable for a request whose VM has *not* torn down (a worker /// still running when the main thread exits): a VM's teardown waits for its @@ -260,21 +261,22 @@ impl http::HTTPClientResultHandler for FetchShared { // No JS-thread drain will reclaim it. state.scheduled_response_buffer = MutableString::default(); state.released_at_shutdown = true; + self.post_progress_update(); self.hand_back(&mut state); } - self.post_progress_update(); // The HTTP thread is done with this fetch. self.ticket.hand_back(); } } impl FetchShared { - /// HTTP thread, `state` locked, nothing more to deliver: let the JS thread - /// release `http_ref`. + /// HTTP thread, `state` locked, nothing more to deliver and the last progress + /// update posted: let the JS thread release `http_ref`. fn hand_back(&self, state: &mut SharedState) { if self.posts_drain_hops { - // Its own hop: FIFO after every `RequestBodyDrainHop` this thread - // posted, which is what keeps the tasklet alive for those. + // Its own hop, the last one to name the tasklet: FIFO after every + // `RequestBodyDrainHop` and progress hop this thread posted, which + // is what keeps the tasklet alive for those. let task = self.hop(task_tag::FetchTaskletHandBack); let node = self .hand_back_task @@ -306,14 +308,22 @@ impl TaskHop for ProgressHop { fn run(this: ThisPtr) -> JsResult<()> { FetchTasklet::on_progress_update(this) } - /// The VM is tearing down (its wait for the ticket is over, so the fetch was - /// handed back): release what the update would have. + /// The VM is tearing down: release what the update would have. Consumes the + /// hop like `on_progress_update` does, so a terminal result still on its way + /// posts a fresh one (released here in turn) that carries the hand-back. fn release_unrun(this: ThisPtr) { - let handed_back = this.shared.state.lock().handed_back; + let _guard = RefPtr::from_this(this); + let handed_back = { + let state = this.shared.state.lock(); + this.shared + .has_schedule_callback + .store(false, Ordering::Relaxed); + state.handed_back + }; + FetchTasklet::release(this, |t| &t.progress_ref); if handed_back { FetchTasklet::release(this, |t| &t.http_ref); } - FetchTasklet::release(this, |t| &t.progress_ref); } } From fa0a913c9a94c4a5938b950f538628da032a1ff7 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 27 Aug 2026 08:29:44 +0000 Subject: [PATCH 07/11] VmHandle: InFlightTicket::post goes through the teardown test gate like Ticket::post --- src/jsc/VmHandle.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 22ef0f7e5dd5..11ccd6ae2e44 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -234,7 +234,11 @@ impl Ticket { /// before this returns, so `self` must not live inside that memory: move /// the ticket out of the work's struct first, post, then drop it. pub fn post(&self, task: NonNull) { - test_gate::before_ticket_post(self); + test_gate::before_ticket_post( + &self.shared, + #[cfg(debug_assertions)] + self.id, + ); debug_assert!( self.shared.state() != State::Closed, "ticket post after its VM closed (a ticket was created after the wait)" @@ -322,6 +326,11 @@ impl InFlightTicket { !self.returned.load(Ordering::Relaxed), "post after hand_back" ); + test_gate::before_ticket_post( + &self.shared, + #[cfg(debug_assertions)] + self.id, + ); debug_assert!( self.shared.state() != State::Closed, "ticket post after its VM closed (a ticket was created after the wait)" @@ -606,7 +615,7 @@ pub extern "C" fn Bun__VM__currentLoopKind(vm: &VirtualMachine) -> LoopKind { // under the gate, not in production. #[cfg(debug_assertions)] mod test_gate { - use super::{Ordering, Posted, Shared, State, Ticket, VmHandle}; + use super::{Ordering, Posted, Shared, State, VmHandle}; type Task = core::ptr::NonNull; impl VmHandle { @@ -637,17 +646,10 @@ mod test_gate { let _ = w.flush(); } - pub(super) fn before_ticket_post(t: &Ticket) { - if armed(&t.shared) { - park_until_draining(&t.shared); - let l = *t - .shared - .debug - .live - .lock() - .at - .get(&t.id) - .expect("live ticket"); + pub(super) fn before_ticket_post(shared: &Shared, id: u64) { + if armed(shared) { + park_until_draining(shared); + let l = *shared.debug.live.lock().at.get(&id).expect("live ticket"); say(format_args!( "late completion from {}:{}", l.file(), @@ -684,14 +686,14 @@ mod test_gate { } #[cfg(not(debug_assertions))] mod test_gate { - use super::{Posted, Shared, Ticket, VmHandle}; + use super::{Posted, Shared, VmHandle}; type Task = core::ptr::NonNull; impl VmHandle { #[inline(always)] pub(crate) fn arm_test_gate(&self) {} } #[inline(always)] - pub(super) fn before_ticket_post(_: &Ticket) {} + pub(super) fn before_ticket_post(_: &Shared) {} #[inline(always)] pub(super) fn weak_post(_: &Shared, task: Task, post: impl FnOnce(Task) -> Posted) -> Posted { post(task) From 39882664b56ed10a8ba343ee13a2c06acf3ba462 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 06:57:25 +0000 Subject: [PATCH 08/11] Rebase fixups: AbortSignal::ref_ replaces retain, ResponseRef drops through CellRefCounted::deref_nn, NewSource destroy without destroy_box_with_mut --- src/jsc/AbortSignal.rs | 9 +-------- src/runtime/webcore/ReadableStream.rs | 18 ++++++++++++------ src/runtime/webcore/Response.rs | 4 ++-- src/runtime/webcore/fetch.rs | 1 - 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/jsc/AbortSignal.rs b/src/jsc/AbortSignal.rs index 16684d6a8f65..811433dd3bd0 100644 --- a/src/jsc/AbortSignal.rs +++ b/src/jsc/AbortSignal.rs @@ -125,7 +125,7 @@ impl AbortSignal { C::on_abort(unsafe { bun_ptr::ThisPtr::new(ptr.cast::()) }, reason); } let ctx = listener.this_ptr().as_ptr().cast::(); - let signal = self.retain(); + let signal = self.ref_(); self.pending_activity_ref(); self.add_listener(ctx, callback::); AbortListenerRegistration { signal, ctx } @@ -211,13 +211,6 @@ impl AbortSignal { unsafe { AbortSignalRef::adopt(WebCore__AbortSignal__ref(self)) } } - /// Take a counted reference on this signal. - pub fn retain(&self) -> AbortSignalRef { - // SAFETY: `&AbortSignal` only exists for a live C++ `WebCore::AbortSignal` - // (opaque FFI handle); `ref_()` returns it with the count bumped. - unsafe { AbortSignalRef::adopt(self.ref_()) } - } - pub fn unref(&self) { WebCore__AbortSignal__unref(self) } diff --git a/src/runtime/webcore/ReadableStream.rs b/src/runtime/webcore/ReadableStream.rs index 0c3844db2867..46d809e27dcd 100644 --- a/src/runtime/webcore/ReadableStream.rs +++ b/src/runtime/webcore/ReadableStream.rs @@ -851,6 +851,7 @@ pub trait SourceContext: Sized { // adjacent fields as the loader, returning empty bodies. #[repr(C)] #[derive(bun_ptr::CellRefCounted)] +#[ref_count(destroy = Self::destroy)] pub struct NewSource { pub context: C, pub cancelled: bool, @@ -1079,12 +1080,6 @@ const _: () = assert!(core::mem::offset_of!(NewSource, context) const _: () = assert!(core::mem::offset_of!(NewSource, context) == 0); const _: () = assert!(core::mem::offset_of!(NewSource, context) == 0); -impl Drop for NewSource { - fn drop(&mut self) { - self.context.deinit_fn(); - } -} - impl NewSource { /// Point the `owner` slot at the GC cell of the peer producing into this /// source (its `producer` backref), so rooting the source roots the @@ -1269,6 +1264,17 @@ impl NewSource { rc - 1 } + /// `CellRefCounted` destroy: context teardown, then free. + /// + /// # Safety + /// Only for the `#[ref_count(destroy = …)]` derive: `this` is the sole + /// live owner of the `Box` from [`Self::new`]. + unsafe fn destroy(this: *mut Self) { + // SAFETY: fn contract. + let mut source = unsafe { Box::from_raw(this) }; + source.context.deinit_fn(); + } + /// Release one reference. If the count hits zero, runs context teardown and /// **frees the allocation**. /// diff --git a/src/runtime/webcore/Response.rs b/src/runtime/webcore/Response.rs index 074b50b7c414..e25bed4a7542 100644 --- a/src/runtime/webcore/Response.rs +++ b/src/runtime/webcore/Response.rs @@ -69,7 +69,7 @@ impl core::ops::Deref for ResponseRef { impl Drop for ResponseRef { fn drop(&mut self) { - Response::unref(self.0.as_const_ptr().cast_mut()); + ::deref_nn(self.0.into()); } } @@ -77,7 +77,7 @@ impl bun_jsc::NativeAbortListener for BodyAbortListener { fn on_abort(this: bun_ptr::ThisPtr, reason: JSValue) { reason.ensure_still_alive(); // Copy out up front: erroring a still-streaming body can re-enter - // `Response::unref` via `FetchTasklet::abandon_response_body` + // the last `Response` deref via `FetchTasklet::abandon_response_body` // and destroy this listener. let (response, global) = (this.response, this.global); let _keepalive = ResponseRef::retain(response); diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index 357c91b0f589..4109d3eb0100 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -64,7 +64,6 @@ use crate::webcore::{ use crate::webcore::{blob, readable_stream, response}; use bun_http_jsc as _; use bun_http_jsc::headers_jsc::from_fetch_headers; -use bun_jsc::AbortSignalRef; #[cfg(windows)] use bun_paths::resolve_path::PosixToWinNormalizer; use bun_picohttp as picohttp; From 0a9a1ece10c14a8621ca628c87e78ee17899b3e4 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 07:41:52 +0000 Subject: [PATCH 09/11] HeadersRef is the only FetchHeaders release; SourceHandle::can_start_streaming; FetchRequestBodySink::end_from_stream closes a copied-out handle; Bun.serve upgrade/static-route header conversions hold a HeadersRef --- src/jsc/FetchHeaders.rs | 11 ++--- src/runtime/server/server_body.rs | 46 +++++-------------- src/runtime/webcore/Blob.rs | 5 +- .../webcore/fetch/FetchRequestBodySink.rs | 3 +- src/runtime/webcore/streams.rs | 6 +++ 5 files changed, 24 insertions(+), 47 deletions(-) diff --git a/src/jsc/FetchHeaders.rs b/src/jsc/FetchHeaders.rs index 152d1bdd2f89..7d4054f6795d 100644 --- a/src/jsc/FetchHeaders.rs +++ b/src/jsc/FetchHeaders.rs @@ -293,10 +293,6 @@ impl FetchHeaders { }) } - pub fn deref(&mut self) { - WebCore__FetchHeaders__deref(self) - } - pub fn copy_to(&mut self, names: *mut StringPointer, values: *mut StringPointer, buf: *mut u8) { // SAFETY: caller guarantees names/values/buf are sized per a prior `count()` call unsafe { WebCore__FetchHeaders__copyTo(self, names, values, buf) } @@ -397,9 +393,8 @@ impl core::ops::DerefMut for HeadersRef { impl Drop for HeadersRef { #[inline] fn drop(&mut self) { - // `self.0` is live; releasing our +1 ref via WebCore__FetchHeaders__deref. - // Explicit UFCS to avoid `core::ops::Deref::deref` resolution ambiguity. - // `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref. - FetchHeaders::deref(bun_opaque::opaque_deref_mut(self.0.as_ptr())); + // `FetchHeaders` is an opaque ZST FFI handle (S008); `self.0` is live and + // this is the release of our +1. + WebCore__FetchHeaders__deref(bun_opaque::opaque_deref_mut(self.0.as_ptr())); } } diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 037be482622a..d3e6bd095733 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -632,16 +632,8 @@ impl AnyRoute { let Some(headers_js) = argument.get(init_ctx.global, b"headers")? else { return Ok(None); }; - let fetch_headers = FetchHeaders::create_from_js(init_ctx.global, headers_js)?; - let _fh_guard = scopeguard::guard(fetch_headers, |fh| { - // S008: `FetchHeaders` is an `opaque_ffi!` ZST — safe deref. - if let Some(h) = fh { - bun_opaque::opaque_deref_mut(h.as_ptr()).deref(); - } - }); - - // S008: `FetchHeaders` is an `opaque_ffi!` ZST — safe deref. - let headers_ref = fetch_headers.map(|p| bun_opaque::opaque_deref(p.as_ptr().cast_const())); + let fetch_headers = HeadersRef::create_from_js(init_ctx.global, headers_js)?; + let headers_ref = fetch_headers.as_deref(); let route = Self::from_options(init_ctx.global, headers_ref, &mut path)?; if is_index_route { @@ -1687,15 +1679,8 @@ where let mut data_value = JSValue::ZERO; - // if we converted a HeadersInit to a Headers object, we need to free it - let fetch_headers_to_deref: core::cell::Cell> = - core::cell::Cell::new(None); - let _fh_guard = scopeguard::guard(&fetch_headers_to_deref, |cell| { - if let Some(fh) = cell.get() { - // S008: `FetchHeaders` is an `opaque_ffi!` ZST — safe deref. - bun_opaque::opaque_deref_mut(fh).deref(); - } - }); + // Holds a Headers object converted from a HeadersInit until this returns. + let mut created_headers: Option = None; // Copied out of `options.headers` because `fast_remove` frees the // entry they would otherwise borrow. @@ -1729,11 +1714,11 @@ where None => 'brk: { if headers_value.is_object() { if let Some(fetch_headers) = - FetchHeaders::create_from_js(global, headers_value)? + HeadersRef::create_from_js(global, headers_value)? { - fetch_headers_to_deref - .set(Some(fetch_headers.as_ptr())); - break 'brk fetch_headers.as_ptr(); + break 'brk created_headers + .insert(fetch_headers) + .as_ptr(); } } return Err(global.throw_invalid_arguments(format_args!( @@ -1905,14 +1890,8 @@ where return Ok(JSValue::FALSE); } let mut data_value = JSValue::ZERO; - // Non-unit guard state: holds the temporarily-created FetchHeaders (if - // any) and derefs it on scope exit. Populated below via DerefMut. - let mut fetch_headers_to_deref = scopeguard::guard(None::<*mut FetchHeaders>, |fh| { - // S008: `FetchHeaders` is an `opaque_ffi!` ZST — safe deref. - if let Some(h) = fh { - bun_opaque::opaque_deref_mut(h).deref() - } - }); + // Holds a Headers object converted from a HeadersInit until this returns. + let mut created_headers: Option = None; let mut fetch_headers_to_use: Option<*mut FetchHeaders> = None; if let Some(opts) = optional { @@ -1939,10 +1918,9 @@ where None => 'brk: { if headers_value.is_object() { if let Some(created) = - FetchHeaders::create_from_js(global, headers_value)? + HeadersRef::create_from_js(global, headers_value)? { - *fetch_headers_to_deref = Some(created.as_ptr()); - break 'brk created.as_ptr(); + break 'brk created_headers.insert(created).as_ptr(); } } return Err(global.throw_invalid_arguments(format_args!( diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index cda6916cf570..a00ce1e639a6 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -5004,10 +5004,7 @@ pub(crate) fn write_file_internal( }; locked.readable.has() || locked.on_start_streaming.is_some() - || matches!( - locked.producer, - crate::webcore::streams::SourceHandle::FetchResponseBody(_) - ) + || locked.producer.can_start_streaming() }; if streamable { // SAFETY: exclusive borrow scoped to the call (may run JS). diff --git a/src/runtime/webcore/fetch/FetchRequestBodySink.rs b/src/runtime/webcore/fetch/FetchRequestBodySink.rs index 5f7112a095a8..fe0411907cc7 100644 --- a/src/runtime/webcore/fetch/FetchRequestBodySink.rs +++ b/src/runtime/webcore/fetch/FetchRequestBodySink.rs @@ -228,7 +228,8 @@ impl FetchRequestBodySink { Some(StreamError::Error(e)) => Some(e), _ => None, }; - self.source.get_mut().close(sys_err); + let mut source = self.source.get(); + source.close(sys_err); } pub fn end_from_js(&mut self, _global_this: &JSGlobalObject) -> bun_sys::Result { diff --git a/src/runtime/webcore/streams.rs b/src/runtime/webcore/streams.rs index 3f7a9c937c48..98bf7a1295f1 100644 --- a/src/runtime/webcore/streams.rs +++ b/src/runtime/webcore/streams.rs @@ -986,6 +986,12 @@ impl SourceHandle { } } + /// Whether [`start_streaming`](Self::start_streaming) would hand over a + /// buffer, i.e. the body can be realised as a stream right now. + pub fn can_start_streaming(&self) -> bool { + matches!(self, SourceHandle::FetchResponseBody(_)) + } + /// `Body::PendingValue` producer hook: the body is being realised as a /// `ByteStream`; hand over whatever is already buffered. pub fn start_streaming(&self) -> Option { From dc8136558de8c77e2afaeb1036f25ab825bf22e5 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 07:59:26 +0000 Subject: [PATCH 10/11] server.fetch(url, { headers: Headers }) copies the Headers like new Request() does instead of adopting the wrapper's reference --- src/runtime/server/server_body.rs | 25 +++++-------- test/js/bun/http/bun-server.test.ts | 54 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index d3e6bd095733..e05d10cf6897 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2289,22 +2289,15 @@ where if let Some(headers_) = opts.fast_get(ctx, jsc::BuiltinName::Headers)? { if let Some(headers__) = FetchHeaders::cast_(headers_, ctx.vm()) { - // NOTE: `cast_` returns the `FetchHeaders*` held by the - // JS `Headers` wrapper (`JSFetchHeaders`'s internal - // `Ref`) without bumping the refcount — - // the FFI surface has `WebCore__FetchHeaders__deref` but - // no `ref()`, so a +1 cannot be taken here. Adopting - // hands that wrapper-held ref to the constructed - // `Request` (via `Request::init2` below): the eventual - // single deref happens when the Request's finalizer - // drops its `headers` field (`HeadersRef::Drop`, - // Response.rs), pairing with the wrapper's +1. - // SAFETY: `headers__` is live (rooted by `headers_`), - // and ownership of one ref transfers as described above. - headers = Some(unsafe { HeadersRef::adopt(headers__) }); - } else if let Some(headers__) = FetchHeaders::create_from_js(ctx, headers_)? { - // SAFETY: create_from_js returns a +1 ref. - headers = Some(unsafe { HeadersRef::adopt(headers__) }); + // The JS `Headers` keeps its own reference; the Request + // gets a copy, as `new Request(url, { headers })` does. + // S008: `FetchHeaders` is an opaque ZST FFI handle — safe deref. + headers = bun_opaque::opaque_deref_mut(headers__.as_ptr()) + .clone_this(ctx)? + // SAFETY: `clone_this` returns a +1 ref. + .map(|p| unsafe { HeadersRef::adopt(p) }); + } else { + headers = HeadersRef::create_from_js(ctx, headers_)?; } } diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 1c8845085a89..cf76be501daf 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -352,6 +352,60 @@ describe.concurrent("Server", () => { } }); + test("server.fetch(url, { headers: Headers }) copies the headers; the Headers object stays usable across GC", async () => { + using server = Bun.serve({ + port: 0, + fetch(req) { + return new Response(req.headers.get("x-i") ?? "none"); + }, + }); + const url = `http://${server.hostname}:${server.port}/`; + const kept: Headers[] = []; + for (let i = 0; i < 200; i++) { + const headers = new Headers({ "x-i": String(i) }); + const response = await server.fetch(url, { headers }); + expect(await response.text()).toBe(String(i)); + if (i % 2 === 0) kept.push(headers); + if (i % 50 === 0) Bun.gc(true); + } + Bun.gc(true); + // The Request built by server.fetch() has been collected; the Headers we + // kept must still own their list. + expect(kept.map(h => h.get("x-i"))).toEqual(kept.map((_, i) => String(i * 2))); + }); + + test("server.upgrade(req, { headers }) with a Headers object and with a plain object, across GC", async () => { + for (const makeHeaders of [() => new Headers({ "x-up": "1" }), () => ({ "x-up": "1" })]) { + let upgraded = 0; + using server = Bun.serve({ + port: 0, + fetch(req, srv) { + if (srv.upgrade(req, { headers: makeHeaders() })) { + upgraded++; + return; + } + return new Response("no upgrade", { status: 400 }); + }, + websocket: { + open(ws) { + ws.close(); + }, + message() {}, + }, + }); + for (let i = 0; i < 100; i++) { + const ws = new WebSocket(`ws://${server.hostname}:${server.port}/`); + const { promise, resolve } = Promise.withResolvers(); + ws.onclose = () => resolve(); + ws.onerror = () => resolve(); + await promise; + if (i % 25 === 0) Bun.gc(true); + } + Bun.gc(true); + expect(upgraded).toBe(100); + } + }); + test("server should return a body for a OPTIONS Request", async () => { using server = Bun.serve({ port: 0, From 95995c521ff6243c13499940ee5b22c0f2d9a0fc Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 29 Aug 2026 08:28:06 +0000 Subject: [PATCH 11/11] test: server.upgrade headers variants via test.each --- test/js/bun/http/bun-server.test.ts | 57 +++++++++++++++-------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index cf76be501daf..3d321951f070 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -374,36 +374,37 @@ describe.concurrent("Server", () => { expect(kept.map(h => h.get("x-i"))).toEqual(kept.map((_, i) => String(i * 2))); }); - test("server.upgrade(req, { headers }) with a Headers object and with a plain object, across GC", async () => { - for (const makeHeaders of [() => new Headers({ "x-up": "1" }), () => ({ "x-up": "1" })]) { - let upgraded = 0; - using server = Bun.serve({ - port: 0, - fetch(req, srv) { - if (srv.upgrade(req, { headers: makeHeaders() })) { - upgraded++; - return; - } - return new Response("no upgrade", { status: 400 }); - }, - websocket: { - open(ws) { - ws.close(); - }, - message() {}, + test.each([ + ["Headers object", () => new Headers({ "x-up": "1" })], + ["plain object", () => ({ "x-up": "1" })], + ] as const)("server.upgrade(req, { headers }) with a %s, across GC", async (_, makeHeaders) => { + let upgraded = 0; + using server = Bun.serve({ + port: 0, + fetch(req, srv) { + if (srv.upgrade(req, { headers: makeHeaders() })) { + upgraded++; + return; + } + return new Response("no upgrade", { status: 400 }); + }, + websocket: { + open(ws) { + ws.close(); }, - }); - for (let i = 0; i < 100; i++) { - const ws = new WebSocket(`ws://${server.hostname}:${server.port}/`); - const { promise, resolve } = Promise.withResolvers(); - ws.onclose = () => resolve(); - ws.onerror = () => resolve(); - await promise; - if (i % 25 === 0) Bun.gc(true); - } - Bun.gc(true); - expect(upgraded).toBe(100); + message() {}, + }, + }); + for (let i = 0; i < 100; i++) { + const ws = new WebSocket(`ws://${server.hostname}:${server.port}/`); + const { promise, resolve } = Promise.withResolvers(); + ws.onclose = () => resolve(); + ws.onerror = () => resolve(); + await promise; + if (i % 25 === 0) Bun.gc(true); } + Bun.gc(true); + expect(upgraded).toBe(100); }); test("server should return a body for a OPTIONS Request", async () => {