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..f073596629bb 100644 --- a/test/js/bun/http/req-url-leak.test.ts +++ b/test/js/bun/http/req-url-leak.test.ts @@ -1,57 +1,62 @@ 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); }, }); + 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); + // 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); - } - }), - ); + // 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[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 < 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()); + await Promise.all(batch); } - await Promise.all(promises); + samples.push(await report()); } - console.log("Max RSS", (maxRSS / 1024 / 1024) | 0, "MB"); + proc.kill(); + await Promise.all([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(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); }, - isASAN ? 90_000 : 10_000, + isASAN || isDebug ? 90_000 : 10_000, );