From d81153143bf9a4ee6925e0048a663f1f84da1663 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 01:24:44 +0000 Subject: [PATCH 1/6] fetch: keep PathBuffer scratch out of the fetch_impl stack frame --- src/runtime/node/node_fs.rs | 11 +++++++++++ src/runtime/webcore/fetch.rs | 21 ++++++++------------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 466f65ab71b5..1f0d9fd1561a 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4432,6 +4432,17 @@ impl Default for NodeFS { } } +impl NodeFS { + /// A fresh `NodeFS` on the heap. `sync_error_buf` makes the struct + /// `MAX_PATH_BYTES` large (96 KB on Windows), so a stack local of it costs + /// that much frame in every caller, taken or not. + pub(crate) fn new_boxed() -> Box { + // SAFETY: all-zero bytes are a valid `NodeFS`: `sync_error_buf` is a + // `u8` array and a null `Option>` is `None`. + unsafe { Box::::new_zeroed().assume_init() } + } +} + /// Encode a path returned by the OS (`mkdtemp`/`readlink`/`realpath`) using the /// caller's `encoding` option, matching Node.js: `"buffer"` yields a `Buffer` /// of the raw bytes, any other encoding is `Buffer.from(bytes).toString(enc)`. diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index f17986040e48..9dfed124d0a1 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -47,7 +47,6 @@ 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_paths::{self, PathBuffer}; 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::`. @@ -1267,8 +1266,8 @@ fn fetch_impl( // We don't pass along headers, we ignore method, we ignore status code... // But it's better than status quo. if url_type != URLType::Remote { - let mut path_buf = PathBuffer::uninit(); - let mut path_buf2 = PathBuffer::uninit(); + let mut path_buf = bun_paths::path_buffer_pool::get(); + let mut path_buf2 = bun_paths::path_buffer_pool::get(); let decoded_len = match PercentEncoding::decode_into( &mut path_buf2[..], match url_type { @@ -1347,11 +1346,11 @@ fn fetch_impl( } #[cfg(windows)] - let mut cwd_buf = PathBuffer::uninit(); + let mut cwd_buf = bun_paths::path_buffer_pool::get(); #[cfg(windows)] // `bun_sys::getcwd` returns the byte length written into // `cwd_buf`; slice it here. - let cwd: &[u8] = match bun_sys::getcwd(&mut cwd_buf) { + let cwd: &[u8] = match bun_sys::getcwd(&mut cwd_buf[..]) { Ok(len) => &cwd_buf[..len], Err(err) => { return Err(global_this.throw_error(err, "Failed to resolve file url")); @@ -1517,10 +1516,7 @@ fn fetch_impl( } if body.needs_to_read_file() { 'prepare_body: { - // A local `PathBuffer` serves as NUL-termination scratch for - // `path.slice_z()` (the `vm.node_fs()` accessor is gated behind a - // jsc↔runtime cycle). - let mut open_path_buf = PathBuffer::uninit(); + let mut open_path_buf = bun_paths::path_buffer_pool::get(); let opened_fd_res: bun_sys::Result = { let store = body.store().expect("needs_to_read_file implies store"); match &store.data.as_file().pathlike { @@ -1617,10 +1613,9 @@ fn fetch_impl( // TODO: make this async + lazy let blob_offset = body.any_blob().blob().offset.get(); let blob_size = body.any_blob().blob().size.get(); - // The `vm.node_fs()` accessor is a jsc↔runtime cycle. `read_file` - // with an `Fd` path only touches `self.sync_error_buf` for - // path-variant inputs, so a fresh `NodeFS` is sufficient here. - let mut node_fs = node::fs::NodeFS::default(); + // `read_file` with an `Fd` path only touches `self.sync_error_buf` + // for path-variant inputs, so a fresh `NodeFS` is sufficient here. + let mut node_fs = node::fs::NodeFS::new_boxed(); // `ReadFile` has `Drop`; can't use FRU `..Default::default()`. let mut rf_args = node::fs::args::ReadFile::default(); rf_args.encoding = Encoding::Buffer; From daa2537c83ca03a6fae350e165a0d42f4fbd94ce Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 01:32:43 +0000 Subject: [PATCH 2/6] test: fetch() recursion through a body asyncIterator getter reaches the stack limit late --- test/js/web/fetch/body-async-iterator.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/js/web/fetch/body-async-iterator.test.ts b/test/js/web/fetch/body-async-iterator.test.ts index 981ff04a4f96..6d0fe323b6b6 100644 --- a/test/js/web/fetch/body-async-iterator.test.ts +++ b/test/js/web/fetch/body-async-iterator.test.ts @@ -27,6 +27,41 @@ test("Response.bytes() with async iterable body does not crash with null deref", expect(exitCode).toBe(0); }); +// Each level of this recursion holds one native fetch() frame on the stack. +// Windows used to reach 16 levels before the RangeError because that frame +// carried three 96 KB path buffers (Linux: 387 levels with the same script). +// A debug build of the fix reaches 96 on Windows; stock 1.4.1 reaches 16. +test("fetch() called from a body's Symbol.asyncIterator getter recurses deeply before the stack limit", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + let depth = 0; + const signal = AbortSignal.abort(); + const body = { + get [Symbol.asyncIterator]() { + depth++; + fetch("http://localhost/", { method: "POST", body, signal }).catch(() => {}); + return undefined; + }, + }; + fetch("http://localhost/", { method: "POST", body, signal }).catch(() => {}); + console.log(depth); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(Number(stdout.trim())).toBeGreaterThanOrEqual(32); + expect(exitCode).toBe(0); +}); + test("Response.arrayBuffer() with async iterable body does not crash with null deref", async () => { await using proc = Bun.spawn({ cmd: [ From b05158390206b99fd8ea40a0f6f004a72561ec51 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:22:58 +0000 Subject: [PATCH 3/6] ci: retrigger From d699f46e08b2fc21875a60a506a3cbfd842adc67 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:24:48 +0000 Subject: [PATCH 4/6] fetch: shorten the NodeFS comments --- src/runtime/node/node_fs.rs | 4 +--- src/runtime/webcore/fetch.rs | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 1f0d9fd1561a..8e6558ea3545 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4433,9 +4433,7 @@ impl Default for NodeFS { } impl NodeFS { - /// A fresh `NodeFS` on the heap. `sync_error_buf` makes the struct - /// `MAX_PATH_BYTES` large (96 KB on Windows), so a stack local of it costs - /// that much frame in every caller, taken or not. + /// Heap allocated: `sync_error_buf` makes this struct 96 KB on Windows. pub(crate) fn new_boxed() -> Box { // SAFETY: all-zero bytes are a valid `NodeFS`: `sync_error_buf` is a // `u8` array and a null `Option>` is `None`. diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index 9dfed124d0a1..ef418b5e291b 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -1613,8 +1613,7 @@ fn fetch_impl( // TODO: make this async + lazy let blob_offset = body.any_blob().blob().offset.get(); let blob_size = body.any_blob().blob().size.get(); - // `read_file` with an `Fd` path only touches `self.sync_error_buf` - // for path-variant inputs, so a fresh `NodeFS` is sufficient here. + // A fresh `NodeFS` suffices: `read_file` on an `Fd` never touches `sync_error_buf`. let mut node_fs = node::fs::NodeFS::new_boxed(); // `ReadFile` has `Drop`; can't use FRU `..Default::default()`. let mut rf_args = node::fs::args::ReadFile::default(); From f63c3747f0f47d110318a8073ca7223bac93aaf3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:16:41 +0000 Subject: [PATCH 5/6] node:fs: store NodeFS.sync_error_buf as a pooled path buffer NodeFS embedded a PathBuffer by value, which made every NodeFS::default() stack local 96 KB on Windows. The buffer now comes from bun_paths::path_buffer_pool, so the struct is two words and the NodeFS::new_boxed() workaround is not needed. The #[repr(C)] alignment contract goes with it: the u16 reinterpretation relies on the heap allocation's alignment, which the existing assert checks. --- src/runtime/jsc_hooks.rs | 2 +- src/runtime/node/node_fs.rs | 33 +++++++-------------------------- src/runtime/webcore/fetch.rs | 2 +- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 02769d6675d7..0ea2bde97c7f 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1331,7 +1331,7 @@ unsafe fn create_node_fs(vm: *mut VirtualMachine) -> *mut c_void { None }; bun_core::heap::into_raw(Box::new(NodeFS { - sync_error_buf: bun_paths::PathBuffer::uninit(), + sync_error_buf: bun_paths::path_buffer_pool::get(), vm: vm_field, })) .cast::() diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 8e6558ea3545..fa73a1e954fe 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4406,41 +4406,22 @@ pub mod ret { // https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/fs.d.ts // ────────────────────────────────────────────────────────────────────────── -// `#[repr(C)]` pins `sync_error_buf` (a `[u8; N]`, nominal align = 1) at -// offset 0. The struct's overall alignment is ≥ `align_of::<*const ()>()` -// (from the `vm` field), so the buffer's address inherits that alignment. -// This is load-bearing on Windows where `sync_error_buf` is reinterpreted as -// `&mut [u16]` / `&mut WPathBuffer` (see `mkdir_recursive_os_path_impl` and -// the `os_path_kernel32` callers); a misaligned `&mut [u16]` is instant UB. -#[repr(C)] pub struct NodeFS { - /// Buffer to store a temporary file path that might appear in a returned error message. - /// - /// We want to avoid allocating a new path buffer for every error message so that jsc can clone + GC it. - /// That means a stack-allocated buffer won't suffice. Instead, we re-use - /// the heap allocated buffer on the NodeFS struct - pub(crate) sync_error_buf: PathBuffer, // must be align_of::()-aligned — enforced via #[repr(C)] + field order, see above + /// Scratch for a temporary file path that might appear in a returned error message. + /// Pooled on the heap: a `PathBuffer` is 96 KB on Windows and a `NodeFS` is often a stack local. + pub(crate) sync_error_buf: paths::path_buffer_pool::Guard, pub(crate) vm: Option>, } impl Default for NodeFS { fn default() -> Self { Self { - sync_error_buf: PathBuffer::uninit(), + sync_error_buf: paths::path_buffer_pool::get(), vm: None, } } } -impl NodeFS { - /// Heap allocated: `sync_error_buf` makes this struct 96 KB on Windows. - pub(crate) fn new_boxed() -> Box { - // SAFETY: all-zero bytes are a valid `NodeFS`: `sync_error_buf` is a - // `u8` array and a null `Option>` is `None`. - unsafe { Box::::new_zeroed().assume_init() } - } -} - /// Encode a path returned by the OS (`mkdtemp`/`readlink`/`realpath`) using the /// caller's `encoding` option, matching Node.js: `"buffer"` yields a `Buffer` /// of the raw bytes, any other encoding is `Buffer.from(bytes).toString(enc)`. @@ -5550,8 +5531,8 @@ impl NodeFS { } } - // SAFETY: `NodeFS` is `#[repr(C)]` with `sync_error_buf` at offset 0 and - // struct alignment ≥ pointer-align (from `vm`), so this address is + // SAFETY: `sync_error_buf` is a pooled heap allocation, and every + // supported allocator aligns it to at least 8 bytes, so this address is // ≥ `align_of::()`-aligned. On Windows // `OSPathBuffer = [u16; PATH_MAX_WIDE]` (65 534 B) which fits inside // `PathBuffer` (`MAX_PATH_BYTES` = 98 302 B); on POSIX it is the same @@ -5560,7 +5541,7 @@ impl NodeFS { // `&mut PathBuffer` without reborrowing `&mut self` (which would alias // `working_mem` under stacked borrows). On every such path `working_mem` is // not used afterward, so the re-derive is sound. - let sync_error_buf_ptr: *mut PathBuffer = &raw mut self.sync_error_buf; + let sync_error_buf_ptr: *mut PathBuffer = &raw mut *self.sync_error_buf; assert!( sync_error_buf_ptr.cast::().is_aligned(), "NodeFS.sync_error_buf misaligned for OSPathChar", diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index ef418b5e291b..c0c8cce1fc71 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -1614,7 +1614,7 @@ fn fetch_impl( let blob_offset = body.any_blob().blob().offset.get(); let blob_size = body.any_blob().blob().size.get(); // A fresh `NodeFS` suffices: `read_file` on an `Fd` never touches `sync_error_buf`. - let mut node_fs = node::fs::NodeFS::new_boxed(); + let mut node_fs = node::fs::NodeFS::default(); // `ReadFile` has `Drop`; can't use FRU `..Default::default()`. let mut rf_args = node::fs::args::ReadFile::default(); rf_args.encoding = Encoding::Buffer; From 74db05dad71e4fe678e6631efb700e34c92a5eb3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:17:51 +0000 Subject: [PATCH 6/6] node:fs: one-line doc comment on sync_error_buf --- src/runtime/node/node_fs.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index fa73a1e954fe..e4a25d948d89 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -4408,7 +4408,6 @@ pub mod ret { pub struct NodeFS { /// Scratch for a temporary file path that might appear in a returned error message. - /// Pooled on the heap: a `PathBuffer` is 96 KB on Windows and a `NodeFS` is often a stack local. pub(crate) sync_error_buf: paths::path_buffer_pool::Guard, pub(crate) vm: Option>, }