From 1253703ea1d0db320e48850f141c2dac9283c4d3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:14:21 +0000 Subject: [PATCH 1/5] test(fetch-leak): replace fixed sleeps with polling, shrink the bodies, run the children concurrently fetch-leak.test.ts took 48-82 s on every CI lane. Almost all of it was fixture-5 sleeping 100 ms after each of its 50 batches for each of the seven body types (35 s of sleep), plus fixture-2 pushing 55 x 36 MB bodies through three TLS/TCP variants. - fixture, fixture-2, fixture-5: poll heapStats() for the Response / Promise counts to drop (bounded) instead of sleeping, print one JSON line that the test asserts on, and exit non-zero with the counts when they never drop. - fixture-2: 4 MiB bodies (the deflated one still incompressible) and 80 measured requests for every variant; the test asserts the growth in bodies against COUNT / 4, where a real leak measures about COUNT. The old thresholds were 1000 bodies at COUNT=1000 and, under ASAN, 80 at COUNT=50, i.e. a one-body-per-request leak did not fail them. - fixture-5: 150 requests per type, RSS growth asserted against an absolute 96 MB (a leak measures 280-300 MB, clean runs under 15 MB) instead of "less than 10x the first sample"; the server tallies how each request framed its body so an empty body cannot pass. - The RSS-measuring children run with MIMALLOC_PURGE_DELAY=0 and the ASAN quarantine disabled so RSS tracks live memory (clean-run jitter went from +-40 MB to a few MB), which is what allows the tighter bounds. - The two groups of children run concurrently; the in-process "do not leak" test sits between them as the barrier, and now actually awaits and asserts something (it returned before its interval ever fired and called listen() twice): AbortSignal and Response counts after GC. - Children's stdout/stderr are piped and asserted rather than inherited. Release build: 60 s -> 7.6 s locally, 85 s -> 8.3 s on Windows arm64. Debug+ASAN build: 301 s with the URLSearchParams case timing out -> ~125 s passing. --- .../js/web/fetch/fetch-leak-test-fixture-2.js | 135 +++---- .../js/web/fetch/fetch-leak-test-fixture-5.js | 116 ++++-- test/js/web/fetch/fetch-leak-test-fixture.js | 64 +-- test/js/web/fetch/fetch-leak.test.ts | 375 ++++++++++-------- 4 files changed, 392 insertions(+), 298 deletions(-) diff --git a/test/js/web/fetch/fetch-leak-test-fixture-2.js b/test/js/web/fetch/fetch-leak-test-fixture-2.js index da15bfd089c9..181ea0c30ed2 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture-2.js +++ b/test/js/web/fetch/fetch-leak-test-fixture-2.js @@ -1,3 +1,10 @@ +// Spawned by fetch-leak.test.ts ("fetch doesn't leak > ... > fixture #2"). Fetches +// SERVER COUNT times, buffering every body through the ReadableStream fast +// path, and prints one JSON line with how many bodies' worth of RSS the process +// grew by. The parent owns the thresholds. +// +// env: SERVER (url), COUNT (measured requests), NAME ("tcp" | "tls" | +// "tls-with-client"; the last one passes per-request tls options to fetch()). import { heapStats } from "bun:jsc"; const rss = @@ -5,83 +12,71 @@ const rss = ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; -const { SERVER } = process.env; - -if (typeof SERVER === "undefined" || !SERVER?.length) { - throw new Error("SERVER environment variable is not set"); -} - -const COUNT = parseInt(process.env.COUNT || "50", 10); -// ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run far higher under bun-asan; widen the per-request threshold there. -const isASAN = process.execPath.includes("bun-asan"); -var oks = 0; -var textLength = 0; -Bun.gc(true); -async function getBaseline() { - async function iterate() { - const tls = - process.env.NAME === "tls-with-client" - ? { - cert: "-----BEGIN CERTIFICATE-----\nMIIDXTCCAkWgAwIBAgIJAKLdQVPy90jjMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV\nBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX\naWRnaXRzIFB0eSBMdGQwHhcNMTkwMjAzMTQ0OTM1WhcNMjAwMjAzMTQ0OTM1WjBF\nMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50\nZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA7i7IIEdICTiSTVx+ma6xHxOtcbd6wGW3nkxlCkJ1UuV8NmY5ovMsGnGD\nhJJtUQ2j5ig5BcJUf3tezqCNW4tKnSOgSISfEAKvpn2BPvaFq3yx2Yjz0ruvcGKp\nDMZBXmB/AAtGyN/UFXzkrcfppmLHJTaBYGG6KnmU43gPkSDy4iw46CJFUOupc51A\nFIz7RsE7mbT1plCM8e75gfqaZSn2k+Wmy+8n1HGyYHhVISRVvPqkS7gVLSVEdTea\nUtKP1Vx/818/HDWk3oIvDVWI9CFH73elNxBkMH5zArSNIBTehdnehyAevjY4RaC/\nkK8rslO3e4EtJ9SnA4swOjCiqAIQEwIDAQABo1AwTjAdBgNVHQ4EFgQUv5rc9Smm\n9c4YnNf3hR49t4rH4yswHwYDVR0jBBgwFoAUv5rc9Smm9c4YnNf3hR49t4rH4ysw\nDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEATcL9CAAXg0u//eYUAlQa\nL+l8yKHS1rsq1sdmx7pvsmfZ2g8ONQGfSF3TkzkI2OOnCBokeqAYuyT8awfdNUtE\nEHOihv4ZzhK2YZVuy0fHX2d4cCFeQpdxno7aN6B37qtsLIRZxkD8PU60Dfu9ea5F\nDDynnD0TUabna6a0iGn77yD8GPhjaJMOz3gMYjQFqsKL252isDVHEDbpVxIzxPmN\nw1+WK8zRNdunAcHikeoKCuAPvlZ83gDQHp07dYdbuZvHwGj0nfxBLc9qt90XsBtC\n4IYR7c/bcLMmKXYf0qoQ4OzngsnPI5M+v9QEHvYWaKVwFY4CTcSNJEwfXw+BAeO5\nOA==\n-----END CERTIFICATE-----", - } - : null; - const resp = await fetch(SERVER, { tls }); - textLength = Number(resp.headers.get("Content-Length")); - if (!textLength) { - throw new Error("Content-Length header is not set"); - } - (await resp.arrayBuffer()).byteLength; - } - - for (let i = 0; i < Math.max(Math.ceil(COUNT / 10), 1); i++) { - await iterate(); - } - - Bun.gc(true); - - return rss(); +const { SERVER, NAME } = process.env; +const COUNT = parseInt(process.env.COUNT, 10); +if (!SERVER || !Number.isSafeInteger(COUNT) || COUNT < 10) { + throw new Error("SERVER and COUNT (>= 10) must be set: " + JSON.stringify({ SERVER, COUNT: process.env.COUNT })); } -const baseline = await getBaseline(); - -for (let j = 0; j < COUNT; j++) { - await (async function runAll() { - const tls = - process.env.NAME === "tls-with-client" - ? { - cert: "-----BEGIN CERTIFICATE-----\nMIIDXTCCAkWgAwIBAgIJAKLdQVPy90jjMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV\nBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX\naWRnaXRzIFB0eSBMdGQwHhcNMTkwMjAzMTQ0OTM1WhcNMjAwMjAzMTQ0OTM1WjBF\nMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50\nZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA7i7IIEdICTiSTVx+ma6xHxOtcbd6wGW3nkxlCkJ1UuV8NmY5ovMsGnGD\nhJJtUQ2j5ig5BcJUf3tezqCNW4tKnSOgSISfEAKvpn2BPvaFq3yx2Yjz0ruvcGKp\nDMZBXmB/AAtGyN/UFXzkrcfppmLHJTaBYGG6KnmU43gPkSDy4iw46CJFUOupc51A\nFIz7RsE7mbT1plCM8e75gfqaZSn2k+Wmy+8n1HGyYHhVISRVvPqkS7gVLSVEdTea\nUtKP1Vx/818/HDWk3oIvDVWI9CFH73elNxBkMH5zArSNIBTehdnehyAevjY4RaC/\nkK8rslO3e4EtJ9SnA4swOjCiqAIQEwIDAQABo1AwTjAdBgNVHQ4EFgQUv5rc9Smm\n9c4YnNf3hR49t4rH4yswHwYDVR0jBBgwFoAUv5rc9Smm9c4YnNf3hR49t4rH4ysw\nDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEATcL9CAAXg0u//eYUAlQa\nL+l8yKHS1rsq1sdmx7pvsmfZ2g8ONQGfSF3TkzkI2OOnCBokeqAYuyT8awfdNUtE\nEHOihv4ZzhK2YZVuy0fHX2d4cCFeQpdxno7aN6B37qtsLIRZxkD8PU60Dfu9ea5F\nDDynnD0TUabna6a0iGn77yD8GPhjaJMOz3gMYjQFqsKL252isDVHEDbpVxIzxPmN\nw1+WK8zRNdunAcHikeoKCuAPvlZ83gDQHp07dYdbuZvHwGj0nfxBLc9qt90XsBtC\n4IYR7c/bcLMmKXYf0qoQ4OzngsnPI5M+v9QEHvYWaKVwFY4CTcSNJEwfXw+BAeO5\nOA==\n-----END CERTIFICATE-----", - } - : null; - oks += !!( - await fetch(SERVER, { tls }).then(r => { - r.body; - return r.arrayBuffer(); - }) - )?.byteLength; - })(); -} +const tls = + NAME === "tls-with-client" + ? { + cert: "-----BEGIN CERTIFICATE-----\nMIIDXTCCAkWgAwIBAgIJAKLdQVPy90jjMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV\nBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX\naWRnaXRzIFB0eSBMdGQwHhcNMTkwMjAzMTQ0OTM1WhcNMjAwMjAzMTQ0OTM1WjBF\nMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50\nZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA7i7IIEdICTiSTVx+ma6xHxOtcbd6wGW3nkxlCkJ1UuV8NmY5ovMsGnGD\nhJJtUQ2j5ig5BcJUf3tezqCNW4tKnSOgSISfEAKvpn2BPvaFq3yx2Yjz0ruvcGKp\nDMZBXmB/AAtGyN/UFXzkrcfppmLHJTaBYGG6KnmU43gPkSDy4iw46CJFUOupc51A\nFIz7RsE7mbT1plCM8e75gfqaZSn2k+Wmy+8n1HGyYHhVISRVvPqkS7gVLSVEdTea\nUtKP1Vx/818/HDWk3oIvDVWI9CFH73elNxBkMH5zArSNIBTehdnehyAevjY4RaC/\nkK8rslO3e4EtJ9SnA4swOjCiqAIQEwIDAQABo1AwTjAdBgNVHQ4EFgQUv5rc9Smm\n9c4YnNf3hR49t4rH4yswHwYDVR0jBBgwFoAUv5rc9Smm9c4YnNf3hR49t4rH4ysw\nDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEATcL9CAAXg0u//eYUAlQa\nL+l8yKHS1rsq1sdmx7pvsmfZ2g8ONQGfSF3TkzkI2OOnCBokeqAYuyT8awfdNUtE\nEHOihv4ZzhK2YZVuy0fHX2d4cCFeQpdxno7aN6B37qtsLIRZxkD8PU60Dfu9ea5F\nDDynnD0TUabna6a0iGn77yD8GPhjaJMOz3gMYjQFqsKL252isDVHEDbpVxIzxPmN\nw1+WK8zRNdunAcHikeoKCuAPvlZ83gDQHp07dYdbuZvHwGj0nfxBLc9qt90XsBtC\n4IYR7c/bcLMmKXYf0qoQ4OzngsnPI5M+v9QEHvYWaKVwFY4CTcSNJEwfXw+BAeO5\nOA==\n-----END CERTIFICATE-----", + } + : undefined; -if (oks !== COUNT) { - throw new Error("Not all requests succeeded"); +let requests = 0; +// Decoded size of the body, taken from the first response; every response must +// match it. (Content-Length is the encoded size when the body is compressed.) +let bodySize = 0; +async function request() { + const response = await fetch(SERVER, { tls }); + if (!response.headers.has("Content-Length")) throw new Error("Content-Length header is not set"); + // Touching .body first makes arrayBuffer() go through the buffered + // ReadableStream fast path instead of reading the body directly. + response.body; + const { byteLength } = await response.arrayBuffer(); + bodySize ||= byteLength; + if (byteLength !== bodySize) throw new Error(`body was ${byteLength} bytes, expected ${bodySize}`); + requests++; } -await Bun.sleep(10); -Bun.gc(true); -const delta = rss() - baseline; -if ((heapStats().objectTypeCounts.Response ?? 0) > 5) { - throw new Error("Too many Response objects: " + heapStats().objectTypeCounts.Response); +// Every Response is garbage once request() returns, bar the most recent ones, +// which the native side lets go of on a later event-loop turn: poll for that +// instead of sleeping. A leak never gets under the limit, so the poll is bounded +// and the count is reported instead. +const maxResponsesAlive = 5; +async function collectResponses() { + const deadline = Date.now() + 5_000; + for (;;) { + Bun.gc(true); + const alive = heapStats().objectTypeCounts.Response ?? 0; + if (alive <= maxResponsesAlive || Date.now() >= deadline) return alive; + await new Promise(resolve => setImmediate(resolve)); + } } -const bodiesLeakedPerRequest = delta / textLength; - -const threshold = (textLength > 1024 * 1024 * 2 ? 20 : 1000) * (isASAN ? 4 : 1); +// Warm up so the connection pool, decompressor state and allocator are at +// steady state before the baseline is taken. +for (let i = 0; i < COUNT / 10; i++) await request(); +await collectResponses(); +const baseline = rss(); -console.log({ delta, count: COUNT, bodySize: textLength, bodiesLeakedPerRequest, threshold }); +for (let i = 0; i < COUNT; i++) await request(); +const responsesAlive = await collectResponses(); -if (bodiesLeakedPerRequest > threshold) { - console.log("\n--fail--\n"); - process.exit(1); -} else { - console.log("\n--pass--\n"); +const growth = rss() - baseline; +console.log( + JSON.stringify({ + requests, + count: COUNT, + bodySize, + responsesAlive, + rssGrowthMB: Math.round((growth / 1024 / 1024) * 10) / 10, + // A body retained per request shows up here as roughly COUNT. + bodiesRetained: Math.round((growth / bodySize) * 10) / 10, + }), +); +if (responsesAlive > maxResponsesAlive) { + throw new Error(`${responsesAlive} Response objects are still alive after ${requests} buffered fetches`); } diff --git a/test/js/web/fetch/fetch-leak-test-fixture-5.js b/test/js/web/fetch/fetch-leak-test-fixture-5.js index 41293fa722d3..b13e49fd5332 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture-5.js +++ b/test/js/web/fetch/fetch-leak-test-fixture-5.js @@ -1,27 +1,34 @@ +// Spawned by fetch-leak.test.ts ("Sending does not leak"): +// fetch-leak-test-fixture-5.js +// POSTs the requests in batches, waits for each batch's Response objects and +// fetch() promises to be collected, and prints one JSON line for the parent to +// assert on. RSS growth is measured from after the first batch so one-time +// allocations (JIT, allocator warm-up, the cached body) are not counted. import { heapStats } from "bun:jsc"; -import { expect } from "bun:test"; const rss = process.platform === "darwin" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; -function getHeapStats() { - return heapStats().objectTypeCounts; -} const server = process.argv[2]; -const batch = 10; -const iterations = 50; -const threshold = batch * 2 + batch / 2; -// JSC's C++ module loader keeps a handful of pipeline JSPromises live in the -// module map (fetch/module/load per registry entry) for the life of the -// process. These are constant across iterations, so account for them -// separately from the per-batch leak threshold. -const promiseThreshold = threshold + 10; const BODY_SIZE = parseInt(process.argv[3], 10); -if (!Number.isSafeInteger(BODY_SIZE)) { - console.error("BODY_SIZE must be a safe integer", BODY_SIZE, process.argv); +const type = process.argv[4]; +const REQUESTS = parseInt(process.argv[5], 10); +const batch = 10; +if (!Number.isSafeInteger(BODY_SIZE) || !Number.isSafeInteger(REQUESTS) || REQUESTS % batch !== 0) { + console.error("body size must be an integer and the request count a multiple of", batch, process.argv); process.exit(1); } +// Everything a batch allocated is garbage once the batch has settled, but the +// most recent batch or two can still be pinned by the native side when a GC +// runs (see collectBatch), so only counts that stay above a couple of batches' +// worth are a leak. +const maxResponses = batch * 2 + batch / 2; +// JSC's C++ module loader keeps a handful of pipeline JSPromises live in the +// module map (fetch/module/load per registry entry) for the life of the +// process. These are constant across batches, so account for them separately +// from the per-batch leak threshold. +const maxPromises = maxResponses + 10; function getFormData() { const formData = new FormData(); @@ -48,13 +55,13 @@ function getString() { return getBuffer().toString(); } function getURLSearchParams() { + // The URLSearchParams itself is what gets copied and serialized per request; + // the string it is built from can be shared. const urlSearchParams = new URLSearchParams(); - urlSearchParams.set("file", getString()); + urlSearchParams.set("file", (cachedBody ??= getString())); return urlSearchParams; } -const type = process.argv[4]; - // Cache only buffer/string since those aren't reference counted the same way. let cachedBody; function getBody() { @@ -83,6 +90,8 @@ function getBody() { case "stream": body = new ReadableStream({ async pull(c) { + // Hand the chunk over after the request has started so the body is + // streamed into an in-flight request rather than buffered up front. await Bun.sleep(10); c.enqueue((cachedBody ??= getBuffer())); c.close(); @@ -96,36 +105,61 @@ function getBody() { return body; } -async function iterate() { +let requests = 0; +async function sendBatch() { const promises = []; for (let j = 0; j < batch; j++) { promises.push(fetch(server, { method: "POST", body: getBody() })); } - await Promise.all(promises); + for (const response of await Promise.all(promises)) { + if (response.status !== 200) throw new Error(`unexpected status ${response.status}`); + requests++; + } } -try { - for (let i = 0; i < iterations; i++) { - await iterate(); - - { - Bun.gc(true); - await Bun.sleep(100); - Bun.gc(true); - const stats = getHeapStats(); - expect(stats.Response || 0).toBeLessThanOrEqual(threshold); - expect(stats.Promise || 0).toBeLessThanOrEqual(promiseThreshold); - process.send({ - rss: rss(), - }); +// A settled fetch() still has its Response and promise pinned until the native +// side lets go of them on a later event-loop turn, so poll for the counts to +// drop rather than sleeping a fixed amount. A real leak never drops below the +// limits, so the poll is bounded and the last counts seen are reported. +async function collectBatch() { + const deadline = Date.now() + 5_000; + for (;;) { + Bun.gc(true); + const counts = heapStats().objectTypeCounts; + const responses = counts.Response ?? 0; + const promises = counts.Promise ?? 0; + if ((responses <= maxResponses && promises <= maxPromises) || Date.now() >= deadline) { + return { responses, promises }; } + await new Promise(resolve => setImmediate(resolve)); } - process.send({ - rss: rss(), - }); - await Bun.sleep(10); - process.exit(0); -} catch (e) { - console.error(e); - process.exit(1); +} + +let peakResponses = 0; +let peakPromises = 0; +let baseline = 0; +while (requests < REQUESTS) { + await sendBatch(); + const { responses, promises } = await collectBatch(); + peakResponses = Math.max(peakResponses, responses); + peakPromises = Math.max(peakPromises, promises); + // Once a batch has failed to be collected every later one would only wait out + // the deadline again. + if (responses > maxResponses || promises > maxPromises) break; + if (requests === batch) baseline = rss(); +} + +console.log( + JSON.stringify({ + type, + requests, + peakResponses, + peakPromises, + rssGrowthMB: Math.round(((rss() - baseline) / 1024 / 1024) * 10) / 10, + }), +); +if (peakResponses > maxResponses || peakPromises > maxPromises) { + throw new Error( + `${peakResponses} Response and ${peakPromises} Promise objects survived GC after ${requests} requests (limits ${maxResponses} and ${maxPromises})`, + ); } diff --git a/test/js/web/fetch/fetch-leak-test-fixture.js b/test/js/web/fetch/fetch-leak-test-fixture.js index a785d6d40fdc..bdb52e19d983 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture.js +++ b/test/js/web/fetch/fetch-leak-test-fixture.js @@ -1,34 +1,50 @@ +// Spawned by fetch-leak.test.ts ("fetch doesn't leak > fixture #1"). Fetches +// SERVER COUNT times without ever reading the bodies, which used to keep every +// Response reachable from the native side forever, then prints one JSON line +// with how many Response objects survived GC. import { heapStats } from "bun:jsc"; const { SERVER } = process.env; - -if (typeof SERVER === "undefined" || !SERVER?.length) { - throw new Error("SERVER environment variable is not set"); +const COUNT = parseInt(process.env.COUNT, 10); +if (!SERVER || !Number.isSafeInteger(COUNT)) { + throw new Error("SERVER and COUNT must be set: " + JSON.stringify({ SERVER, COUNT: process.env.COUNT })); } +// A leak keeps all COUNT Responses alive. Unconsumed Responses are otherwise +// garbage as soon as fetch() has settled; the poll below normally ends with one +// survivor (the most recent Response, still visible to the conservative stack +// scan), so this only needs to be well under the size of the last batch. +const maxResponsesAlive = 5; -const COUNT = parseInt(process.env.COUNT || "200", 10); -await (async function runAll() { - var fetches = new Array(COUNT); - let i = 0; - while (i < Math.max(COUNT - 32, 0)) { - for (let j = 0; j < 32; j++) { - fetches.push(fetch(SERVER)); +// Runs in its own frame so that none of the Responses are still on the stack +// when the survivors are counted below. +async function fetchWithoutReadingBodies() { + let requests = 0; + while (requests < COUNT) { + const batch = []; + for (let i = 0; i < Math.min(32, COUNT - requests); i++) batch.push(fetch(SERVER)); + for (const response of await Promise.all(batch)) { + if (response.status !== 200) throw new Error(`unexpected status ${response.status}`); + requests++; } - await Promise.all(fetches); - fetches.length = 0; - i += 32; } + return requests; +} - while (i++ < COUNT) { - fetches.push(fetch(SERVER)); - } +const requests = await fetchWithoutReadingBodies(); + +// The last batch stays pinned until the native side releases it on a later +// event-loop turn, so poll instead of sleeping. Bounded so that a leak, which +// never gets under the limit, is reported instead of spinning here. +const deadline = Date.now() + 5_000; +let responsesAlive; +for (;;) { + Bun.gc(true); + responsesAlive = heapStats().objectTypeCounts.Response ?? 0; + if (responsesAlive <= maxResponsesAlive || Date.now() >= deadline) break; + await new Promise(resolve => setImmediate(resolve)); +} - await Promise.all(fetches); - fetches.length = 0; - fetches = []; -})(); -await Bun.sleep(10); -Bun.gc(true); -if ((heapStats().objectTypeCounts.Response ?? 0) > 1 + ((COUNT / 2) | 0)) { - throw new Error("Too many Response objects: " + heapStats().objectTypeCounts.Response); +console.log(JSON.stringify({ requests, responsesAlive })); +if (responsesAlive > maxResponsesAlive) { + throw new Error(`${responsesAlive} Response objects are still alive after ${requests} unconsumed fetches`); } diff --git a/test/js/web/fetch/fetch-leak.test.ts b/test/js/web/fetch/fetch-leak.test.ts index 5c9a9cf290d1..166a1c0c831a 100644 --- a/test/js/web/fetch/fetch-leak.test.ts +++ b/test/js/web/fetch/fetch-leak.test.ts @@ -1,206 +1,255 @@ -import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, tls as COMMON_CERT, gc, isASAN, isCI, isDebug } from "harness"; +import { heapStats } from "bun:jsc"; +import { beforeAll, describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tls as COMMON_CERT, gc, isASAN } from "harness"; +import { randomBytes } from "node:crypto"; import { once } from "node:events"; import { createServer } from "node:http"; import net from "node:net"; import { join } from "node:path"; -describe("fetch doesn't leak", () => { - test("fixture #1", async () => { - const body = new Blob(["some body in here!".repeat(100)]); - var count = 0; - using server = Bun.serve({ - port: 0, - idleTimeout: 0, - fetch(req) { - count++; - return new Response(body); - }, - }); +// Environment for the children below whose RSS growth is asserted. Both +// allocators are told to hand freed memory straight back to the OS so that RSS +// tracks live memory: mimalloc otherwise keeps freed pages resident until later +// allocator activity purges them (with multi-MB bodies that alone moves the +// measurement by tens of MB from one run to the next), and ASAN's quarantine +// retains freed allocations (256 MB by default). Leaked memory is never freed, +// so neither setting hides a leak. +const rssEnv = { + ...bunEnv, + MIMALLOC_PURGE_DELAY: "0", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "quarantine_size_mb=0"].filter(Boolean).join(":"), +}; + +// Every test here spawns one child that talks to a server in this process and +// measures its own memory, so the group runs concurrently. (The in-process +// "do not leak" test below is what keeps this group and the "Sending" group +// from running at the same time.) +describe.concurrent("fetch doesn't leak", () => { + // A debug+ASAN build takes 2-4 s per child with the whole group running at + // once (release: under 1 s). + const timeout = 30_000; + + // Response objects whose bodies are never read. + test( + "fixture #1", + async () => { + const body = new Blob(["some body in here!".repeat(100)]); + let served = 0; + using server = Bun.serve({ + port: 0, + idleTimeout: 0, + fetch() { + served++; + return new Response(body); + }, + }); - await using proc = Bun.spawn({ - env: { - ...bunEnv, - SERVER: server.url.href, - COUNT: "200", - }, - stderr: "inherit", - stdout: "inherit", - cmd: [bunExe(), "--smol", join(import.meta.dir, "fetch-leak-test-fixture.js")], - }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--smol", join(import.meta.dir, "fetch-leak-test-fixture.js")], + env: { ...bunEnv, SERVER: server.url.href, COUNT: "200" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const exitCode = await proc.exited; - expect(exitCode).toBe(0); - expect(count).toBe(200); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toMatchObject({ requests: 200 }); + expect(served).toBe(200); + expect(exitCode).toBe(0); + }, + timeout, + ); + + // Response bodies (buffered through the ReadableStream fast path) and the + // Response objects themselves, over plain TCP, TLS, and TLS with per-request + // tls options, with and without Content-Encoding. + // + // The child reports its RSS growth in units of one body. Leaking a body per + // request comes out at about COUNT; a clean run is left with a few MB of + // connection buffers and GC jitter either way, which does not grow with COUNT + // and measured at most 2.6 bodies on release and 3.6 on debug+ASAN across all + // six variants running at once. COUNT / 4 = 20 is well clear of both. + const BODY_BYTES = 4 * 1024 * 1024; + const COUNT = 80; + let plainBody: Blob; + let deflatedBody: Blob; + beforeAll(() => { + plainBody = new Blob([Buffer.alloc(BODY_BYTES, "some body in here!")]); + // Random bytes do not compress, so this body also arrives as ~4 MiB of + // packets rather than as one small chunk. Deflating them takes a debug + // build ~0.4 s, hence once for all three variants. + deflatedBody = new Blob([Bun.deflateSync(randomBytes(BODY_BYTES))]); }); - // This tests for body leakage and Response object leakage. - async function runTest(compressed, name) { - const body = !compressed - ? new Blob(["some body in here!".repeat(2000000)]) - : new Blob([Bun.deflateSync(crypto.getRandomValues(new Buffer(65123)))]); + async function runTest(compressed: boolean, name: string) { + const body = compressed ? deflatedBody : plainBody; + const headers: Record = { "Content-Type": "application/octet-stream" }; + if (compressed) headers["Content-Encoding"] = "deflate"; const tls = name.includes("tls"); - const headers = { - "Content-Type": "application/octet-stream", - }; - if (compressed) { - headers["Content-Encoding"] = "deflate"; - } - - const serveOptions = { + let served = 0; + using server = Bun.serve({ port: 0, idleTimeout: 0, - fetch(req) { + ...(tls && { tls: { ...COMMON_CERT } }), + fetch() { + served++; return new Response(body, { headers }); }, - }; - - if (tls) { - serveOptions.tls = { ...COMMON_CERT }; - } - - using server = Bun.serve(serveOptions); - - const env = { - ...bunEnv, - SERVER: server.url.href, - BUN_JSC_forceRAMSize: (1024 * 1024 * 64).toString(10), - NAME: name, - }; - - if (isASAN) { - // The fixture judges leakage by RSS delta, but ASAN's quarantine retains - // freed allocations (256 MB by default): 1000 compressed-body requests - // free ~65 MB of transient buffers and the delta still reads ~276 MB — - // pure quarantine, measured with zero real leakage. Cap the child's - // quarantine so RSS tracks live memory again; a genuine - // one-body-per-request leak still exceeds the threshold by orders of - // magnitude. - env.ASAN_OPTIONS = `${bunEnv.ASAN_OPTIONS ?? ""}:quarantine_size_mb=32`.replace(/^:/, ""); - } - - if (tls) { - env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; - } - - if (compressed) { - env.COUNT = "1000"; - } + }); await using proc = Bun.spawn({ - env, - stderr: "inherit", - stdout: "inherit", cmd: [bunExe(), "--smol", join(import.meta.dir, "fetch-leak-test-fixture-2.js")], + env: { + ...rssEnv, + SERVER: server.url.href, + COUNT: String(COUNT), + NAME: name, + BUN_JSC_forceRAMSize: (1024 * 1024 * 64).toString(10), + ...(tls && { NODE_TLS_REJECT_UNAUTHORIZED: "0" }), + }, + stdout: "pipe", + stderr: "pipe", }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + console.log(stdout.trim()); - const exitCode = await proc.exited; + expect(stderr).toBe(""); + const { bodiesRetained, rssGrowthMB, responsesAlive, ...report } = JSON.parse(stdout); + // The child warms up with COUNT / 10 requests before it measures COUNT. + expect(report).toEqual({ requests: COUNT + COUNT / 10, count: COUNT, bodySize: BODY_BYTES }); + expect(served).toBe(COUNT + COUNT / 10); + expect(bodiesRetained).toBeLessThan(COUNT / 4); expect(exitCode).toBe(0); } - for (let compressed of [true, false]) { + for (const compressed of [true, false]) { describe(compressed ? "compressed" : "uncompressed", () => { - for (let name of ["tcp", "tls", "tls-with-client"]) { + for (const name of ["tcp", "tls", "tls-with-client"]) { describe(name, () => { - test("fixture #2", async () => { - await runTest(compressed, name); - }, 100000); + test("fixture #2", () => runTest(compressed, name), timeout); }); } }); } }); -describe.each(["FormData", "Blob", "Buffer", "String", "URLSearchParams", "stream", "iterator"])("Sending %s", type => { - test( - "does not leak", - async () => { - using server = Bun.serve({ - port: 0, - idleTimeout: 0, - fetch(req) { - return new Response(); - }, - }); - - const rss = []; - - await using process = Bun.spawn({ - cmd: [ - bunExe(), - "--smol", - join(import.meta.dir, "fetch-leak-test-fixture-5.js"), - server.url.href, - 1024 * 1024 * 2 + "", - type, - ], - stdin: "ignore", - stdout: "inherit", - stderr: "inherit", - env: { - ...bunEnv, - }, - ipc(message) { - rss.push(message.rss); - }, - }); - - await process.exited; - - const first = rss[0]; - const last = rss[rss.length - 1]; - if (!isCI || !(last < first * 10)) { - console.log({ rss, delta: (((last - first) / 1024 / 1024) | 0) + " MB" }); - } - expect(last).toBeLessThan(first * 10); - }, - // The URLSearchParams variant URL-encodes the 2MB body on each of the 500 - // requests - pure throughput that a debug build cannot fit in 20s, and - // ASAN instrumentation overruns the 20s release deadline the same way - // (observed 20000.61ms on the x64-asan lane). - isDebug || isASAN ? 120 * 1000 : 20 * 1000, - ); -}); - +// undici's fetch leak scenario: fetch() calls that each carry an AbortSignal, +// against a node:http server, must not keep the signal (which the in-flight +// request holds on to) or the Response alive once they have settled. Runs in +// this process, so it is serial, which also makes it the barrier between the +// two concurrent groups of children around it. test("do not leak", async () => { await using server = createServer((req, res) => { res.end(); - }).listen(0); + }).listen(0, "127.0.0.1"); await once(server, "listening"); + const url = `http://127.0.0.1:${(server.address() as net.AddressInfo).port}/`; - let url; - let isDone = false; - server.listen(0, "127.0.0.1", function attack() { - if (isDone) { - return; + async function attack(requests: number) { + for (let i = 0; i < requests; i++) { + const controller = new AbortController(); + const response = await fetch(url, { signal: controller.signal }); + await response.arrayBuffer(); } - url ??= new URL(`http://127.0.0.1:${server.address().port}`); - const controller = new AbortController(); - fetch(url, { signal: controller.signal }) - .then(res => res.arrayBuffer()) - .catch(() => {}) - .then(attack); - }); - - let prev = Infinity; - let count = 0; - var interval = setInterval(() => { - isDone = true; + } + // The Responses die on the first GC; their finalizers release the signals, + // which die on the next one. + async function survivors() { gc(); - const next = process.memoryUsage().heapUsed; - if (next <= prev) { - expect(true).toBe(true); - clearInterval(interval); - } else if (count++ > 20) { - clearInterval(interval); - expect.unreachable(); - } else { - prev = next; - } - }, 1e3); + await new Promise(resolve => setImmediate(resolve)); + gc(); + const counts = heapStats().objectTypeCounts; + return { signals: counts.AbortSignal ?? 0, responses: counts.Response ?? 0 }; + } + + await attack(10); + const baseline = await survivors(); + await attack(40); + const after = await survivors(); + + // Leaking either object would leave ~40 more of it than at the baseline; the + // last few requests may still be waiting for the native side to release them. + expect(after.signals).toBeLessThanOrEqual(baseline.signals + 5); + expect(after.responses).toBeLessThanOrEqual(baseline.responses + 5); }); +// Request bodies of every supported type, each of which fetch() turns into a +// per-request native body (a Blob store, the encoded FormData / URLSearchParams +// bytes, the streamed chunks) that has to be released once the request is done +// (#13657 kept it alive). +describe.concurrent.each(["FormData", "Blob", "Buffer", "String", "URLSearchParams", "stream", "iterator"])( + "Sending %s", + type => { + const BODY_SIZE = 1024 * 1024 * 2; + const REQUESTS = 150; + // Leaking a body per request grows the child by BODY_SIZE per request: + // measured 280-300 MB for every type with the fixture made to keep its + // bodies alive. Clean runs under rssEnv measure under 10 MB on Linux release + // and under 15 MB on debug+ASAN (quarantine is off, so ASAN needs no bound + // of its own); the limit sits about 3x below the leak and leaves the rest of + // the distance as headroom for allocator behaviour on the other platforms. + const MAX_RSS_GROWTH_MB = 96; + // URL-encoding the 2 MB body takes a debug build about 0.25 s per request + // (release: about 2 s for the whole run). + const timeout = 90_000; + + test( + "does not leak", + async () => { + // Tally how each request framed its body: this proves every request + // carried the full body (a body type that serialized to nothing would + // pass the RSS check trivially) without reading the bodies, which the + // server deliberately ignores. + const received: Record = {}; + using server = Bun.serve({ + port: 0, + idleTimeout: 0, + fetch(req) { + const contentLength = req.headers.get("content-length"); + const framing = + contentLength === null + ? `transfer-encoding: ${req.headers.get("transfer-encoding")}` + : Number(contentLength) >= BODY_SIZE + ? "content-length >= BODY_SIZE" + : `content-length: ${contentLength}`; + received[framing] = (received[framing] ?? 0) + 1; + return new Response(); + }, + }); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "--smol", + join(import.meta.dir, "fetch-leak-test-fixture-5.js"), + server.url.href, + String(BODY_SIZE), + type, + String(REQUESTS), + ], + env: rssEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + console.log(stdout.trim()); + + expect(stderr).toBe(""); + const { rssGrowthMB, peakResponses, peakPromises, ...report } = JSON.parse(stdout); + expect(report).toEqual({ type, requests: REQUESTS }); + expect(received).toEqual({ + [type === "stream" || type === "iterator" ? "transfer-encoding: chunked" : "content-length >= BODY_SIZE"]: + REQUESTS, + }); + expect(rssGrowthMB).toBeLessThan(MAX_RSS_GROWTH_MB); + expect(exitCode).toBe(0); + }, + timeout, + ); + }, +); + test("fetch(data:) with percent-encoding does not leak", async () => { // DataURL.decodeData leaked the intermediate percent-decoded buffer (and the // base64 output buffer on decode error). Each fetch of a percent-encoded From b98f8b5bbe4690b6fd278174477af3e8139e44c7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:42:59 +0000 Subject: [PATCH 2/5] test(fetch-leak): wait for the object counts to settle and bound them tightly The fixtures polled heapStats() but kept the limits sized for the old fixed-sleep transient (25 Responses / 35 promises per 10-request batch in fixture-5, 10 in fixture #1), so the poll returned on its first reading and a leak had to reach about 9% of the requests before it failed. Move the loop into fetch-leak-test-helpers.js, have it return only once two rounds agree, and bound every fixture at 5 live Responses; fixture-5 measures the promise floor after its first batch (JSC keeps a process-constant set of module-loader promises alive) and allows half a batch on top of it. Every fixture now settles at 1 Response (and 22 promises) on release and debug+ASAN builds. With that bound, the stream body type showed that a pull() landing after the server had already answered keeps the batch's Responses alive for about a second (handed off separately), so the "Sending" server now reads each body before answering, which also lets the test assert that all 150 uploads arrived in full instead of inferring it from the framing headers. The URLSearchParams child needs 40-50 s on a debug build, so that group's timeout goes back to 120 s. Comments that blamed the single surviving Response on the stack scanner, or described the thresholds' ownership wrongly, are replaced by what was measured. --- .../js/web/fetch/fetch-leak-test-fixture-2.js | 30 ++------ .../js/web/fetch/fetch-leak-test-fixture-5.js | 71 ++++++------------- test/js/web/fetch/fetch-leak-test-fixture.js | 51 ++++--------- test/js/web/fetch/fetch-leak-test-helpers.js | 40 +++++++++++ test/js/web/fetch/fetch-leak.test.ts | 54 +++++++------- 5 files changed, 109 insertions(+), 137 deletions(-) create mode 100644 test/js/web/fetch/fetch-leak-test-helpers.js diff --git a/test/js/web/fetch/fetch-leak-test-fixture-2.js b/test/js/web/fetch/fetch-leak-test-fixture-2.js index 181ea0c30ed2..74c880694007 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture-2.js +++ b/test/js/web/fetch/fetch-leak-test-fixture-2.js @@ -1,11 +1,11 @@ // Spawned by fetch-leak.test.ts ("fetch doesn't leak > ... > fixture #2"). Fetches -// SERVER COUNT times, buffering every body through the ReadableStream fast -// path, and prints one JSON line with how many bodies' worth of RSS the process -// grew by. The parent owns the thresholds. +// SERVER COUNT times, buffering every body, checks that the Response objects +// were collected, and prints one JSON line with how many bodies' worth of RSS +// the process grew by; the parent owns that threshold. // // env: SERVER (url), COUNT (measured requests), NAME ("tcp" | "tls" | // "tls-with-client"; the last one passes per-request tls options to fetch()). -import { heapStats } from "bun:jsc"; +import { expectCollected, maxResponsesAlive } from "./fetch-leak-test-helpers.js"; const rss = process.platform === "darwin" && typeof Bun.unsafe.memoryFootprint === "function" @@ -41,29 +41,16 @@ async function request() { requests++; } -// Every Response is garbage once request() returns, bar the most recent ones, -// which the native side lets go of on a later event-loop turn: poll for that -// instead of sleeping. A leak never gets under the limit, so the poll is bounded -// and the count is reported instead. -const maxResponsesAlive = 5; -async function collectResponses() { - const deadline = Date.now() + 5_000; - for (;;) { - Bun.gc(true); - const alive = heapStats().objectTypeCounts.Response ?? 0; - if (alive <= maxResponsesAlive || Date.now() >= deadline) return alive; - await new Promise(resolve => setImmediate(resolve)); - } -} +const collected = () => expectCollected({ Response: maxResponsesAlive }, `${requests} buffered fetches`); // Warm up so the connection pool, decompressor state and allocator are at // steady state before the baseline is taken. for (let i = 0; i < COUNT / 10; i++) await request(); -await collectResponses(); +await collected(); const baseline = rss(); for (let i = 0; i < COUNT; i++) await request(); -const responsesAlive = await collectResponses(); +const { Response: responsesAlive } = await collected(); const growth = rss() - baseline; console.log( @@ -77,6 +64,3 @@ console.log( bodiesRetained: Math.round((growth / bodySize) * 10) / 10, }), ); -if (responsesAlive > maxResponsesAlive) { - throw new Error(`${responsesAlive} Response objects are still alive after ${requests} buffered fetches`); -} diff --git a/test/js/web/fetch/fetch-leak-test-fixture-5.js b/test/js/web/fetch/fetch-leak-test-fixture-5.js index b13e49fd5332..ce2806ae5290 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture-5.js +++ b/test/js/web/fetch/fetch-leak-test-fixture-5.js @@ -1,10 +1,11 @@ // Spawned by fetch-leak.test.ts ("Sending does not leak"): // fetch-leak-test-fixture-5.js -// POSTs the requests in batches, waits for each batch's Response objects and -// fetch() promises to be collected, and prints one JSON line for the parent to -// assert on. RSS growth is measured from after the first batch so one-time -// allocations (JIT, allocator warm-up, the cached body) are not counted. -import { heapStats } from "bun:jsc"; +// POSTs the requests in batches, checks after every batch that its Response +// objects and fetch() promises were collected, and prints one JSON line with the +// RSS growth, whose threshold the parent owns. The growth is measured from after +// the first batch so one-time allocations (JIT, allocator warm-up, the cached +// body) are not counted. +import { expectCollected, maxResponsesAlive } from "./fetch-leak-test-helpers.js"; const rss = process.platform === "darwin" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint @@ -19,16 +20,12 @@ if (!Number.isSafeInteger(BODY_SIZE) || !Number.isSafeInteger(REQUESTS) || REQUE console.error("body size must be an integer and the request count a multiple of", batch, process.argv); process.exit(1); } -// Everything a batch allocated is garbage once the batch has settled, but the -// most recent batch or two can still be pinned by the native side when a GC -// runs (see collectBatch), so only counts that stay above a couple of batches' -// worth are a leak. -const maxResponses = batch * 2 + batch / 2; // JSC's C++ module loader keeps a handful of pipeline JSPromises live in the // module map (fetch/module/load per registry entry) for the life of the -// process. These are constant across batches, so account for them separately -// from the per-batch leak threshold. -const maxPromises = maxResponses + 10; +// process, so the promise count that a collected batch settles at is measured +// after the first batch instead of being hard-coded; a leaked promise per +// request then adds a whole batch on top of it. +const promiseSlack = batch / 2; function getFormData() { const formData = new FormData(); @@ -90,8 +87,10 @@ function getBody() { case "stream": body = new ReadableStream({ async pull(c) { - // Hand the chunk over after the request has started so the body is - // streamed into an in-flight request rather than buffered up front. + // Hand the chunk over a little after the request has started so the + // body is streamed into an in-flight request rather than buffered up + // front. (The server reads the body before answering, so no pull is + // still pending once the responses are in.) await Bun.sleep(10); c.enqueue((cachedBody ??= getBuffer())); c.close(); @@ -117,49 +116,23 @@ async function sendBatch() { } } -// A settled fetch() still has its Response and promise pinned until the native -// side lets go of them on a later event-loop turn, so poll for the counts to -// drop rather than sleeping a fixed amount. A real leak never drops below the -// limits, so the poll is bounded and the last counts seen are reported. -async function collectBatch() { - const deadline = Date.now() + 5_000; - for (;;) { - Bun.gc(true); - const counts = heapStats().objectTypeCounts; - const responses = counts.Response ?? 0; - const promises = counts.Promise ?? 0; - if ((responses <= maxResponses && promises <= maxPromises) || Date.now() >= deadline) { - return { responses, promises }; - } - await new Promise(resolve => setImmediate(resolve)); - } -} +// The first batch provides the settled promise count and the RSS baseline. +await sendBatch(); +let alive = await expectCollected({ Response: maxResponsesAlive, Promise: Infinity }, `the first ${requests} requests`); +const limits = { Response: maxResponsesAlive, Promise: alive.Promise + promiseSlack }; +const baseline = rss(); -let peakResponses = 0; -let peakPromises = 0; -let baseline = 0; while (requests < REQUESTS) { await sendBatch(); - const { responses, promises } = await collectBatch(); - peakResponses = Math.max(peakResponses, responses); - peakPromises = Math.max(peakPromises, promises); - // Once a batch has failed to be collected every later one would only wait out - // the deadline again. - if (responses > maxResponses || promises > maxPromises) break; - if (requests === batch) baseline = rss(); + alive = await expectCollected(limits, `${requests} requests`); } console.log( JSON.stringify({ type, requests, - peakResponses, - peakPromises, + responsesAlive: alive.Response, + promisesAlive: alive.Promise, rssGrowthMB: Math.round(((rss() - baseline) / 1024 / 1024) * 10) / 10, }), ); -if (peakResponses > maxResponses || peakPromises > maxPromises) { - throw new Error( - `${peakResponses} Response and ${peakPromises} Promise objects survived GC after ${requests} requests (limits ${maxResponses} and ${maxPromises})`, - ); -} diff --git a/test/js/web/fetch/fetch-leak-test-fixture.js b/test/js/web/fetch/fetch-leak-test-fixture.js index bdb52e19d983..5f5dd9cded35 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture.js +++ b/test/js/web/fetch/fetch-leak-test-fixture.js @@ -1,50 +1,27 @@ // Spawned by fetch-leak.test.ts ("fetch doesn't leak > fixture #1"). Fetches // SERVER COUNT times without ever reading the bodies, which used to keep every -// Response reachable from the native side forever, then prints one JSON line -// with how many Response objects survived GC. -import { heapStats } from "bun:jsc"; +// Response reachable from the native side forever, then checks that the +// Responses were collected and prints one JSON line for the parent. +import { expectCollected, maxResponsesAlive } from "./fetch-leak-test-helpers.js"; const { SERVER } = process.env; const COUNT = parseInt(process.env.COUNT, 10); if (!SERVER || !Number.isSafeInteger(COUNT)) { throw new Error("SERVER and COUNT must be set: " + JSON.stringify({ SERVER, COUNT: process.env.COUNT })); } -// A leak keeps all COUNT Responses alive. Unconsumed Responses are otherwise -// garbage as soon as fetch() has settled; the poll below normally ends with one -// survivor (the most recent Response, still visible to the conservative stack -// scan), so this only needs to be well under the size of the last batch. -const maxResponsesAlive = 5; -// Runs in its own frame so that none of the Responses are still on the stack -// when the survivors are counted below. -async function fetchWithoutReadingBodies() { - let requests = 0; - while (requests < COUNT) { - const batch = []; - for (let i = 0; i < Math.min(32, COUNT - requests); i++) batch.push(fetch(SERVER)); - for (const response of await Promise.all(batch)) { - if (response.status !== 200) throw new Error(`unexpected status ${response.status}`); - requests++; - } +let requests = 0; +while (requests < COUNT) { + const batch = []; + for (let i = 0; i < Math.min(32, COUNT - requests); i++) batch.push(fetch(SERVER)); + for (const response of await Promise.all(batch)) { + if (response.status !== 200) throw new Error(`unexpected status ${response.status}`); + requests++; } - return requests; -} - -const requests = await fetchWithoutReadingBodies(); - -// The last batch stays pinned until the native side releases it on a later -// event-loop turn, so poll instead of sleeping. Bounded so that a leak, which -// never gets under the limit, is reported instead of spinning here. -const deadline = Date.now() + 5_000; -let responsesAlive; -for (;;) { - Bun.gc(true); - responsesAlive = heapStats().objectTypeCounts.Response ?? 0; - if (responsesAlive <= maxResponsesAlive || Date.now() >= deadline) break; - await new Promise(resolve => setImmediate(resolve)); } +const { Response: responsesAlive } = await expectCollected( + { Response: maxResponsesAlive }, + `${requests} fetches whose bodies were never read`, +); console.log(JSON.stringify({ requests, responsesAlive })); -if (responsesAlive > maxResponsesAlive) { - throw new Error(`${responsesAlive} Response objects are still alive after ${requests} unconsumed fetches`); -} diff --git a/test/js/web/fetch/fetch-leak-test-helpers.js b/test/js/web/fetch/fetch-leak-test-helpers.js new file mode 100644 index 000000000000..41359bd48c8d --- /dev/null +++ b/test/js/web/fetch/fetch-leak-test-helpers.js @@ -0,0 +1,40 @@ +// Shared by the fetch-leak-test-fixture-*.js children of fetch-leak.test.ts. +import { heapStats } from "bun:jsc"; + +// Every fixture's Response count settles at one survivor once the requests have +// been released (measured on release and debug+ASAN builds; it does not go away +// however long you wait), while a leak keeps roughly one Response per request, +// and the fewest requests any fixture checks after is fixture-5's batch of 10. +export const maxResponsesAlive = 5; + +// Returns how many objects of each type in `limits` survive a GC, once every +// count is within its limit and has stopped falling (the same reading in two +// consecutive rounds), or throws once a count is still over its limit after five +// seconds. The objects of a settled fetch() stay reachable until the native side +// lets go of them on a later event-loop turn, and what they were holding on to +// only goes away in the GC pass after that (measured: a batch's promise count +// comes down in two or three steps), so a single reading right after the +// requests is meaningless; re-checking once per turn until two rounds agree gets +// the settled count without sleeping for a fixed time, and leaked objects are +// the ones that never come down. +export async function expectCollected(limits, context) { + const types = Object.keys(limits); + const deadline = Date.now() + 5_000; + let previous; + for (;;) { + Bun.gc(true); + const { objectTypeCounts } = heapStats(); + const counts = {}; + for (const type of types) counts[type] = objectTypeCounts[type] ?? 0; + const within = types.every(type => counts[type] <= limits[type]); + const settled = previous !== undefined && types.every(type => counts[type] === previous[type]); + const timedOut = Date.now() >= deadline; + if (within && (settled || timedOut)) return counts; + if (timedOut) { + const alive = types.map(type => `${counts[type]} ${type} objects (limit ${limits[type]})`); + throw new Error(`still alive after ${context}: ${alive.join(", ")}`); + } + previous = counts; + await new Promise(resolve => setImmediate(resolve)); + } +} diff --git a/test/js/web/fetch/fetch-leak.test.ts b/test/js/web/fetch/fetch-leak.test.ts index 166a1c0c831a..e2f4ea0995af 100644 --- a/test/js/web/fetch/fetch-leak.test.ts +++ b/test/js/web/fetch/fetch-leak.test.ts @@ -21,9 +21,9 @@ const rssEnv = { }; // Every test here spawns one child that talks to a server in this process and -// measures its own memory, so the group runs concurrently. (The in-process -// "do not leak" test below is what keeps this group and the "Sending" group -// from running at the same time.) +// measures its own memory, so the group runs concurrently. The children check +// their own Response (and promise) counts, see fetch-leak-test-helpers.js; the +// tests own the RSS bounds. describe.concurrent("fetch doesn't leak", () => { // A debug+ASAN build takes 2-4 s per child with the whole group running at // once (release: under 1 s). @@ -60,9 +60,9 @@ describe.concurrent("fetch doesn't leak", () => { timeout, ); - // Response bodies (buffered through the ReadableStream fast path) and the - // Response objects themselves, over plain TCP, TLS, and TLS with per-request - // tls options, with and without Content-Encoding. + // Buffered response bodies and the Response objects themselves, over plain + // TCP, TLS, and TLS with per-request tls options, with and without + // Content-Encoding. // // The child reports its RSS growth in units of one body. Leaking a body per // request comes out at about COUNT; a clean run is left with a few MB of @@ -190,30 +190,29 @@ describe.concurrent.each(["FormData", "Blob", "Buffer", "String", "URLSearchPara // of its own); the limit sits about 3x below the leak and leaves the rest of // the distance as headroom for allocator behaviour on the other platforms. const MAX_RSS_GROWTH_MB = 96; - // URL-encoding the 2 MB body takes a debug build about 0.25 s per request - // (release: about 2 s for the whole run). - const timeout = 90_000; + // URL-encoding the 2 MB body takes a debug build about 0.25 s per request, + // i.e. the URLSearchParams child needs 40-50 s there (release: 2-3 s). + const timeout = 120_000; test( "does not leak", async () => { - // Tally how each request framed its body: this proves every request - // carried the full body (a body type that serialized to nothing would - // pass the RSS check trivially) without reading the bodies, which the - // server deliberately ignores. - const received: Record = {}; + // The server reads every body before answering, both to prove that each + // request really uploaded the whole body (a type that serialized to + // nothing would pass the RSS check trivially) and so that no body is + // still being produced when the child counts what the responses left + // behind. (Answering while the body is still uploading is what the + // "server ignores the body" test further down covers.) + let bodies = 0; + let shortestBody = Infinity; using server = Bun.serve({ port: 0, idleTimeout: 0, - fetch(req) { - const contentLength = req.headers.get("content-length"); - const framing = - contentLength === null - ? `transfer-encoding: ${req.headers.get("transfer-encoding")}` - : Number(contentLength) >= BODY_SIZE - ? "content-length >= BODY_SIZE" - : `content-length: ${contentLength}`; - received[framing] = (received[framing] ?? 0) + 1; + async fetch(req) { + let byteLength = 0; + for await (const chunk of req.body!) byteLength += chunk.byteLength; + bodies++; + shortestBody = Math.min(shortestBody, byteLength); return new Response(); }, }); @@ -236,12 +235,11 @@ describe.concurrent.each(["FormData", "Blob", "Buffer", "String", "URLSearchPara console.log(stdout.trim()); expect(stderr).toBe(""); - const { rssGrowthMB, peakResponses, peakPromises, ...report } = JSON.parse(stdout); + const { rssGrowthMB, responsesAlive, promisesAlive, ...report } = JSON.parse(stdout); expect(report).toEqual({ type, requests: REQUESTS }); - expect(received).toEqual({ - [type === "stream" || type === "iterator" ? "transfer-encoding: chunked" : "content-length >= BODY_SIZE"]: - REQUESTS, - }); + expect(bodies).toBe(REQUESTS); + // FormData and URLSearchParams add their framing on top of the payload. + expect(shortestBody).toBeGreaterThanOrEqual(BODY_SIZE); expect(rssGrowthMB).toBeLessThan(MAX_RSS_GROWTH_MB); expect(exitCode).toBe(0); }, From 7e84794bc2e9529f6a5e6808d66cbce49a419e53 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:10:53 +0000 Subject: [PATCH 3/5] test(fetch-leak): space the settle rounds with a timer, not setImmediate On the x64-asan lane every "Sending" child sat at a whole batch of Responses (11, limit 5) for the full five seconds from its second batch on: a loop of full GCs and setImmediate turns never picked up the HTTP thread's hand-off of the finished requests. Idling for a millisecond between rounds lets it through (the same thing was reproducible locally with a body chunk arriving after the response). Release and debug+ASAN builds still settle at 1 Response / 22 promises for every fixture. --- test/js/web/fetch/fetch-leak-test-helpers.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/js/web/fetch/fetch-leak-test-helpers.js b/test/js/web/fetch/fetch-leak-test-helpers.js index 41359bd48c8d..06d6d18b8800 100644 --- a/test/js/web/fetch/fetch-leak-test-helpers.js +++ b/test/js/web/fetch/fetch-leak-test-helpers.js @@ -10,13 +10,17 @@ export const maxResponsesAlive = 5; // Returns how many objects of each type in `limits` survive a GC, once every // count is within its limit and has stopped falling (the same reading in two // consecutive rounds), or throws once a count is still over its limit after five -// seconds. The objects of a settled fetch() stay reachable until the native side -// lets go of them on a later event-loop turn, and what they were holding on to -// only goes away in the GC pass after that (measured: a batch's promise count -// comes down in two or three steps), so a single reading right after the -// requests is meaningless; re-checking once per turn until two rounds agree gets -// the settled count without sleeping for a fixed time, and leaked objects are -// the ones that never come down. +// seconds. The objects of a settled fetch() stay reachable until the HTTP thread +// is done with the request and the main thread has picked that up, and what they +// were holding on to only goes away in the GC pass after that (measured: a +// batch's promise count comes down in two or three steps), so a single reading +// right after the requests is meaningless; re-checking until two rounds agree +// gets the settled count without sleeping for a fixed time, and leaked objects +// are the ones that never come down. The rounds are spaced by a timer rather +// than setImmediate: a loop kept busy with immediates and full GCs was seen not +// to pick up the HTTP thread's hand-off at all (the x64-asan lane sat at a whole +// batch of Responses for the full five seconds, for every body type), whereas +// idling for a millisecond lets it through. export async function expectCollected(limits, context) { const types = Object.keys(limits); const deadline = Date.now() + 5_000; @@ -35,6 +39,6 @@ export async function expectCollected(limits, context) { throw new Error(`still alive after ${context}: ${alive.join(", ")}`); } previous = counts; - await new Promise(resolve => setImmediate(resolve)); + await Bun.sleep(1); } } From 4b5d9523b9fe8d909aa300357825997f74cc8f82 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:01:04 +0000 Subject: [PATCH 4/5] test(fetch-leak): check fixture-5's object counts once at the end, not after every batch Build 96371 showed that on x64-asan and Windows 2019 the native side lets go of a finished batch about a second after its responses arrive (two seconds for the streaming body types), so waiting for that after each of the 14 batches put the "Sending" group at 15-30 s on those lanes (file: 41.7 s on asan, 28.8 s on Windows 2019, against 8-9 s elsewhere). The per-batch check found nothing a final check does not: a leaked Response or promise per request still shows up as ~140 survivors against a bound of 5 over the settled count, so the fixture now settles after the first batch (baseline) and once at the end, like fixture #1 and fixture-2 do. Counts still settle at 1 Response / 22 promises; clean-run growth is unchanged on release and 2-14 MB standalone on debug+ASAN (up to ~23 MB with the whole group on a loaded box), against the 96 MB bound. --- .../js/web/fetch/fetch-leak-test-fixture-5.js | 34 +++++++++++-------- test/js/web/fetch/fetch-leak.test.ts | 7 ++-- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/test/js/web/fetch/fetch-leak-test-fixture-5.js b/test/js/web/fetch/fetch-leak-test-fixture-5.js index ce2806ae5290..f9b39052d781 100644 --- a/test/js/web/fetch/fetch-leak-test-fixture-5.js +++ b/test/js/web/fetch/fetch-leak-test-fixture-5.js @@ -1,10 +1,13 @@ // Spawned by fetch-leak.test.ts ("Sending does not leak"): // fetch-leak-test-fixture-5.js -// POSTs the requests in batches, checks after every batch that its Response -// objects and fetch() promises were collected, and prints one JSON line with the -// RSS growth, whose threshold the parent owns. The growth is measured from after -// the first batch so one-time allocations (JIT, allocator warm-up, the cached -// body) are not counted. +// POSTs the requests in batches, checks that the Response objects and fetch() +// promises were collected, and prints one JSON line with the RSS growth, whose +// threshold the parent owns. The growth is measured from after the first batch +// so one-time allocations (JIT, allocator warm-up, the cached body) are not +// counted. The counts are checked after the first batch and at the end only: on +// some lanes (x64-asan, Windows 2019) the native side takes about a second to +// let go of a finished batch, so checking after every batch cost a second per +// batch there without catching anything the final check does not. import { expectCollected, maxResponsesAlive } from "./fetch-leak-test-helpers.js"; const rss = process.platform === "darwin" && typeof Bun.unsafe.memoryFootprint === "function" @@ -22,9 +25,9 @@ if (!Number.isSafeInteger(BODY_SIZE) || !Number.isSafeInteger(REQUESTS) || REQUE } // JSC's C++ module loader keeps a handful of pipeline JSPromises live in the // module map (fetch/module/load per registry entry) for the life of the -// process, so the promise count that a collected batch settles at is measured -// after the first batch instead of being hard-coded; a leaked promise per -// request then adds a whole batch on top of it. +// process, so the promise count the process settles at is measured after the +// first batch instead of being hard-coded; a leaked promise per request then +// adds the remaining requests (REQUESTS - batch of them) on top of it. const promiseSlack = batch / 2; function getFormData() { @@ -118,14 +121,17 @@ async function sendBatch() { // The first batch provides the settled promise count and the RSS baseline. await sendBatch(); -let alive = await expectCollected({ Response: maxResponsesAlive, Promise: Infinity }, `the first ${requests} requests`); -const limits = { Response: maxResponsesAlive, Promise: alive.Promise + promiseSlack }; +const settled = await expectCollected( + { Response: maxResponsesAlive, Promise: Infinity }, + `the first ${requests} requests`, +); const baseline = rss(); -while (requests < REQUESTS) { - await sendBatch(); - alive = await expectCollected(limits, `${requests} requests`); -} +while (requests < REQUESTS) await sendBatch(); +const alive = await expectCollected( + { Response: maxResponsesAlive, Promise: settled.Promise + promiseSlack }, + `${requests} requests`, +); console.log( JSON.stringify({ diff --git a/test/js/web/fetch/fetch-leak.test.ts b/test/js/web/fetch/fetch-leak.test.ts index e2f4ea0995af..3dca31f29149 100644 --- a/test/js/web/fetch/fetch-leak.test.ts +++ b/test/js/web/fetch/fetch-leak.test.ts @@ -186,9 +186,10 @@ describe.concurrent.each(["FormData", "Blob", "Buffer", "String", "URLSearchPara // Leaking a body per request grows the child by BODY_SIZE per request: // measured 280-300 MB for every type with the fixture made to keep its // bodies alive. Clean runs under rssEnv measure under 10 MB on Linux release - // and under 15 MB on debug+ASAN (quarantine is off, so ASAN needs no bound - // of its own); the limit sits about 3x below the leak and leaves the rest of - // the distance as headroom for allocator behaviour on the other platforms. + // and under 15 MB on debug+ASAN, up to ~25 MB with all seven children on a + // loaded box (quarantine is off, so ASAN needs no bound of its own); the + // limit sits about 3x below the leak and leaves the rest of the distance as + // headroom for allocator behaviour on the other platforms. const MAX_RSS_GROWTH_MB = 96; // URL-encoding the 2 MB body takes a debug build about 0.25 s per request, // i.e. the URLSearchParams child needs 40-50 s there (release: 2-3 s). From bb79622039824ac615f10bc3b7c7d999d0c3ee4b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:09:12 +0000 Subject: [PATCH 5/5] ci: retrigger