deflake serve-body-leak: skip on ASAN where RSS-based leak detection is not meaningful by robobun · Pull Request #28301 · oven-sh/bun

robobun · 2026-03-20T03:17:01Z

The serve-body-leak test sends 10k warmup + 10k test requests per test case (7 cases = 140k requests) with 512KB payloads. Under ASAN this causes timeouts and crashes because:

ASAN adds 2-3x memory overhead, making RSS measurements meaningless for leak detection
ASAN significantly slows execution, causing test timeouts (40s/60s limits)
The subprocess gets OOM-killed under the added memory pressure

RSS-based memory leak detection is not meaningful under ASAN, which has its own built-in leak detection. Skip these tests on ASAN builds.

…is not meaningful

robobun · 2026-03-20T03:17:12Z

^{Updated 11:24 PM PT - Mar 19th, 2026}

✅ @alii, your commit 02c24f6644d49d1965211b170466370032e49e88 passed in Build #40195! 🎉

🧪 To try this PR locally:

bunx bun-pr 28301

That installs a local version of the PR into your bun-28301 executable, so you can run:

bun-28301 --bun

coderabbitai · 2026-03-20T03:19:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e0b85c19-8697-478b-a43a-9979aa472e87

📥 Commits

Reviewing files that changed from the base of the PR and between 245f9ab and 02c24f6.

📒 Files selected for processing (1)

test/js/bun/http/serve-body-leak.test.ts

Walkthrough

Imported the ASAN environment flag into a memory-leak test and updated test registration to mark these cases as todo when ASAN is active; expanded the inline comment to state ASAN makes RSS-based memory-leak thresholds unreliable.

Changes

Cohort / File(s)	Summary
Test ASAN Handling `test/js/bun/http/serve-body-leak.test.ts`	Added `isASAN` to imported environment flags; updated test registration to include ASAN in the `it.todoIf(...)` conditions so memory-leak tests are marked todo under ASAN; extended inline comment noting ASAN invalidates RSS-based leak thresholds.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: skipping the serve-body-leak test on ASAN builds where RSS-based leak detection is not meaningful.
Description check	✅ Passed	The description provides comprehensive context explaining the problem (ASAN overhead, timeouts, memory pressure), the solution (skip tests on ASAN), and reasoning.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

📝 Coding Plan

Generate coding plan for human review comments

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-03-20T03:22:02Z

Found 7 issues this PR may fix:

Bun gets OOM-killed on trivial scripts #27371 - OOM-killed issues that ASAN builds experience due to memory overhead, exactly what this PR addresses
Out of memory: total-vm:72GB RAM #27998 - Out-of-memory issues that could be related to ASAN's memory overhead making memory measurements meaningless
jemalloc reserves ~485GB VSZ per process on aarch64 macOS (Apple Silicon, 16KB pages) #28138 - Virtual memory reservation issues that could be exacerbated by ASAN builds, affecting RSS-based leak detection
Bun.serve -> request.header memory leak #9261 - Bun.serve memory leak that this PR's test is designed to catch in non-ASAN builds
Likely memoryleak inside bun runtime on service http requests #14065 - HTTP request memory leak that serve-body-leak.test.ts would be designed to detect
bun:test does not show a stack trace when a test times out #27486 - Test timeout issues that could be related to the timeout problems experienced in ASAN environments
internal: running bun's test suite concurrently causes many unusual failures #8071 - Test reliability issues that could be compounded by ASAN builds, making conditional test skipping valuable

If this is helpful, consider adding 'Fixes #' to the PR description to auto-close the issue on merge.

🤖 Generated with Claude Code

claude

LGTM, straightforward test skip for ASAN where RSS-based leak detection is meaningless.

Extended reasoning...

Overview

This PR modifies a single test file (test/js/bun/http/serve-body-leak.test.ts) to skip RSS-based memory leak tests when running under ASAN builds. The change adds isASAN to the import from harness and appends || isASAN to the it.todoIf() condition.

Security risks

None. This is a test-only change that skips tests under a specific build configuration. No production code is modified.

Level of scrutiny

Minimal scrutiny needed. This is a standard test deflaking pattern — skipping tests whose methodology (RSS measurement) is incompatible with ASAN's memory overhead. The isASAN helper is already well-established in the test harness and used elsewhere in the codebase.

Other factors

The rationale is sound: ASAN adds 2-3x memory overhead making RSS thresholds meaningless, and ASAN already has its own built-in leak detection. The change is consistent with how other tests handle ASAN incompatibility in this repo.

robobun · 2026-03-20T04:36:37Z

Deflake complete — test/js/bun/http/serve-body-leak.test.ts passed on all platforms at 245f9ab. Ready to merge.

…, not LSan Bun CI runs with detect_leaks=0 so LSan is disabled. The actual reason to skip is that ASAN's 256MB quarantine + allocator metadata pushes RSS past the 512MB threshold even without leaks, and the 2x slowdown risks hitting the 40/60s timeout.

claude

LGTM — straightforward test deflake adding isASAN to the existing skip condition.

Extended reasoning...

Overview

This PR modifies a single test file (test/js/bun/http/serve-body-leak.test.ts) to skip RSS-based memory leak tests on ASAN builds. The change adds isASAN to the import and appends || isASAN to the existing it.todoIf(...) condition. A clarifying comment is also added.

Security risks

None. This is a test-only change that conditionally skips tests under ASAN. No production code, no auth paths, no user-facing behavior is affected.

Level of scrutiny

Low scrutiny is appropriate. This is a small, mechanical test deflake that follows the exact same pattern already used in this file (e.g., isFlaky && isWindows for conditional skipping). The rationale is sound: ASAN inflates RSS via its quarantine and metadata, making the end_memory ≤ 512MB threshold meaningless, and ASAN has its own built-in leak detection anyway.

Other factors

CI passed on all platforms per the robobun comment.
No outstanding reviewer comments or unresolved discussions.
CodeRabbit found no actionable issues.
The bug hunting system found no bugs.
The change is consistent with how this repo handles ASAN-incompatible tests elsewhere.

…tection is not meaningful" (#28337) Reverts #28301

…is not meaningful (oven-sh#28301) The serve-body-leak test sends 10k warmup + 10k test requests per test case (7 cases = 140k requests) with 512KB payloads. Under ASAN this causes timeouts and crashes because: - ASAN adds 2-3x memory overhead, making RSS measurements meaningless for leak detection - ASAN significantly slows execution, causing test timeouts (40s/60s limits) - The subprocess gets OOM-killed under the added memory pressure RSS-based memory leak detection is not meaningful under ASAN, which has its own built-in leak detection. Skip these tests on ASAN builds.

…tection is not meaningful" (oven-sh#28337) Reverts oven-sh#28301

…is not meaningful (oven-sh#28301) The serve-body-leak test sends 10k warmup + 10k test requests per test case (7 cases = 140k requests) with 512KB payloads. Under ASAN this causes timeouts and crashes because: - ASAN adds 2-3x memory overhead, making RSS measurements meaningless for leak detection - ASAN significantly slows execution, causing test timeouts (40s/60s limits) - The subprocess gets OOM-killed under the added memory pressure RSS-based memory leak detection is not meaningful under ASAN, which has its own built-in leak detection. Skip these tests on ASAN builds.

…tection is not meaningful" (oven-sh#28337) Reverts oven-sh#28301

The debian-13-x64-asan-test-bun failure on the previous CI run hit test/js/bun/http/serve-body-leak.test.ts, which is a well-documented ASAN-flaky memory-leak test (see PR #28301 and test/no-validate-leaksan.txt:274). This PR only touches src/install/lockfile/ and test/cli/install/ — nothing in the HTTP/Bun.serve path. Re-rolling once.

deflake serve-body-leak: skip on ASAN where RSS-based leak detection …

245f9ab

…is not meaningful

github-actions Bot added the claude label Mar 20, 2026

claude Bot reviewed Mar 20, 2026

View reviewed changes

alii enabled auto-merge (squash) March 20, 2026 04:42

alii approved these changes Mar 20, 2026

View reviewed changes

claude Bot reviewed Mar 20, 2026

View reviewed changes

alii merged commit 9e93bfa into main Mar 20, 2026
64 checks passed

alii deleted the farm/3da66f99/deflake-serve-body-leak branch March 20, 2026 06:24

alii mentioned this pull request Mar 20, 2026

Revert "deflake serve-body-leak: skip on ASAN where RSS-based leak detection is not meaningful" #28337

Merged

alii added a commit that referenced this pull request Mar 20, 2026

Revert "deflake serve-body-leak: skip on ASAN where RSS-based leak de…

2ac158c

…tection is not meaningful" (#28337) Reverts #28301

This was referenced Mar 23, 2026

[develop] Michijs Dependabot changes michijs/michijs#623

Closed

[master] Michijs Dependabot changes michijs/michijs-storybook-template#108

Closed

[master] Michijs Dependabot changes michijs/vscode-mathml#155

Merged

structwafel pushed a commit to structwafel/bun that referenced this pull request Apr 25, 2026

Revert "deflake serve-body-leak: skip on ASAN where RSS-based leak de…

28f4f9e

…tection is not meaningful" (oven-sh#28337) Reverts oven-sh#28301

xhjkl pushed a commit to xhjkl/bun that referenced this pull request May 14, 2026

Revert "deflake serve-body-leak: skip on ASAN where RSS-based leak de…

c2fd129

…tection is not meaningful" (oven-sh#28337) Reverts oven-sh#28301

robobun mentioned this pull request May 22, 2026

install: preserve empty trustedDependencies across install + lockfile round-trip #31197

Open

robobun mentioned this pull request Jun 11, 2026

Error.captureStackTrace: keep frames when the callee frame was elided by a tail call #32136

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deflake serve-body-leak: skip on ASAN where RSS-based leak detection is not meaningful#28301

deflake serve-body-leak: skip on ASAN where RSS-based leak detection is not meaningful#28301
alii merged 2 commits into
mainfrom
farm/3da66f99/deflake-serve-body-leak

robobun commented Mar 20, 2026

Uh oh!

robobun commented Mar 20, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Mar 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Mar 20, 2026

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Mar 20, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented Mar 20, 2026

Uh oh!

robobun commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Mar 20, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Mar 20, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Mar 20, 2026 •

edited

Loading

coderabbitai Bot commented Mar 20, 2026 •

edited

Loading