From bc315957c4063ee58e387ec5272ebb32e29aaec0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:53:12 +0000 Subject: [PATCH 1/3] test(spawn): speed up spawn-noread-leak.test.ts (27s -> 2s on Windows arm64) - Reduce total spawns 3000 -> 750 (3 warmup + 12 measure batches of 50). - Spawn native `cmd /c echo x` on Windows instead of msys2 `cat` (~2.5 ms vs ~9 ms per spawn), and `echo x` instead of `cat` on POSIX. - Pipe stdout with 2 bytes written instead of an empty stderr pipe. Since the PosixBufferedReader stack-buffer fast path landed, an empty pipe hits EOF in the shared stack buffer and never allocates the per-reader Vec, so the previous form was not exercising the reserve(16 KB) path on POSIX. Writing a byte makes it fall through to the allocating loop. - Assert on post-warmup RSS delta with a per-build-type bound (5 MB release, 30 MB debug/ASAN) instead of the baseline-relative ratio. Tighter than the old `before * 3` check (~8.5 KB/spawn vs ~12 KB/spawn headroom) and stable under ASAN quarantine noise. - Check child exit codes so a missing `cmd`/`echo` fails loudly. - Drop the `0` (infinite) per-test timeout; the rewrite runs in ~2 s on the slowest lane so the default suffices. --- test/js/bun/spawn/spawn-noread-leak.test.ts | 88 ++++++++++++++++----- 1 file changed, 68 insertions(+), 20 deletions(-) diff --git a/test/js/bun/spawn/spawn-noread-leak.test.ts b/test/js/bun/spawn/spawn-noread-leak.test.ts index c3c5e204bdae..6cf59ed5235a 100644 --- a/test/js/bun/spawn/spawn-noread-leak.test.ts +++ b/test/js/bun/spawn/spawn-noread-leak.test.ts @@ -1,27 +1,75 @@ import { expect, test } from "bun:test"; -import { isASAN } from "harness"; +import { isASAN, isDebug, isWindows } from "harness"; -async function spawn() { - const proc = Bun.spawn(["cat", import.meta.path], { - stdio: ["ignore", "ignore", "pipe"], - }); - await proc.exited; -} +// Regression coverage for issue #18265 / PR #20102: a `"pipe"` stdio stream +// that JS never reads must not retain the PipeReader's read buffer past the +// child's exit. +// +// PipeReader reserves a 16 KB read buffer (libuv's suggested_size on Windows) +// after the first byte has been buffered. On POSIX the first read uses a +// shared stack buffer and only falls through to the per-reader `reserve()` +// loop once it has produced data, so the child must write at least one byte to +// the piped stream for that allocation to happen. stdout is piped and never +// consumed; `echo x` / `cmd /c echo x` writes two bytes to it and exits. +// +// The previous form spawned `cat` with only stderr piped. `cat` writes nothing +// to stderr, so since the stack-buffer fast path landed the per-reader buffer +// was never allocated there and the POSIX release lane was not exercising the +// retention path at all. Switching to a tiny stdout write restores that, and +// replacing msys2 `cat` (~9 ms/spawn) with native `cmd` (~2.5 ms/spawn) is what +// brings the Windows arm64 wall time down. + +const MB = 1024 * 1024; +const BATCH = 50; + +const cmd = isWindows ? ["cmd", "/c", "echo x"] : ["echo", "x"]; -async function spawn100() { - return Promise.all(new Array(100).fill(0).map(v => spawn())); +async function spawnBatch(): Promise { + const codes = await Promise.all( + Array.from({ length: BATCH }, async () => { + const proc = Bun.spawn(cmd, { stdio: ["ignore", "pipe", "ignore"] }); + return proc.exited; + }), + ); + // Fold all exit codes so a child that failed to launch surfaces as a test + // failure instead of a silently-different workload. + return codes.reduce((a, b) => a | b, 0); } -test("does not leak", async () => { - const before = process.memoryUsage().rss; - console.log("before", (before / 1024 / 1024).toFixed(3), "MB"); - for (let index = 0; index < 30; index++) { - await spawn100(); +test("unread 'pipe' stdio does not leak the PipeReader buffer", async () => { + let badExit = 0; + + // Warm up so lazily-created runtime state (thread pools, signal fds, JSC + // heap growth) is already in the baseline and only per-spawn retention shows + // up in the delta. + for (let i = 0; i < 3; i++) { + badExit |= await spawnBatch(); + Bun.gc(true); + } + const baseline = process.memoryUsage.rss(); + + const MEASURE_BATCHES = 12; + for (let i = 0; i < MEASURE_BATCHES; i++) { + badExit |= await spawnBatch(); Bun.gc(true); } - const after = process.memoryUsage().rss; - console.log("after", (after / 1024 / 1024).toFixed(3), "MB"); - // ASAN's quarantine retains freed allocations so RSS grows much more under - // bun-asan; widen the multiplier there. - expect(before + after).toBeLessThan(before * (isASAN ? 6 : 3)); -}, 0); + const final = process.memoryUsage.rss(); + const deltaMB = (final - baseline) / MB; + + console.log( + `RSS: ${(baseline / MB).toFixed(1)} MB -> ${(final / MB).toFixed(1)} MB ` + + `(+${deltaMB.toFixed(1)} MB over ${MEASURE_BATCHES * BATCH} spawns)`, + ); + + expect(badExit).toBe(0); + + // Release builds sit at ~0 MB delta across all platforms when nothing leaks; + // 5 MB corresponds to ~8.5 KB/spawn of touched pages, tighter than the + // previous `before * 3` ratio (which allowed ~12 KB/spawn). ASAN quarantine + // retains freed allocations on the same order as the buffer itself so the + // delta there is ~10 MB regardless of whether the retention path is broken; + // keep those lanes as a smoke check with a wider bound (the earlier 6x + // multiplier encoded the same thing). + const limitMB = isASAN || isDebug ? 30 : 5; + expect(deltaMB).toBeLessThan(limitMB); +}); From e43ba3ef65ef268c01a84f3f5b11b644d1180ccf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:17:19 +0000 Subject: [PATCH 2/3] spawn-noread-leak: fault in the retained buffer so release lanes detect it Review feedback: - The previous revision's 2-byte write only touched one page of the ~16 KB reserved buffer, so a regressed retention showed ~4 MB over 600 spawns on Linux (overcommit) and slipped under the 5 MB bound. The child now writes 16 KB via `cat`/`cmd /c type` of a temp file so the whole buffer is faulted in; the regressed delta is ~10 MB (verified by replacing the into_boxed_slice() with mem::forget in on_close_io locally: +10.0 MB x3, test fails as intended). - Drop `|| isDebug` from the bound selector: non-ASAN debug builds (native Windows/macOS-x64 `bun bd`, `--asan=off`) should use the tight bound so they can detect the leak locally. Matches the `isASAN ? ...` pattern in the other spawn leak tests. - Fix the buffer-size comment: libuv's pipe alloc_cb passes 65536 on Windows, 16 KB is the POSIX hardcode. - Bump the ASAN bound to 100 MB (observed ~50-52 MB with the 16 KB payload going through quarantine). --- test/js/bun/spawn/spawn-noread-leak.test.ts | 71 +++++++++++---------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/test/js/bun/spawn/spawn-noread-leak.test.ts b/test/js/bun/spawn/spawn-noread-leak.test.ts index 6cf59ed5235a..e07059dd5957 100644 --- a/test/js/bun/spawn/spawn-noread-leak.test.ts +++ b/test/js/bun/spawn/spawn-noread-leak.test.ts @@ -1,42 +1,46 @@ import { expect, test } from "bun:test"; -import { isASAN, isDebug, isWindows } from "harness"; +import { isASAN, isWindows, tempDir } from "harness"; +import { join } from "node:path"; // Regression coverage for issue #18265 / PR #20102: a `"pipe"` stdio stream // that JS never reads must not retain the PipeReader's read buffer past the // child's exit. // -// PipeReader reserves a 16 KB read buffer (libuv's suggested_size on Windows) -// after the first byte has been buffered. On POSIX the first read uses a -// shared stack buffer and only falls through to the per-reader `reserve()` -// loop once it has produced data, so the child must write at least one byte to -// the piped stream for that allocation to happen. stdout is piped and never -// consumed; `echo x` / `cmd /c echo x` writes two bytes to it and exits. +// PipeReader reserves a per-reader read buffer once data arrives (~16 KB on +// POSIX via the hardcoded `reserve(16 * 1024)` in PosixBufferedReader, ~64 KB +// on Windows via libuv's pipe alloc_cb suggested_size). On POSIX the first +// read uses a shared stack buffer and only falls through to the per-reader +// `reserve()` loop once it has produced data, so the child must write to the +// piped stream for that allocation to happen. The child writes a full 16 KB so +// the retained buffer's pages are actually faulted in; with a tiny write only +// one page is touched and the regressed delta sits under any useful RSS bound. // -// The previous form spawned `cat` with only stderr piped. `cat` writes nothing -// to stderr, so since the stack-buffer fast path landed the per-reader buffer -// was never allocated there and the POSIX release lane was not exercising the -// retention path at all. Switching to a tiny stdout write restores that, and -// replacing msys2 `cat` (~9 ms/spawn) with native `cmd` (~2.5 ms/spawn) is what -// brings the Windows arm64 wall time down. +// The previous form spawned msys2 `cat` (~9 ms/spawn on Windows arm64) with an +// empty stderr pipe; native `cmd /c type` / POSIX `cat` keep the spawn cost at +// ~2.5 ms / ~0.2 ms respectively. const MB = 1024 * 1024; const BATCH = 50; -const cmd = isWindows ? ["cmd", "/c", "echo x"] : ["echo", "x"]; +test("unread 'pipe' stdio does not leak the PipeReader buffer", async () => { + using dir = tempDir("spawn-noread-leak", { + "payload.bin": Buffer.alloc(16 * 1024, "x").toString(), + }); + const payload = join(String(dir), "payload.bin"); + const cmd = isWindows ? ["cmd", "/c", "type", payload] : ["cat", payload]; -async function spawnBatch(): Promise { - const codes = await Promise.all( - Array.from({ length: BATCH }, async () => { - const proc = Bun.spawn(cmd, { stdio: ["ignore", "pipe", "ignore"] }); - return proc.exited; - }), - ); - // Fold all exit codes so a child that failed to launch surfaces as a test - // failure instead of a silently-different workload. - return codes.reduce((a, b) => a | b, 0); -} + async function spawnBatch(): Promise { + const codes = await Promise.all( + Array.from({ length: BATCH }, async () => { + const proc = Bun.spawn(cmd, { stdio: ["ignore", "pipe", "ignore"] }); + return proc.exited; + }), + ); + // Fold all exit codes so a child that failed to launch surfaces as a test + // failure instead of a silently-different workload. + return codes.reduce((a, b) => a | b, 0); + } -test("unread 'pipe' stdio does not leak the PipeReader buffer", async () => { let badExit = 0; // Warm up so lazily-created runtime state (thread pools, signal fds, JSC @@ -63,13 +67,12 @@ test("unread 'pipe' stdio does not leak the PipeReader buffer", async () => { expect(badExit).toBe(0); - // Release builds sit at ~0 MB delta across all platforms when nothing leaks; - // 5 MB corresponds to ~8.5 KB/spawn of touched pages, tighter than the - // previous `before * 3` ratio (which allowed ~12 KB/spawn). ASAN quarantine - // retains freed allocations on the same order as the buffer itself so the - // delta there is ~10 MB regardless of whether the retention path is broken; - // keep those lanes as a smoke check with a wider bound (the earlier 6x - // multiplier encoded the same thing). - const limitMB = isASAN || isDebug ? 30 : 5; + // Release builds sit at ~0-1 MB delta across all platforms when nothing + // leaks; a retained 16 KB buffer over 600 spawns shows as ~10 MB. ASAN + // quarantine holds the freed buffers so the delta there (~50 MB) is + // dominated by quarantine growth regardless of whether the retention path is + // broken; keep that lane as a smoke check with a wider bound (the earlier 6x + // multiplier on the ratio assertion encoded the same thing). + const limitMB = isASAN ? 100 : 5; expect(deltaMB).toBeLessThan(limitMB); }); From 540c8ed3912abd4213d5e80b04b6684c3509f46a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:24:36 +0000 Subject: [PATCH 3/3] spawn-noread-leak: assert RSS delta before exit code --- test/js/bun/spawn/spawn-noread-leak.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/js/bun/spawn/spawn-noread-leak.test.ts b/test/js/bun/spawn/spawn-noread-leak.test.ts index e07059dd5957..38d257581a86 100644 --- a/test/js/bun/spawn/spawn-noread-leak.test.ts +++ b/test/js/bun/spawn/spawn-noread-leak.test.ts @@ -65,8 +65,6 @@ test("unread 'pipe' stdio does not leak the PipeReader buffer", async () => { `(+${deltaMB.toFixed(1)} MB over ${MEASURE_BATCHES * BATCH} spawns)`, ); - expect(badExit).toBe(0); - // Release builds sit at ~0-1 MB delta across all platforms when nothing // leaks; a retained 16 KB buffer over 600 spawns shows as ~10 MB. ASAN // quarantine holds the freed buffers so the delta there (~50 MB) is @@ -75,4 +73,5 @@ test("unread 'pipe' stdio does not leak the PipeReader buffer", async () => { // multiplier on the ratio assertion encoded the same thing). const limitMB = isASAN ? 100 : 5; expect(deltaMB).toBeLessThan(limitMB); + expect(badExit).toBe(0); });