From c4e7d1f1ffbf1c354df4b47c54c9df13f94fa6af Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:11:53 +0000 Subject: [PATCH 1/3] test: speed up req-url-leak.test.ts by asserting post-GC RSS growth 4x fewer requests (16,384 -> 4,096) with the same leak signal: the fixture now exposes a GC-then-report RSS endpoint, and the test compares post-GC RSS after a 1024-request warmup against the final sample instead of checking absolute RSS against an isASAN-branched threshold. A leaked 15 KB req.url over the 3072 requests in between is ~45 MB; on a fixed build the samples are flat within a few MB. Also pipes stderr and asserts it is empty, drops the 1s RSS setInterval noise from the fixture, and fails fast if the fixture exits before sending its URL. --- test/js/bun/http/req-url-leak-fixture.js | 18 +++--- test/js/bun/http/req-url-leak.test.ts | 78 ++++++++++++------------ 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/test/js/bun/http/req-url-leak-fixture.js b/test/js/bun/http/req-url-leak-fixture.js index 25dbbdf06173..299405e33d0e 100644 --- a/test/js/bun/http/req-url-leak-fixture.js +++ b/test/js/bun/http/req-url-leak-fixture.js @@ -2,6 +2,15 @@ let existingPromise = null; const server = Bun.serve({ port: 0, async fetch(req) { + // Route by method so the long-URL GET path (the leak scenario from #16787) + // stays byte-for-byte the same as before: no req.url access, only the + // await-a-microtask + rss reply. + if (req.method === "POST") { + Bun.gc(true); + await Bun.sleep(10); + Bun.gc(true); + return new Response(process.memoryUsage.rss().toString()); + } if (!existingPromise) { existingPromise = Bun.sleep(0); } @@ -14,13 +23,6 @@ const server = Bun.serve({ }, }); -setInterval(() => { - console.log("RSS", (process.memoryUsage.rss() / 1024 / 1024) | 0); -}, 1000); console.log("Server started on", server.url.href); -if (process.channel) { - process.send({ - url: server.url.href, - }); -} +process.send?.({ url: server.url.href }); diff --git a/test/js/bun/http/req-url-leak.test.ts b/test/js/bun/http/req-url-leak.test.ts index 5c94b3b0d628..5788859fe16b 100644 --- a/test/js/bun/http/req-url-leak.test.ts +++ b/test/js/bun/http/req-url-leak.test.ts @@ -1,57 +1,59 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug } from "harness"; import path from "path"; + test( "req.url doesn't leak memory", async () => { - const { promise, resolve } = Promise.withResolvers(); - await using process = Bun.spawn({ + const { promise, resolve, reject } = Promise.withResolvers(); + await using proc = Bun.spawn({ cmd: [bunExe(), path.join(import.meta.dir, "req-url-leak-fixture.js")], env: bunEnv, - stdout: "inherit", - stderr: "inherit", - stdin: "inherit", - ipc(message, child) { - if (message.url) { - resolve(message.url); - } + stdout: "pipe", + stderr: "pipe", + ipc(message) { + if (message.url) resolve(message.url); }, }); + proc.exited.then(code => reject(new Error(`req-url-leak fixture exited (${code}) before sending its URL`))); const baseURL = await promise; - const url = new URL(Buffer.alloc(1024 * 15, "Z").toString(), baseURL); + // POST = GC-then-report RSS (routed by method so the long-URL GET path keeps + // the exact #16787 scenario: no req.url access in the handler). + const report = async () => parseInt(await (await fetch(baseURL, { method: "POST" })).text()); - let maxRSS = 0; - - for (let i = 0; i < 256; i++) { - const batchSize = 64; - const promises = []; - for (let j = 0; j < batchSize; j++) { - promises.push( - fetch(url) - .then(r => r.text()) - .then(rssText => { - const rss = parseFloat(rssText); - if (Number.isSafeInteger(rss)) { - maxRSS = Math.max(maxRSS, rss); - } - }), - ); + // 8 windows of 8×64 = 4096 GETs total. A leaked 15 KB url per request grows + // RSS linearly; between samples[1] (after 1024 requests, allocator warmed up) + // and samples[7] that is 3072×15 KB ≈ 45 MB. With the fix the post-GC samples + // are flat within ±3 MB on ASAN, so 4× fewer requests than the old 16 K sweep + // keep the same separation while asserting growth instead of absolute RSS. + const batchSize = 64; + const samples: number[] = []; + for (let window = 0; window < 8; window++) { + for (let i = 0; i < 8; i++) { + const batch = new Array(batchSize); + for (let j = 0; j < batchSize; j++) batch[j] = fetch(url).then(r => r.text()); + await Promise.all(batch); } - await Promise.all(promises); + samples.push(await report()); } - console.log("Max RSS", (maxRSS / 1024 / 1024) | 0, "MB"); + proc.kill(); + const [stderr] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + + const growthMB = (samples.at(-1)! - samples[1]) / 1024 / 1024; + console.log( + "RSS samples (MB):", + samples.map(s => (s / 1024 / 1024) | 0).join(" "), + "growth:", + growthMB.toFixed(1) + "MB", + ); - // 297 MB on Bun 1.2 - // 44 MB on Bun 1.3 - // ASAN's quarantine + shadow memory raise the absolute RSS floor; widen there. - expect(maxRSS).toBeLessThan(1024 * 1024 * (isASAN ? 450 : 150)); - // Same calibration for the deadline: the 16K fetches that finish in ~3s on a - // release build run ~10x slower under ASAN's instrumentation (observed - // ~12-13s), and the run used to die on this timeout with the RSS assertion - // itself passing. + expect(stderr).toBe(""); + // 297 MB on Bun 1.2 vs 44 MB on Bun 1.3 for the old 16 K absolute-RSS check; + // as a post-GC delta the leak shows as ~45 MB here versus ~0 on a fixed build. + expect(growthMB).toBeLessThan(20); }, - isASAN ? 90_000 : 10_000, + isASAN || isDebug ? 30_000 : 10_000, ); From a5a903b1d1a7eb82c11fde9a062cd8e799e44b68 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:45:15 +0000 Subject: [PATCH 2/3] test: 3072 requests is enough for the req.url growth delta 6 windows instead of 8 keeps a ~30 MB leak signal over a 15 MB bound (fixed build sits at ~5 MB) and lets the ASAN/debug timeout stay at the original 90 s on slow hosts; CI ASAN is ~2-3 s. --- test/js/bun/http/req-url-leak.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/js/bun/http/req-url-leak.test.ts b/test/js/bun/http/req-url-leak.test.ts index 5788859fe16b..10474173dfba 100644 --- a/test/js/bun/http/req-url-leak.test.ts +++ b/test/js/bun/http/req-url-leak.test.ts @@ -23,14 +23,14 @@ test( // the exact #16787 scenario: no req.url access in the handler). const report = async () => parseInt(await (await fetch(baseURL, { method: "POST" })).text()); - // 8 windows of 8×64 = 4096 GETs total. A leaked 15 KB url per request grows + // 6 windows of 8×64 = 3072 GETs total. A leaked 15 KB url per request grows // RSS linearly; between samples[1] (after 1024 requests, allocator warmed up) - // and samples[7] that is 3072×15 KB ≈ 45 MB. With the fix the post-GC samples - // are flat within ±3 MB on ASAN, so 4× fewer requests than the old 16 K sweep - // keep the same separation while asserting growth instead of absolute RSS. + // and samples[5] that is 2048×15 KB ≈ 30 MB. With the fix the post-GC samples + // are flat within ±3 MB on ASAN, so the old 16 K sweep's absolute-RSS check + // becomes a growth bound at a fifth of the requests. const batchSize = 64; const samples: number[] = []; - for (let window = 0; window < 8; window++) { + for (let window = 0; window < 6; window++) { for (let i = 0; i < 8; i++) { const batch = new Array(batchSize); for (let j = 0; j < batchSize; j++) batch[j] = fetch(url).then(r => r.text()); @@ -52,8 +52,8 @@ test( expect(stderr).toBe(""); // 297 MB on Bun 1.2 vs 44 MB on Bun 1.3 for the old 16 K absolute-RSS check; - // as a post-GC delta the leak shows as ~45 MB here versus ~0 on a fixed build. - expect(growthMB).toBeLessThan(20); + // as a post-GC delta the leak shows as ~30 MB here versus ~0 on a fixed build. + expect(growthMB).toBeLessThan(15); }, - isASAN || isDebug ? 30_000 : 10_000, + isASAN || isDebug ? 90_000 : 10_000, ); From f97a6cd383ebdce6250534862fbd4c7941168402 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:56:35 +0000 Subject: [PATCH 3/3] test: fold fixture stderr into the early-exit reject message The stderr pipe is now started once up front and awaited on both the early-exit reject path and the happy-path assertion, so a fixture startup crash surfaces its stack trace in the test failure instead of only the exit code. --- test/js/bun/http/req-url-leak.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/js/bun/http/req-url-leak.test.ts b/test/js/bun/http/req-url-leak.test.ts index 10474173dfba..f073596629bb 100644 --- a/test/js/bun/http/req-url-leak.test.ts +++ b/test/js/bun/http/req-url-leak.test.ts @@ -15,7 +15,10 @@ test( if (message.url) resolve(message.url); }, }); - proc.exited.then(code => reject(new Error(`req-url-leak fixture exited (${code}) before sending its URL`))); + const stderr = proc.stderr.text(); + proc.exited.then(async code => + reject(new Error(`req-url-leak fixture exited (${code}) before sending its URL: ${await stderr}`)), + ); const baseURL = await promise; const url = new URL(Buffer.alloc(1024 * 15, "Z").toString(), baseURL); @@ -40,7 +43,7 @@ test( } proc.kill(); - const [stderr] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + await Promise.all([proc.stdout.text(), proc.exited]); const growthMB = (samples.at(-1)! - samples[1]) / 1024 / 1024; console.log( @@ -50,7 +53,7 @@ test( growthMB.toFixed(1) + "MB", ); - expect(stderr).toBe(""); + expect(await stderr).toBe(""); // 297 MB on Bun 1.2 vs 44 MB on Bun 1.3 for the old 16 K absolute-RSS check; // as a post-GC delta the leak shows as ~30 MB here versus ~0 on a fixed build. expect(growthMB).toBeLessThan(15);