Repository navigation
test(serve-body-leak): run the fixture without the ASAN quarantine and with eager page release - #41560
test(serve-body-leak): run the fixture without the ASAN quarantine and with eager page release#41560robobun wants to merge 1 commit into
Conversation
…d with eager page release Under ASAN, freed memory stays resident: the quarantine pins up to 256 MB of freed blocks and the allocator returns free pages to the OS at most every 5 s. The fixture's RSS then moves by 60 to 90 MB between two samples taken 2000 requests apart with no leak, which is over the 64 MB allowance (x64-asan, build #110910, leak 83). Spawn the fixture with quarantine_size_mb=0, thread_local_quarantine_size_kb=0 and allocator_release_to_os_interval_ms=0, as archive.test.ts and fetch-leak.test.ts already do. A retained 64 KB chunk per request still measures about 120 MB and a retained body over 1 GB.
|
Updated 5:10 AM PT - Sep 6th, 2026
❌ @robobun, your commit 1274c64 has 4 failures in
The baseline build contains instructions not available on Static scan violations |
|
Status: test-only change, ready for review. Reproduced on the local debug ASAN build: leak 63 and 64 with the default ASAN options, bound 64. With the fixture's ASAN quarantine off and eager page release, CI (build #111033): |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. WalkthroughThe body-leak fixture subprocess now receives ASAN allocator settings that disable quarantines and enable immediate page release. Existing environment values remain preserved, and the settings remain inert outside ASAN. ChangesBody-leak fixture
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The fixture configures ASAN to release freed memory promptly, reducing RSS measurement noise while retaining its leak thresholds. No current merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only ASAN de-flake following the established ASAN_OPTIONS idiom.
What was reviewed:
- The
{ ...bunEnv, ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, ...].filter(Boolean).join(":") }shape matchesharness.tsexpectRssDeltaBelowand a dozen other RSS-leak tests exactly, so existing harnessASAN_OPTIONS(allow_user_segv_handler=1:...) are preserved. - The 64 MB assertion is unchanged; per the positive-control numbers a real per-request retention still measures 124 MB–1 GB, so the test still catches the regression it guards.
- Inert on non-ASAN builds (env var is ignored); colon-separated option syntax is the standard sanitizer format on all lanes.
Extended reasoning...
Overview
This PR touches a single test file, test/js/bun/http/serve-body-leak.test.ts, changing only the env passed to the spawned fixture subprocess in beforeAll. It appends three ASAN allocator options (quarantine_size_mb=0, thread_local_quarantine_size_kb=0, allocator_release_to_os_interval_ms=0) to ASAN_OPTIONS so freed memory leaves RSS immediately, eliminating 60–90 MB of quarantine/page-release jitter that was tripping the 64 MB leak allowance on the x64-asan lane with no actual leak present.
Security risks
None. This is a test-environment tweak affecting only the ASAN runtime's allocator behavior in a spawned test fixture. No production code, no user input, no auth/crypto/permission surface.
Level of scrutiny
Low. The change is mechanical and copies an idiom already used verbatim in test/harness.ts (line 355) and ~12 other leak tests (fetch-leak.test.ts, archive.test.ts, etc.). It spreads bunEnv per REVIEW.md convention, preserves the harness-set ASAN_OPTIONS via .filter(Boolean).join(":"), and leaves the assertion threshold untouched — so it does not weaken the safety net. The PR description documents a positive control (patched fixture retaining chunks measures 124 MB / 1043 MB against the 64 MB bound), satisfying the "bound well below what the unfixed leak produces" requirement.
Other factors
The added comment is on the longer side, but it explains a genuinely non-obvious mechanism (ASAN's 256 MB quarantine + 5 s page-release timer causing RSS jitter between two samples) that a future reader would otherwise spend real effort rediscovering — it clears the CLAUDE.md rule-14 bar. Bug hunt exited on dry_streak with no findings and no ruled-out candidates. No prior reviews or outstanding objections on the timeline.
|
The automated review raised no actionable comments, and there are no open review threads. Buildkite build #111033 is running. The test-only diff stays as pushed (1274c64). |
|
Thanks. No changes requested, so the diff stays as pushed (1274c64). Waiting on Buildkite build #111033. |
This branch has not been deployed
1 participant
Problem
test/js/bun/http/serve-body-leak.test.tsfails on the x64-asan lane:expect(report.leak).toBeLessThanOrEqual(64)received 83 (build #110910, "streaming the body incompletely"). Two earlier x64-asan hits on other PRs measured 70 and 80 ("streaming the body"). No hit on main in the 125 main builds before. No single culprit commit: the flake is allocator noise with no margin.report.leakis the difference of two RSS samples 2000 requests apart. With no leak, that difference moved by 60 to 90 MB (serve-body-leak.test.ts:125).Fix
quarantine_size_mb=0,thread_local_quarantine_size_kb=0andallocator_release_to_os_interval_ms=0inASAN_OPTIONS. Freed memory leaves RSS at once, so RSS tracks live memory. The options are inert off ASAN.archive.test.ts,fetch-leak.test.tsandharness.ts(expectRssDeltaBelow) use for RSS leak checks under ASAN. No bound changes.leakmeasured 124 MB (incomplete streaming) and 1043 MB (full streaming), against the 64 MB bound.bun bd test test/js/bun/http/serve-body-leak.test.ts(debug ASAN, 15 pass, max leak 4). Before the change, the same build measured leak 63 and 64 with the default options.Background
Bun.serveprocess./reportrunsBun.gc(true), sleeps 50 ms and returns its RSS. The test sends batches of 512 KB bodies and samples RSS every 1000 requests.Notes
Measurements on the local debug ASAN build,
leakin MB per scenario run (each run is 3000 streaming requests, the statistic is the last sample minus the first one):quarantine_size_mb=0:thread_local_quarantine_size_kb=0, 10 runs: 0, 57, 28, 0, 0, 46, 39, 0, 0, 44 (max 57).allocator_release_to_os_interval_ms=0, 10 runs: 3, 0, 0, 12, 0, 6, 0, 17, 3, 0 (max 17). Two full test file runs: max 1 and max 37.bun bd testafter rebuild at HEAD: max 4.Bun.gc(true)+ 50 ms until RSS stops falling) did not reduce the residual noise (max 42 over 10 runs), so it is not part of this change.process.memoryUsage()in the fixture at each sample shows heapUsed under 20 MB while RSS moves by 40 to 50 MB, so the residual noise is native allocator state, not JS heap.CI scan: 400 builds between 2026-09-03 and 2026-09-06 on all branches, 3 failures of this file, all on x64-asan (leak 70, 80, 83). 125 main builds between 2026-08-20 and 2026-09-03: none.
CI on this PR (build #111033, x64-asan release): the file passes, 15 tests in 38 s. The fixture's RSS baseline dropped from 495 to 650 MB (build #110910) to 290 to 340 MB.
leakper scenario: 0, 0, 3, 0, 35, 0, 0 over HTTP/1, then 0, 0, 0, 0, 2, 0, 0 over HTTP/2. The remaining 35 is in the streaming scenario (samples 300 and 343 MB, end 338 MB).[auto-merge] gate passed · iteration 0 · 1 files touched
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file
root cause · written by the author bot
The test's
leakvalue is the difference between two RSS samples taken from a fixture process running under ASAN's default allocator settings, where freed blocks sit in a 256 MB quarantine and pages are only returned to the OS every 5 seconds, so this deferred freeing alone shifted the measurement by 60 to 90 MB on the x64-asan lane and tripped the 64 MB bound with no real leak present. The fix spawns the fixture withquarantine_size_mb=0,thread_local_quarantine_size_kb=0, andallocator_release_to_os_interval_ms=0appended toASAN_OPTIONS, the same idiom used inharness.tsand o…