Repository navigation
Conversation
|
Status: fix and test pushed. The PR now also stores CI on the previous head was green on 180 of 181 jobs. The one red job was Reproduction (Windows x64, stock 1.4.1 and 1.4.1-canary): 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); // windows: 16, linux: 387
The test cannot fail on Linux before the fix: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review. WalkthroughChangesFetch filesystem safety
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change moves fetch scratch buffers off the native stack while preserving fetch behavior, with targeted tests and probes covering the affected paths; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the problem, fix, rationale, regression test, verification results, and known pre-existing failures. It provides the information required by the repository template, although it uses Problem and Fix headings instead of the exact template headings. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — mechanical swap to the documented pooled-buffer pattern.
What was reviewed:
PoolGuard<PathBuffer>derefs to the samePathBuffertype; all four call sites (decode_into,resolve_cwd_with_external_buf_z,join_abs_string_buf,getcwd,slice_z) still receive the same argument type via deref coercion, and every buffer is write-then-slice so stale pooled bytes are never read.- Verified the
new_boxedSAFETY claim against theNodeFSstruct: it is exactlyPathBuffer(u8 array) +Option<NonNull<_>>, so all-zero is bit-identical toDefault::default(). - Pool re-entrancy:
borrow_mut()is scoped to thepop(), and cache misses fall through tonew_boxed, so taking 3 guards at once (Windows path) and recursing intofetch_implwhile guards are live is fine. - Test uses a pre-aborted signal so no socket opens; threshold 32 sits between the unfixed Windows count (16) and the fixed debug counts (71 Linux / 96 Windows).
Extended reasoning...
Overview
The PR shrinks the native stack frame of fetch_impl by moving five PathBuffer-sized locals off the stack. Four PathBuffer::uninit() locals in src/runtime/webcore/fetch.rs become bun_paths::path_buffer_pool::get() guards, and the stack-local NodeFS::default() becomes a boxed NodeFS::new_boxed() added in src/runtime/node/node_fs.rs. A regression test in test/js/web/fetch/body-async-iterator.test.ts measures re-entrant fetch() depth via a Symbol.asyncIterator getter and asserts it reaches ≥ 32 (was 16 on Windows).
Security risks
None. No auth, crypto, permission, or input-validation logic is touched. The one new unsafe block (Box::<NodeFS>::new_zeroed().assume_init()) is sound: I checked the NodeFS struct definition — its only fields are a [u8; N]-backed PathBuffer and an Option<NonNull<VirtualMachine>>, both of which are valid at all-zero (the latter via the guaranteed null-pointer niche = None), matching the existing Default impl exactly. This is the same idiom already used in path_buffer_pool.rs itself. The test never opens a socket because the request carries a pre-aborted AbortSignal, so the recursion happens entirely in body extraction.
Level of scrutiny
Low-to-moderate. This is the textbook application of an in-tree pattern whose module doc (src/paths/path_buffer_pool.rs) names this exact problem, and src/CLAUDE.md prescribes it. PoolGuard<PathBuffer> implements Deref/DerefMut to PathBuffer, so every downstream consumer (resolve_cwd_with_external_buf_z takes &mut PathBuffer, getcwd takes &mut [u8], indexing/slicing) sees the identical type it saw before via coercion. The pool's RefCell::borrow_mut() is scoped to a single pop(), so holding three guards simultaneously and recursing into fetch_impl while they're live cannot deadlock or double-borrow; excess concurrent gets just heap-allocate. The buffers are not zeroed on reuse, but neither were the originals (PathBuffer::uninit()), and every use site writes-then-slices-by-returned-length. RAII via Drop on PoolGuard returns buffers on all early-return paths.
Other factors
No CODEOWNERS cover the changed files. No outstanding reviewer objections in the timeline. The test follows harness conventions (bunExe/bunEnv, await using, concurrent pipe drain, stderr/stdout asserted before exit code, appended to the existing module test file). The 32-level threshold is 2× the unfixed Windows number and well under the fixed debug numbers on both platforms, so it should not flake. The PR description is unusually thorough with concrete measurements, and the diff matches the claims.
|
Updated 12:09 AM PT - Aug 28th, 2026
✅ @robobun, your commit 74db05dad71e4fe678e6631efb700e34c92a5eb3 passed in 🧪 To try this PR locally: bunx bun-pr 40680That installs a local version of the PR into your bun-40680 --bun |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — mechanical swap to the documented pooled-buffer pattern, with a sound new_boxed() helper.
What was reviewed:
PoolGuard<PathBuffer>derefs toPathBuffer, so every call site (decode_into,resolve_cwd_with_external_buf_z,join_abs_string_buf,getcwd,slice_z) receives the same type as before; guards drop at scope exit on all early-return paths.NodeFS::new_boxed()SAFETY: the struct is exactly[u8; N]+Option<NonNull<_>>, both valid at all-zeroes; mirrors the existingPoolStorage::new_boxedpattern inpath_buffer_pool.rs.- Test: pre-aborted signal keeps recursion inside body extraction (no sockets), pipes drained concurrently, stderr/stdout asserted before exit code, threshold (32) sits between the unfixed Windows floor (16) and the fixed debug figure (96).
Extended reasoning...
Overview
The PR moves four stack-local PathBuffer::uninit() scratch buffers in fetch_impl (src/runtime/webcore/fetch.rs) onto the heap via bun_paths::path_buffer_pool::get(), and replaces a stack NodeFS::default() with a new NodeFS::new_boxed() helper in src/runtime/node/node_fs.rs. On Windows a PathBuffer is ~96 KB, so this shrinks the fetch_impl frame from ~297 KB to a size that lets recursive fetch() (via a body's Symbol.asyncIterator getter) go from 16 to ~96 levels before JSC's stack limit fires. A regression test in test/js/web/fetch/body-async-iterator.test.ts asserts at least 32 levels are reached.
Security risks
None. No new input parsing, no auth/crypto, no data exposure. The only unsafe addition is Box::<NodeFS>::new_zeroed().assume_init(); I checked the struct definition (line 4416-4424) — its two fields are a u8 array and Option<NonNull<VirtualMachine>>, both of which are valid when zero-filled (the latter via the null-pointer niche → None). This is the same pattern already used for PathBuffer/WPathBuffer in path_buffer_pool.rs:66.
Level of scrutiny
Low-to-moderate. This is the exact transformation src/CLAUDE.md prescribes ("prefer bun_paths::path_buffer_pool::get() over stack PathBuffer to avoid ~64 KB Windows stack frames"). PoolGuard implements Deref/DerefMut<Target = PathBuffer> and returns to the pool on Drop, so every borrow site type-checks against the same &mut PathBuffer/&mut [u8] as before, and every early return in the touched blocks releases the buffer via RAII. The getcwd call now passes &mut cwd_buf[..] explicitly, which is equivalent. The removed bun_paths::{self, PathBuffer} import is genuinely dead after the change.
Other factors
The test follows harness conventions: bunExe()/bunEnv, -e script, await using, Promise.all on stdout/stderr/exited, stderr and stdout asserted before exit code, no sleeps or hardcoded ports. The pre-aborted AbortSignal means no sockets open, so the test is hermetic. The bug hunt exited on dry_streak with zero findings. No CODEOWNERS entries cover the changed paths. Commits since the earlier COMMENTED review only shortened a comment and retriggered CI — no behavioral drift to re-examine. There are no outstanding CHANGES_REQUESTED reviews; the two github-actions inline threads were followed by the comment-shortening commit.
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.
robobun
changed the title
…nc_error_buf The pool stays in `bun_paths`, which `cargo miri test` covers, so its new unit tests run under Miri. The one `bun_core` caller (`spawn_sync`, a cold linux/freebsd path) uses a 4 KB `ZEROED` stack buffer instead. `impl Default for PathBuffer` and `WPathBuffer` are gone. A zero-fill of a 98 KB buffer on Windows is now written as `ZEROED` at each site, and no `#[derive(Default)]` can pick it up by accident. The crash handler uses `ZEROED` explicitly. `NodeFS::sync_error_buf` holds a pool guard. `NodeFS::default()` is a stack local at 15 call sites, so a `ZEROED` field would have been a 98 KB memset per call on Windows. This is the same change as #40680. `#[repr(C)]` goes with it: the `u16` reinterpretation relies on the heap allocation's alignment, which the existing `assert!` checks.
…k arrays (#41442) ### Problem - `PathBuffer::uninit()` and `WPathBuffer::uninit()` (`src/bun_core/util.rs`) return a `[u8; N]` / `[u16; N]` built with `MaybeUninit::uninit().assume_init()`. An integer must be initialized, so this is undefined behavior at the constructor, before any byte is read. Miri: `constructing invalid value of type bun_core::PathBuffer: at .0[0], encountered uninitialized memory, but expected an integer` (`cargo miri test -p bun_sys dir::tests::borrow_does_not_close`). - `bufs_storage_init` (`src/resolver/resolver.rs`) does the same with `Box::<Bufs>::new_uninit().assume_init()`. Both sites silenced the `invalid_value` and `clippy::uninit_assumed_init` lints that catch this. ### Fix - Remove both constructors and `impl Default` for both types. Every scratch call site (about 440, mechanical one-line edits) takes a buffer from `bun_paths::path_buffer_pool::get()` or its `w_` / `os_` siblings. The pool zeroes a buffer once on allocation and reuses it, so there is no per-call memset and no 98 KB stack frame on Windows. Structs built per item (`NodeFS`, `bin::NamesIterator`, `PosixToWinNormalizer`) hold a pool guard. Long-lived struct fields use an explicit `ZEROED`. - The pool gains: a `try_with` fallback to the heap while TLS destructors run, `Default` for `PoolGuard` so structs can derive it, and `__lsan_ignore_object` on each allocation. The last one matters because a guard live at `Global::exit` never drops, and in an optimized build its stack slot is dead, so LeakSanitizer reported the buffer and the ASAN lane aborted every `bun <file.md>` run. - The resolver's `Bufs` uses `Box::new_zeroed()` (once per thread). - Verified: `test/internal/source-lints/uninit-assumed-init.test.ts` (its known-site list is now empty, so it fails on main), new pool unit tests under Miri (`cargo miri test -p bun_paths`, 29 pass), a release ASAN build with the CI `ASAN_OPTIONS` on the markdown, `bun exec` and `bunx` paths, `cargo check` for linux, macOS, FreeBSD, Windows x64 and arm64, clippy, and the fs, shell, spawn, path, resolve and `bun add` suites. ### Background - `PathBuffer` is a `[u8; MAX_PATH_BYTES]` newtype used as write-then-read scratch around path syscalls. `MAX_PATH_BYTES` is 4 096 on Linux, 1 024 on macOS and 98 302 on Windows. `WPathBuffer` is the `[u16; 32767]` sibling for Windows wide paths. - "Every bit pattern is a valid `u8`" is true, but uninitialized memory is not a bit pattern. The Rust reference lists an integer read from uninitialized memory as undefined behavior. - `bun_paths::path_buffer_pool` is a per-thread LIFO of up to four boxed buffers with an RAII guard. It already existed with about 180 callers. A cache miss allocates with `alloc_zeroed`, which is usually fresh OS-zeroed pages. <details><summary>Notes</summary> Why not `ZEROED` at the call sites: on Windows that is a 98 KB memset per call at about 440 sites. A previous attempt did exactly that and the leak and stress tests timed out, which is why `uninit()` was introduced. #31258 proposes `ZEROED` again and has the same cost. Why not `[MaybeUninit<u8>; N]` inside `PathBuffer`: every syscall wrapper in `bun_sys` takes `&mut [u8]`, so an initialized-prefix API would change the whole syscall surface. The pool gives the same result with no API change. Why `impl Default` is gone: `Default` has to return a by-value buffer, so it can only be `ZEROED`. With it, `#[derive(Default)]` on a struct that embeds a `PathBuffer` silently picked up a 98 KB memset per construction on Windows (`PosixToWinNormalizer`, built per resolved import). Every zero-fill is now a visible `ZEROED` at its site: the crash handler (must not allocate), `bun_core::spawn_sync` (cold, linux/freebsd), and once-per-task fields in `PackageInstaller`, `extract_tarball`, the test `Scanner`, `Tree` iterators, `PatchFile`, `WindowsWatcher` and the resolver's `top_level_dir_buf`. Overlap with #40680: that PR moves `NodeFS::sync_error_buf` to a pool guard and the `fetch_impl` locals to the pool for Windows stack depth. This PR contains the same source changes (the `fetch_impl` locals were part of the sweep). #40680's test is not included here. LeakSanitizer and `Global::exit`: reproduced with `bun run build:asan` and `ASAN_OPTIONS=detect_leaks=1:abort_on_error=1`. `render_markdown_file_and_exit` held two guards at `exit`, and LSan reported the 4 KB buffer allocated at `path_buffer_pool::get` (`run_command.rs:3423`). `new_boxed` now calls `__lsan_ignore_object` on each buffer (under `cfg(bun_asan)`), which fixes every such site. The markdown body was also split into a function that returns the exit code. Verified both ways: with the original `-> !` shape plus only the LSan call, the test passes. Perf check (debug builds, main vs this branch, three runs each): `Glob("**/*.rs").scan` over `src/` 457/475/459 ms vs 478/453/452 ms, 20 000 `statSync` + `realpathSync` 2663/2674/2663 ms vs 2663/2717/2683 ms, 2 000 `readdirSync` 617/618/617 vs 617/628/626 ms, 4 000 `Bun.resolveSync` 261/262/262 vs 263/266/265 ms. No difference outside noise. No Windows measurement. Windows has the most to gain (no 98 KB stack frames) and nothing in the hot paths zero-fills per call. `bun_sys` cannot join `MIRI_CRATES` in `scripts/rust-miri.ts`: after this change Miri gets past the constructor and stops at `can't call foreign function openat64`, which it has no shim for. The pool and its unit tests stay in `bun_paths`, which is in `MIRI_CRATES`. `test/js/bun/glob/scan.test.ts` times out in this container on main and on this branch alike: it scans the whole checkout, which holds 685 000 files of build output here. Test suites run with the debug build: `test/js/node/fs/fs.test.ts` (565 pass), `test/js/node/fs/fs-mkdir.test.ts`, `test/js/bun/shell/bunshell.test.ts` (431 pass), `test/js/bun/spawn/spawn.test.ts` (148 tests), `test/js/node/path/path.test.js`, `test/js/bun/resolve/` (390 pass, one 5 s timeout that also fails on main here), `test/cli/install/bun-add.test.ts` (71 pass), `test/cli/run/markdown-entrypoint.test.ts`. With the release ASAN build and the CI leak settings: `test/cli/run/` (1177 pass, 4 environment failures: FUSE, uid/gid, a 60 s timeout) and `test/js/bun/shell/exec.test.ts`. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 1 · 129 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/internal/source-lints/uninit-assumed-init.test.ts bun test v1.4.3 (e0a2b82) test/internal/source-lints/uninit-assumed-init.test.ts: (pass) scans a non-empty set of tracked Rust sources [1.99ms] 68 | }); 69 | 70 | test("no new #[allow(invalid_value)] or #[allow(clippy::uninit_assumed_init)]", () => { 71 | // `[^)]*` spans the multi-line form of the attribute; lint paths never 72 | // contain `)`. 73 | expect(scan(/#!?\[(?:allow|expect)\([^)]*\b(?:invalid_value|clippy::uninit_assumed_init)\b/)).toEqual(KNOWN_SITES); ^ error: expect(received).toEqual(expected) - [] + [ + "src/bun_core/util.rs", + ] - Expected - 1 + Received + 3 at <anonymous> (/workspace/bun/test/internal/source-lints/uninit-assumed-init.test.ts:73:97) (fail) no new #[allow(invalid_value)] or #[allow(clippy::uninit_assumed_init)] [126.91ms] 1 pass 1 fail 2 expect() calls Ran 2 tests across 1 file. [23.59s] error: script "bd" exited with code 1 __F:1:S:0 release without fix: 1 FAILED bun test v1.4.3-canary.1 (1491acd) test/internal/source-lints/uninit-assumed-init.test.ts: (pass) scans a non-empty set of tracked Rust sources [0.11ms] 68 | }); 69 | 70 | test("no new #[allow(invalid_value)] or #[allow(clippy::uninit_assumed_init)]", () => { 71 | // `[^)]*` spans the multi-line form of the attribute; lint paths never 72 | // contain `)`. 73 | expect(scan(/#!?\[(?:allow|expect)\([^)]*\b(?:invalid_value|clippy::uninit_assumed_init)\b/)).toEqual(KNOWN_SITES); ^ error: expect(received).toEqual(expected) - [] + [ + "src/bun_core/util.rs", + ] - Expected - 1 + Received + 3 at <anonymous> (/workspace/bun/test/internal/source-lints/uninit-assumed-init.test.ts:73:97) (fail) no new #[allow(invalid_value)] or #[allow(clippy::uninit_assumed_init)] [12.05ms] 1 pass 1 fail 2 expect() calls Ran 2 tests across 1 file. [561.00ms] __F:1:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/internal/source-lints/uninit-assumed-init.test.ts bun test v1.4.3 (e0a2b82) test/internal/source-lints/uninit-assumed-init.test.ts: (pass) scans a non-empty set of tracked Rust sources [2.21ms] (pass) no new #[allow(invalid_value)] or #[allow(clippy::uninit_assumed_init)] [122.33ms] 2 pass 0 fail 2 expect() calls Ran 2 tests across 1 file. [24.05s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 575ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/5] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 242 extern-C blocks audited [1/5] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_brotli_sys v0.0.0 (/workspace/bun/src/brotli_sys) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compi ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/bun_core/lib.rs | 9 ++ src/bun_core/util.rs | 49 ++------- src/bundler/OutputFile.rs | 7 +- src/bundler/linker_context/computeChunks.rs | 4 +- .../linker_context/generateChunksInParallel.rs | 2 +- .../linker_context/writeOutputFilesToDisk.rs | 6 +- src/bundler/options.rs | 4 +- src/bundler/transpiler.rs | 4 +- src/bunfig/arguments.rs | 6 +- src/crash_handler/lib.rs | 4 +- src/dotenv/env_loader.rs | 4 +- src/event_loop/MiniEventLoop.rs | 2 +- src/glob/GlobWalker.rs | 14 ++- src/install/PackageInstall.rs | 12 +-- src/install/PackageInstaller.rs | 22 ++-- src/install/PackageManager.rs | 14 +-- src/install/PackageManager/CommandLineArguments.rs | 6 +- .../PackageManager/PackageManagerDirectories.rs | 16 +-- .../PackageManager/PackageManagerEnqueue.rs | 8 +- .../PackageManager/PackageManagerOptions.rs | 9 +- .../PackageManager/PackageManagerResolution.rs | 3 +- .../PackageManager/WorkspacePackageJSONCache.rs | 4 +- src/install/PackageManager/patchPackage.rs | 22 ++-- .../PackageManager/updatePackageJSONAndInstall.rs | 9 +- src/install/TarballStream.rs | 6 +- src/install/bin.rs | 12 +-- src/install/extract_tarball.rs | 14 ++- src/install/hoisted_install.rs | 4 +- src/install/isolated_install.rs | 6 +- src/install/lib.rs | 6 +- src/install/lockfile.rs | 8 +- src/install/lockfile/Package.rs | 8 +- src/install/loc ... (truncated) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/bun_core/lib.rs 1 2 11 src/bun_core/util.rs 1 3 14 src/bundler/OutputFile.rs 0 0 9 src/bundler/linker_context/computeChunks.rs 0 0 9 src/bundler/linker_context/generateChunksInParallel.rs 0 0 9 src/bundler/linker_context/writeOutputFilesToDisk.rs 0 0 9 src/bundler/options.rs 0 0 9 src/bundler/transpiler.rs 0 0 9 src/bunfig/arguments.rs 0 0 9 src/crash_handler/lib.rs 0 0 9 src/dotenv/env_loader.rs 0 0 9 src/event_loop/MiniEventLoop.rs 0 0 9 src/glob/GlobWalker.rs 0 0 11 src/install/PackageInstall.rs 0 0 9 src/install/PackageInstaller.rs 0 0 9 src/install/PackageManager.rs 0 0 9 (+ 113 more files) ``` </details> **root cause** · written by the author bot `PathBuffer::uninit`, `WPathBuffer::uninit`, and the resolver's `bufs_storage_init` materialized `[u8; N]` and `[u16; N]` values from `MaybeUninit::uninit().assume_init()`, which is undefined behavior because integer types require initialized memory regardless of whether the bytes are later read, and the `invalid_value` and `uninit_assumed_init` lints that diagnose this were suppressed at those sites. The fix removes those constructors and migrates their call sites to the existing `path_buffer_pool`, which produces buffers through `Box::new_zeroed().assume_init()` so every byte is written b… <!-- robobun:evidence:end -->
|
The test from this PR is now proposed on its own in #41493. |
2 participants
Problem
fetch()on Windows x64 reserves about 297 KB of native stack (thefetch_impl<false>prologue in 1.4.0:movl $0x48bb8, %eax; callq __chkstk). Linux: 11 KB.fetch_impl(src/runtime/webcore/fetch.rs) holds fivePathBuffer-sized locals, three live at once, and aPathBufferis 98 302 bytes on Windows. One of the five is aNodeFS, which embeds aPathBufferby value, so everyNodeFS::default()stack local in the tree (15 sites) is 96 KB too.Symbol.asyncIteratorgetter callsfetch()again recurses 16 levels on Windows before the RangeError, 387 on Linux.Fix
PathBufferlocals come frombun_paths::path_buffer_pool::get(): a 16-byte guard on the stack, the buffer on the heap.NodeFS::sync_error_bufbecomes apath_buffer_pool::Guard.NodeFSshrinks to two words and everyNodeFS::default()site with it.#[repr(C)]goes: theu16reinterpretation now relies on the heap allocation's alignment, which the existingassert!checks.PathBuffertype. A probe over file:, blob: andBun.file()bodies (Notes) gives identical output before and after.test/js/web/fetch/body-async-iterator.test.ts, which stock 1.4.1 fails on Windows (16 < 32). Windows debug build: 9 levels before, 96 after. Windows-host clippylarge_stack_frames(128 KB threshold): 45 functions before, 38 after.Background
PathBufferis[u8; MAX_PATH_BYTES]: 4096 bytes on Linux, 1024 on macOS, 98 302 on Windows.bun_paths::path_buffer_poolcaches up to four heapPathBuffers per thread behind an RAII guard, for exactly this problem.__chkstkthen probes every page of the frame: 73 pages perfetch()call here.Notes
path_buf,path_buf2,cwd_buf(Windows only) in the file:/blob: branch,open_path_bufandNodeFS::sync_error_bufin theBun.file()body branch. The two branches never run together, so the compiler shares their slots and the frame holds three buffers. On Linuxcwd_bufdoes not exist: 2 x 4 KB plus 3 KB of other locals matches the 11 KB prologue.PathBufferis 4 KB there, so the recursion already reaches 387 levels (release) or 57 (debug, ASAN). It fails on Windows only. Measured on windows-x64 with stock1.4.1-canary:Expected: >= 32, Received: 16. With this branch, the Windows debug build reaches 96 and the Linux debug build 71. The test passed on every CI lane, including both Windows release lanes.AbortSignal, so the recursion happens in body extraction and no socket is opened (strace -e connectshows zero calls).cargo clippy -p bun_runtime --no-depson a Windows host (large_stack_framesisdenyinCargo.tomlwithstack-size-threshold = 131072, but CI runs clippy on Linux only; bun_core: fix clippy on the Windows and FreeBSD targets and lint them in CI #37605 adds the Windows target). Functions that leave the list:fetch.rs:385fetch_impl(539 424 bytes estimated, clippy does not model slot sharing),node_fs.rs:4427NodeFS::default,node_fs_binding.rs:126Binding::new,jsc_hooks.rs:1323create_node_fs,Blob.rs:4285,test_command.rs:1668,aggregate.rs:164. The 38 that remain are mostly CLI commands; the JS-facing ones areListener.rs:176(Bun.listen, 206 KB),path.rs:1355(path.resolve, 197 KB),node_fs.rs:8258(297 KB),copy_file.rs:1475(199 KB),glob.rs:33(296 KB) andfilesystem_router.rs:109(203 KB). Those are follow-ups.NodeFS(VirtualMachine::node_fs()) for the fetch read: whenNodeFS.vmis set (standalone executables),read_fileclaims the VM pipe-read scratch and returns aMarkedArrayBuffercreated in the JSC heap. The fetch caller copiesresult.slice()and callsdestroy(), which is a no-op for that buffer, so it would work but leave a dead ArrayBuffer perBun.file()body. A freshNodeFSwithvm: Noneis what the code did before, and it is now cheap.fetch("file:rel.txt")panics atdebug_assert!(crate::is_absolute_windows(maybe_posix_path))inresolve_cwd_with_external_buf_z. The URL parser turns the input into the path/rel.txt,is_absoluteaccepts the leading slash, then the Windows branch strips it and passes the relativerel.txtto the normalizer. Release builds skip the assertion and open the file relative to cwd. Reproduced on main.fetch.test.ts(same 25 release / 41 debug environment failures before and after, allUnable to connectto alocalhostlistener or debug timeouts),fetch-file-upload.test.ts,blob.test.ts,fetch-compress.test.ts,body.test.ts,fetch-args.test.ts,regression/issue/29787.test.ts,node/fs/{fs,fs-mkdir,fs-path-length,cp,promises}.test,bun-write.test.js,bun-file.test.ts,shell/commands/mkdir.test.ts,blob-write.test.ts,test/internal/source-lints. On Windows:body-async-iterator,fs,fs-mkdir,fs-path-length,readdir-windows-ntstatus,rm-windows-ntstatus,bun-file-windows,bun-write. The one failure seen (readSync > works on large files, 5 s timeout) also times out with the stock canary on that machine.[review] gate passed · iteration 0 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file