test(fetch-leak): poll instead of sleeping, shrink the bodies, run the children concurrently - #38490
test(fetch-leak): poll instead of sleeping, shrink the bodies, run the children concurrently#38490robobun wants to merge 5 commits into
Conversation
…s, 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.
|
Warning Review limit reached
Next review available in: 20 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
Status: reproduced the slowness locally before changing anything. On this container the file took 59-60 s with the release build and 301 s with the debug+ASAN build (where With this PR (revision 4b5d952; bb79622 is an empty re-run commit after build 96642 got no build agents) the file takes 7.6-9.8 s locally on release, 123-134 s on debug+ASAN with all 29 passing, and in CI build 96713 it passed on every lane that ran it: 8.2-11.6 s on the Linux lanes, 16.5 s on x64-asan, 12.3 s on Windows 2019 and 10.5 s on macOS 14 aarch64 (previously 51-56 s, 76 s, 55 s and ~54 s); Windows 11 aarch64 passed it in the two earlier builds (8.6 s and 9.5 s) and lost its agent for that shard in this one. What is red in 96713 is unrelated to this change: four other test files on the x64-asan and macOS agents ( |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It's a ~300-line rewrite of the fetch leak fixtures and their assertions (new thresholds, describe.concurrent, MIMALLOC_PURGE_DELAY/quarantine_size_mb=0 for RSS measurement, a rewritten in-process "do not leak" test), so a human look at the threshold choices and the concurrent-children pattern across all CI lanes would still be worthwhile.
What was reviewed:
- Assertions are strictly tightened, not loosened — old fixture-2's
bodiesLeaked <= 1000at COUNT=1000 and the broken interval-based "do not leak" test are replaced with bounds well below the measured leak signal. - Subprocess pattern matches the harness rules: pipes drained concurrently,
stderrasserted empty beforeexitCode, JSON output asserted exactly. bodySize: BODY_BYTESassertion holds for the compressed variant too (child reports the decodedarrayBuffer().byteLength, which is the pre-deflate size).- Removed imports (
isCI,isDebug) are no longer referenced;isASAN/netare still used by the untouched tests.
Extended reasoning...
Overview
This PR rewrites three fetch-leak fixture scripts and the corresponding tests in test/js/web/fetch/fetch-leak.test.ts to cut the file's CI wall time from ~50-80 s down to ~8 s (release) while strengthening the leak assertions. The fixtures replace fixed Bun.sleep() calls with bounded heapStats() polling, shrink body sizes and request counts, print one JSON line that the parent asserts on (with piped stdout/stderr), and the two subprocess groups run under describe.concurrent. A shared rssEnv (MIMALLOC_PURGE_DELAY=0, quarantine_size_mb=0) makes RSS track live memory so the same threshold works across release and ASAN. The previously-broken in-process "do not leak" test (which returned before its interval fired and whose first tick was vacuously true) is rewritten around heapStats().objectTypeCounts for AbortSignal/Response.
Security risks
None. Test-only changes with no runtime code touched, no new external dependencies, no network egress (all servers are local Bun.serve/node:http on port: 0).
Level of scrutiny
Medium-high. Although test-only, this is a large non-mechanical rewrite of leak-detection logic: new thresholds derived from local measurements, a new RSS-measurement environment, and 7-way subprocess concurrency per group. Leak-test thresholds are historically flake-prone across CI lanes; the PR description documents Linux release, debug+ASAN, and Windows aarch64 stability runs but a maintainer should sanity-check that the margins hold on macOS lanes and the 4-vCPU test agents in practice. The description also notes overlap with several open PRs (#37209, #33988, #35081, #35498, #36148, #37425) touching adjacent lines, which a human should be aware of when landing.
Other factors
- The change follows every relevant REVIEW.md test guideline I checked: polls instead of sleeps (bounded at 5 s with a comment), drains subprocess pipes concurrently, asserts stderr/stdout before exit code, lowers timeouts after shrinking the workload rather than raising them, and keeps asserting the same properties the originals protected — more tightly.
- The
bodySizeexact-match assertion in fixture #2 was checked against the compressed path: the child reports the decodedarrayBuffer().byteLength, which equalsBODY_BYTESfor both the plain anddeflateSync(randomBytes(BODY_BYTES))bodies. - The removal of per-lane
isASANthreshold branching in favour ofquarantine_size_mb=0in the child is a design choice already used by other tests in this file (proxy/file:); the description argues it cannot hide a leak because leaked memory is never freed. That reasoning is sound, but it's the kind of methodology change a maintainer should sign off on. - No prior reviews from me or other humans on this PR; the only timeline entry is a CodeRabbit rate-limit notice.
|
On the one open point from the review above, the macOS lanes: those are the only lanes I could not measure locally (Linux release and debug+ASAN, plus Windows 11 aarch64 with the release canary, are in the description). Two things make me expect them to behave like the measured ones: the 4 MiB response bodies and the per-request request bodies are bun allocations, so |
|
Updated 6:04 PM PT - Aug 14th, 2026
❌ @robobun, your commit bb79622 has 5 failures in
🧪 To try this PR locally: bunx bun-pr 38490That installs a local version of the PR into your bun-38490 --bun |
… 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.
|
Pushed a second revision (b98f8b5) after going over the first one again. What changed:
Build 95979 (first revision) passed this file on all nine lanes that ran it, 7-9 s on the release lanes and 15 s on x64-asan; its macOS jobs expired in the queue, so those numbers will come from the new build. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It's a substantial rewrite of the fetch-leak fixtures and thresholds (new polling helper, concurrent child groups, new RSS bounds under MIMALLOC_PURGE_DELAY=0/quarantine_size_mb=0, and a from-scratch "do not leak" test), with macOS still unmeasured and several open PRs touching adjacent lines — a human sign-off on the threshold choices and CI-stability trade-offs would be worthwhile.
What was reviewed:
expectCollected()loop: settles only when counts are within limit and two rounds agree; times out at 5 s with a useful message;setImmediatebetween GCs so native-side releases can land.- fixture-2 compressed path:
randomBytesdeflated → decodedbodySizematchesBODY_BYTES, so the parent'sreport.bodySize === BODY_BYTESassertion holds for both variants. - Concurrent groups: the in-process
heapStatstest sits between the twodescribe.concurrentblocks as a serial barrier, so its counts aren't polluted by concurrently running children. - Imports (
isASAN,net) still used by the untouched tests further down the file.
Extended reasoning...
Overview
Test-only rewrite of test/js/web/fetch/fetch-leak.test.ts plus three fixture children and a new shared helper. Replaces fixed Bun.sleep with a poll-until-settled expectCollected() helper, shrinks bodies (36 MB → 4 MiB for fixture-2), moves threshold ownership from children to the parent test via one-line JSON reports on piped stdout, runs the two 7-child groups under describe.concurrent, and rewrites the previously-vacuous in-process "do not leak" test to assert AbortSignal/Response object counts. Cuts release wall time ~60 s → ~8 s and fixes a debug-build timeout.
Security risks
None. Test infrastructure only; no production code, no new network egress (all servers are local Bun.serve/node:http on port 0), no auth or crypto paths.
Level of scrutiny
Moderate-to-high for a test change. Leak-test thresholds are a recurring source of CI flakes in this repo (the description itself references six open PRs touching this file), the new bounds are absolute rather than ratio-based, and one platform (macOS) was not measured locally. The author's verification is unusually thorough — 18 release runs, 6 debug+ASAN runs, per-lane CI numbers, deliberate-leak measurements to size the margins — but the threshold values (5 Responses, COUNT/4 bodies, 96 MB RSS) and the concurrent-children design are judgment calls that a maintainer who watches these lanes should ratify.
Other factors
- The old "do not leak" test was demonstrably broken (returned before its interval fired;
prev = Infinitymade the first tick vacuously pass; doublelisten()); the rewrite is a real behavioural change to what the test asserts, not a pure speed-up. - Explicit coverage trade-off called out in the description: compressed fixture-2 variants drop from 1000 → 88 requests, giving up some Response-count sensitivity to rare per-request races in exchange for a body-RSS check that can actually fail.
- Multiple open PRs (#37209, #33988, #36148, #37425, #35081, #35498) touch adjacent lines; a human should confirm the intended merge order.
- CI build #96351 for this revision is still in flight at review time; the macOS numbers are meant to come from it.
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.
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it's a substantial rewrite of the leak-test thresholds and polling logic — with macOS lanes still unverified and the x64-asan failure on b98f8b5 only addressed by the just-pushed 7e84794 whose CI hasn't reported yet — a human look once the full build is green would still be worthwhile.
What was reviewed:
expectCollectedpolling loop: checked the settle-twice + 5 s deadline logic, and thatwithin && timedOutstill returns rather than throws.- fixture-2
bodySizecheck for the compressed variant — decodedbyteLengthmatchesBODY_BYTESsincerandomBytes(4 MiB)is deflated then inflated back. shortestBody >= BODY_SIZEfor all seven body types — the alphanumeric payload doesn't shrink under URL/FormData encoding.isASANimport is still used by the untouched tests further down;rssEnvcorrectly extendsbunEnv.ASAN_OPTIONSrather than replacing it.
Extended reasoning...
Overview
This PR rewrites the three fetch-leak-test-fixture*.js children and the top ~200 lines of fetch-leak.test.ts, plus adds a shared fetch-leak-test-helpers.js. It replaces fixed Bun.sleep(100) waits with a poll-until-settled helper, shrinks the 36 MB bodies to 4 MiB, runs the two child groups under describe.concurrent, and moves the RSS/object-count assertions from the children (exit-code only) into the parent via a JSON stdout line. The in-process "do not leak" test is rewritten from a broken setInterval (which returned before firing) into a baseline+delta object-count check.
Security risks
None. Test-only change; no production code touched. The children spawn against local Bun.serve({port:0}) / node:http servers with no external network.
Level of scrutiny
Medium-high for a test change. Leak tests are threshold-calibrated per platform and this PR replaces every threshold: fixture-2 goes from bodiesLeaked <= 1000 to bodiesRetained < 20, fixture-5 from last < first * 10 to rssGrowthMB < 96, fixture #1 from 101/200 Responses to 5/200, and the object-count poll now requires two consecutive stable readings. The description documents the margins carefully, but the author explicitly notes macOS was never measured, and the x64-asan lane failed on b98f8b5 (fixed by 7e84794, whose build is not yet reported). REVIEW.md flags "never silently weaken an existing test" and the description acknowledges one such trade (compressed fixture-2 loses Response-count sensitivity from 5/1100 to 5/88).
Other factors
- The previous revision's CI (build 95979) passed 9/9 lanes that ran; macOS jobs expired in queue.
- Build 96351 (b98f8b5) failed x64-asan; commit 7e84794's helper comment directly explains and fixes that ("the x64-asan lane sat at a whole batch of Responses for the full five seconds" → switched from setImmediate to
Bun.sleep(1)). - The change is well-reasoned and the description is unusually thorough, but the combination of untested macOS behaviour, a just-fixed CI failure, and the acknowledged sensitivity trade-off on compressed fixture-2 makes this one where a maintainer should confirm the CI matrix is green and the coverage trade is acceptable before merge.
|
Third revision, 7e84794, one line of code: the settle rounds in Build 96351 (b98f8b5) showed why: on x64-asan all seven "Sending" children failed their second batch with On the review's remaining point: the macOS measurements can only come from CI, and the current build is the first one where the macOS jobs have a chance to run (95979's expired in the queue); the children still print their report lines, so the numbers will be in that build's logs. |
…t 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.
|
Fourth revision, 4b5d952. Build 96371 (third revision) was green on every lane that ran, all fixtures settling at 1 Response / 22-23 promises, but its timeline showed the cost of waiting properly after every batch: on x64-asan and Windows 2019 the native side lets go of a finished batch only about a second after its responses arrive (two seconds for the stream and iterator bodies), so the seven "Sending" children took 15-30 s there and the file came in at 41.7 s on asan and 28.8 s on Windows 2019, against 6.7-9.5 s on the other lanes. fixture-5 now checks the counts after its first batch (the baseline) and once after the last one, the way fixture #1 and fixture-2 already did, instead of after each of the 14 batches. A leaked Response or promise per request still comes out as ~140 survivors against a bound of 5 over the settled count, so nothing is lost; the two remaining waits cost at most a couple of seconds on the affected lanes. Release and debug+ASAN runs still settle at 1 / 22 (clean-run growth 0-5 MB release, 2-14 MB debug+ASAN standalone, bound 96). Description updated with the 96371 numbers; the macOS jobs expired in the queue again, so those numbers still depend on a build where they get an agent. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it's a substantial rewrite of leak-test thresholds and concurrency across all CI lanes — with macOS still unverified and the most recent CI build (#96371) showing failures — a human look would still be worthwhile before merging.
What was reviewed:
expectCollectedpoll loop: checked thatPromise: Infinityon the first fixture-5 call correctly measures the settled floor and that the timeout path throws only when a limit is exceeded.- The in-process "do not leak" test acting as a barrier between concurrent groups: baseline is taken after warm-up, so residual objects from the prior group don't skew the delta.
- Verified
isASANand other harness imports are still used by the untouched tests in the file;rssEnvcorrectly composes with any existingASAN_OPTIONS.
Extended reasoning...
Overview
This PR rewrites four fetch-leak test fixtures and the driving test file to replace fixed sleeps with polling (expectCollected in a new shared helper), shrink body sizes, run child-spawning tests concurrently, and tighten every threshold. All five changed files are under test/js/web/fetch/; no runtime code is touched.
Security risks
None. Test-only change with no auth, crypto, or user-facing surface.
Level of scrutiny
Moderate-to-high despite being test-only. Leak tests are among the most flake-prone in the suite, and this PR recalibrates absolute RSS bounds, object-count limits, and timeouts for every platform simultaneously while also switching seven child processes to run concurrently. A wrong threshold here becomes a persistent CI flake across ten lanes. The author has been thorough (18 release runs, 6 debug+ASAN runs, per-type standalone runs, measured leak-vs-clean margins), but explicitly notes macOS was never measured locally and the most recent CI build on the timeline (#96371, commit 7e84794) reports failures. A fourth commit (4b5d952) landed after that, moving fixture-5's per-batch count check to end-only, and its CI result isn't in the timeline yet.
Other factors
- The "do not leak" test is a full rewrite from an interval-based heapUsed check (which the PR correctly identifies as vacuous — first tick always passed) to an object-count delta. This is a design change worth a human eye.
- Several thresholds trade coverage for speed (compressed fixture-2 goes from 1000→80 requests; the PR description acknowledges this loses Response-count coverage of rare per-request races on that path).
- The PR interacts with at least six other open PRs touching neighbouring lines (#37209, #33988, #36148, #37425, #35081, #35498), which the author enumerates but which a maintainer should sequence.
- No bugs surfaced from the multi-agent review; the helper's settle-until-two-readings-agree logic, the
rssEnvcomposition, and the concurrent-group barrier all check out.
|
One correction to the summary above, for anyone reading the timeline: build 96371 (third revision) had no failing tests. Its only annotations were flakes that passed on retry, and this file passed on all eight lanes that ran it; what it did show was the 29-42 s wall time on Windows 2019 and x64-asan explained two comments up, which 4b5d952 addresses. The build for 4b5d952 is 96642, in progress. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it's a substantial rewrite of the leak-test thresholds and concurrency structure — with new empirically-derived bounds that haven't yet been verified on the macOS lanes and that intersect with several other open PRs touching this file — a human sign-off on the calibration choices is worthwhile.
What was reviewed
expectCollected()polling loop: termination on both the settled and timeout paths, and thatPromise: Infinitycorrectly makes the first fixture-5 call baseline-only.- The in-process "do not leak" rewrite: baseline/delta approach isolates it from Response/AbortSignal objects left by the preceding concurrent group.
- Removed imports (
isCI,isDebug) confirmed unused in the rest of the file;isASANstill used by untouched tests. - Piped-stdio pattern (
Promise.allon stdout/stderr/exited,stderrasserted before exit code) matches the file's existing conventions.
Extended reasoning...
Overview
This PR rewrites four fetch-leak test fixtures and the parent test file to cut the file's wall time from ~50-80 s to ~8-10 s on release lanes. The changes are: (1) a new shared expectCollected() helper that polls heapStats().objectTypeCounts with Bun.gc(true) until counts settle instead of sleeping fixed intervals; (2) tightened object-count bounds (5 live Responses, down from 25-101) and RSS bounds (96 MB / COUNT/4 bodies, replacing bounds that a per-request leak could pass); (3) children now emit one JSON report line that the parent parses and asserts, with stderr piped and checked; (4) two groups of 7 children each run under describe.concurrent with the rewritten in-process "do not leak" test as the serial barrier between them; (5) fixture-2 bodies drop from 36 MB to 4 MiB and fixture-5 requests from 500 to 150. All five changed files are test code only — no production code is touched.
Security risks
None. This is test infrastructure only; no auth, crypto, or user-facing surface is affected.
Level of scrutiny
Moderate-to-high, despite being test-only. Leak tests are among the most flake-prone tests in the suite, and this PR replaces empirically-tuned thresholds wholesale with new ones derived from the author's local measurements plus two CI builds. The PR description is exceptionally thorough (18 release runs, 6 debug+ASAN runs, per-lane CI numbers, deliberate-leak measurements to prove the bounds fail), but the macOS lanes never ran on any of the four revisions' builds because their jobs expired in the queue, and macOS uses a different RSS accessor (Bun.unsafe.memoryFootprint) whose behavior under MIMALLOC_PURGE_DELAY=0 is inferred rather than measured. The thresholds themselves — 5 Responses, 96 MB RSS, COUNT/4 retained bodies — are judgment calls that trade sensitivity against flake margin, and a maintainer should confirm they're comfortable with those trade-offs (the description explicitly names one: compressed fixture-2 loses per-request Response-count coverage in exchange for a working RSS check).
Other factors
- The author has iterated through four revisions in response to CI feedback (setImmediate → sleep(1) in the poll loop after x64-asan hung; per-batch checks → first+last only after Windows 2019 / asan showed 1-2 s native release latency per batch), which suggests the calibration is genuinely empirical rather than guessed.
- The PR description names six other open PRs that touch overlapping lines in this file; a maintainer will want to sequence merges.
- The "do not leak" rewrite fixes a test that was demonstrably broken (returned before its 1 s interval fired,
prev = Infinitymade the first tick always pass, doublelisten()), so that piece is a strict improvement. - The change to have the "Sending" server read every body before answering removes the early-answer scenario from these tests; the description points to the existing "server ignores the body" test as the remaining coverage for that path, which I confirmed exists further down in the file.
Given the scope of the recalibration and the unverified macOS behavior, this is worth a human look before merging.
|
On the macOS accessor point raised above: on darwin the fixtures read |
|
Build 96713 finally got a macOS agent, so the open question from the reviews is answered with data: on macOS 14 aarch64 this file passes in 10.5 s (the default-lane figure in |
1 participant
Problem
test/js/web/fetch/fetch-leak.test.tsruns in the serial phase and takes 48-56 s on every lane, 73 s on x64-asan and 82 s on Windows 11 aarch64 (test/expected-durations.json: 53.6 s default, 75.8 s asan, 51.4 s musl, 55.2 s windows), so it adds directly to the wall time of all 10 lanes.fetch-leak-test-fixture-5.js("Sending %s") didawait Bun.sleep(100)after each of its 50 batches, for each of the 7 body types: 35 s of the file is sleep. Under a local debug build the URLSearchParams case did not even finish inside its 120 s timeout.fetch-leak-test-fixture-2.jsserved 55 x 36 MB bodies per variant; the two TLS variants alone took 17 s each on Windows aarch64 (see the first side finding below).bodiesLeaked <= 1000withCOUNT = 1000(and<= 80withCOUNT = 50under ASAN), so a one-body-per-request leak passed; fixture Fix ?? operator #1 allowed 101 of its 200 Responses to survive; fixture-5's RSS check waslast < first * 10; the in-process "do not leak" test returned before its 1 s interval fired (and its first tick always passed, sinceprevstarts atInfinity) and calledlisten()on an already listening server.Fix
fetch-leak-test-helpers.js:expectCollected()GCs in rounds a millisecond apart until every count is within its limit and two consecutive readings agree, and throws with the counts after 5 s. The rounds are spaced by a timer because a loop of full GCs andsetImmediateturns was seen not to pick up the HTTP thread's hand-off of the finished requests at all: on the x64-asan lane (build 96351) every body type sat at a whole batch of Responses for the full 5 s from its second batch on, and the same happened locally in the late-chunk case below; idling for a millisecond between rounds lets it through. Every fixture is bounded at 5 live Responses (all of them settle at exactly 1, on release and debug+ASAN); fixture-5 measures the promise count its first batch settles at (22 here: JSC keeps a process-constant set of module-loader promises alive) and allows 5 on top of it when it checks again after the last batch. It checks only at those two points: 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 for the streaming types), so a check after each of the 14 batches cost 15-30 s per child there (file: 41.7 s on asan, 28.8 s on Windows 2019, 7-9.5 s everywhere else) and caught nothing the final check does not: a leaked Response or promise per request still shows up as ~140 survivors against a bound of 5. Each fixture prints one JSON line that the test parses; stdout/stderr are piped andstderris asserted empty before the exit code.beforeAll(deflating 4 MiB costs a debug build 0.4 s). The test assertsbodiesRetained < COUNT / 4(= 20; a body retained per request measures about 80), the exact request count and decoded body size from the child's report, and the server-side request count. The thresholds are no longer keyed on body size or onbun-asanin the binary name.rssGrowthMB < 96asserted by the test. The server now reads every body before answering, so the test asserts that all 150 uploads arrived and none was shorter than the payload (previously inferred fromcontent-length/transfer-encoding), and so that no body is still being produced when the child counts survivors (see the second side finding; the early-answer case stays covered by the existing "server ignores the body" test). The URLSearchParams case reuses its 2 MB source string; the URLSearchParams object is still built and serialized per request.MIMALLOC_PURGE_DELAY=0andquarantine_size_mb=0(rssEnv). Leaked memory is never freed so neither setting hides a leak, while clean-run jitter drops from about +-40 MB (10 runs of fixture-5/URLSearchParams measured -4.8 to +41.6 MB) to a few MB, which is what makes the absolute bounds safe; with the quarantine off the ASAN lane measures a flat 8-15 MB, so no separate ASAN bound is needed (the proxy and file: tests in this file already take the same approach).describe.concurrent, 7 children each) with the rewritten in-process "do not leak" test between them as the barrier: 10 + 40 fetches carrying a signal, then the post-GC AbortSignal and Response counts may exceed the baseline by at most 5 (a leak adds ~40; measured delta 0; the signals need the second GC because the Response finalizer releases them).bun bd test test/js/web/fetch/fetch-leak.test.ts(debug+ASAN): before 301.7 s withSending URLSearchParamsfailing on its timeout; after 29/29 passing in 123-134 s on an idle box (the rewritten tests take 60 s of that under heavy host load, 48 s of it the URLSearchParams child; the rest is tests this PR does not touch, e.g.fetch(file://)39 s and the two proxy tests 22 s).USE_SYSTEM_BUN=1, Linux x64): 59-60 s -> 7.6-9.8 s. Windows 11 aarch64 with the release canary: 84.9 s -> 8.2-8.3 s. CI: build 95979 (first revision, one check per batch but returning on its first reading) took 7.0-9.0 s on the seven Linux lanes, 15.1 s on x64-asan, 9.3 s on Windows 2019 x64 and 8.6 s on Windows 11 aarch64; build 96371 (third revision, waiting properly after every batch) 6.7-8.0 s on the Linux release lanes, 9.5 s on Windows 11 aarch64, but 41.7 s on x64-asan and 28.8 s on Windows 2019 for the reason above; build 96713 (current revision) 8.2-11.6 s on the Linux release lanes, 16.5 s on x64-asan, 12.3 s on Windows 2019 and 10.5 s on macOS 14 aarch64 (the first build in which the macOS queue produced an agent), every fixture at 1-2 live Responses. The Windows 11 aarch64 shard with this file lost its agent in 96713; that lane passed it in 8.6 s and 9.5 s in the two earlier builds, so every lane has now passed the file.should not leak using readable stream(untouched here, being recalibrated in test(fetch-leak): keep the RSS leak thresholds clear of measurement noise #37209) failed once in those runs on its pre-existing 5 MB bound. Margins are in the table below.fetch(file://)(fetch: reject file: URLs whose path cannot be read #37425), the compress/fragmented thresholds (test(fetch): widen fetch-leak RSS thresholds for macOS/Windows arm64 #33988). test: surface ASAN status to leak fixtures via bunEnv #35081 touches fixture-2'sisASANline, which this PR deletes; node:http: throw ERR_SERVER_ALREADY_LISTEN on second listen() #35498 removes the doublelisten()in "do not leak", which this rewrite also removes. The file's entry intest/no-validate-leaksan.txt("error exit root cause unclear") is untouched; the children's stderr is now captured, which should make it easier to revisit.Bun.servehttps response of 512 KiB or more costs a flat ~15.8 ms (256 KiB: 0.6 ms, http: ~1 ms, Linux https: 1.4 ms), which is what made the old TLS variants so slow there; and a ReadableStream request body whosepull()delivers its chunk after the server has already answered keeps that batch's Responses alive for ~1 s (indefinitely while the loop is kept busy), which the 5-Response bound exposed in fixture-5's stream case before its server started reading the bodies.Background
MIMALLOC_PURGE_DELAY=0purges on free; bun's mimalloc reads the standardMIMALLOC_*options, whichtest/napialready relies on), and ASAN keeps freed blocks in a 256 MB quarantine so that use-after-free can be detected (quarantine_size_mb=0turns that off in the child).heapStats().objectTypeCounts(frombun:jsc) counts the live JS objects of each class. A settledfetch()keeps its Response and promise reachable until the native side releases them on a later event-loop turn, and what they referenced dies in the GC pass after that, so a count is only meaningful once consecutive GCs agree on it; a leak is a count that never comes down.test.concurrent/describe.concurrenttests as one batch and treats a plaintest()as a barrier that waits for the batch, which is how the file keeps the number of children running at once to 7.Clean-run measurements behind the bounds
fixture-5
rssGrowthMB, 150 requests, bound 96:MIMALLOC_PURGE_DELAY=0: URLSearchParams -4.8 to +41.6, FormData up to 28, Buffer up to 25.Bun.unsafe.memoryFootprint()).fixture-2
bodiesRetained(4 MiB bodies), bound 20:Object counts: every fixture settles at 1 Response (bound 5) and fixture-5 at 22 promises (bound: first batch + 5) on release and debug+ASAN. In this PR's first revision the poll returned on its first reading, at 11 Responses (21 for the iterator type), which is the transient the old 25 / 35 limits were sized for.
Per-test timings, release build, before and after (Linux x64)
Before: the seven
Sendingtests took 5.2-6.3 s each (URLSearchParams 11.6-12.1 s), the three uncompressed fixture #2 variants 2.4-3.0 s each, everything else under 2.1 s; 59.1-60.3 s total.After: the
Sendinggroup finishes with its URLSearchParams child in 2.2-2.9 s, the fixture #2 group in 0.7-1.3 s,should not leak using readable stream1.9-2.0 s (untouched), everything else under 0.5 s; 7.6-9.8 s total depending on host load.no test proof · iteration 3 · Platform-specific test-only change; deferring to CI.