test(HTMLRewriter): measure kernel peak RSS, not a point sample that straddles a segment decommit

[robobun]

Fixes test/js/workerd/html-rewriter-leak.test.ts red on the Windows x64 / x64-baseline lanes since ~2026-07-19 (9 failures across builds 75679, 75790, 75881, 75934, 75972, 75974, 75988, 75996, 76026; zero in the 280 builds before that).

Failure

error: expect(received).toBeLessThan(expected)
Expected: < 25
Received: 30.6171875

All four retries on build 76026: 30.6 / 30.99 / 31.0 / 31.18 MB. Same ~31 MB on build 75996.

Cause

Not a leak. The fixture sampled process.memoryUsage.rss() once after pass 3 (before) and once after pass 6 (after), but RSS is bimodal between passes: mimalloc periodically hands a ~30 MB segment back to the OS, so each sample lands on either the ~60 MB peak or the ~30-47 MB trough. Ten consecutive passes on Windows release:

59.6, 60.1, 47.0, 60.1, 60.1, 48.0, 60.2, 60.2, 48.4, 60.2
59.6, 60.1, 46.5, 60.1, 60.2, 32.4, 60.2, 60.2, 33.8, 60.2
59.6, 34.5, 60.1, 60.1, 47.7, 60.1, 60.2, 34.5, 60.2, 60.2

after - before can therefore be anywhere in [-30, +30] MB with nothing retained. On the 4-vCPU D4ds_v6 Windows runners the decommit consistently lands between passes 2 and 3, so before catches the trough and after catches the peak; on larger boxes the phase happens to favour pass 3 ≈ pass 6 and the test passed.

No code in src/runtime/api/html_rewriter.rs or vendor/lolhtml changed since the test was added in #33048; the .on()/.onDocument() allocations are all released through Drop when the rewriter is collected.

Fix

Read process.resourceUsage().maxRSS (the kernel's monotone peak: VmHWM on Linux, PeakWorkingSetSize on Windows, ru_maxrss on macOS) after each 3-pass window instead of point-sampling rss(). The kernel tracks the peak continuously, so it cannot miss one between sample points regardless of when the decommit fires. A real leak raises it every pass; freed memory leaves it flat.

Same six passes, same 25 MB bound, same ~1 s runtime. The fixture now also emits the per-pass rss() trace and the test logs it before asserting, so a future red carries the diagnostic in the CI log.

Verification

Windows x64 release, 20 runs:   maxRSS delta 0.03 MB every run, 40/40 assertions pass
Linux   x64 release, 10 runs:   maxRSS delta 0 MB every run,    20/20 assertions pass
bun bd (debug+ASAN):            1 pass, 1 skip (unchanged; RSS test is skipIf(isDebug))

Leak still caught: simulating retention (push every rewriter into an array) gives a 98.9 MB maxRSS delta on Windows release.

Test-only change (the regression it guards lives entirely in LOLHTMLContext's Drop, which has not changed), so there is no src/ diff for fail-before.

---

no test proof · iteration 0 · Platform-specific test-only change; deferring to CI.

[robobun]

^{Updated 1:23 AM PT - Jul 20th, 2026}

✅ @robobun, your commit 598fcfb51e1cb736cfcee53320986e6849bceff6 passed in Build #76114! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 34786

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

bun-34786 --bun

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 19 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fc516d0e-30df-43bb-bd56-90d4f5967a3c

📥 Commits

Reviewing files that changed from the base of the PR and between f64cade and 598fcfb.

📒 Files selected for processing (1)

• test/js/workerd/html-rewriter-leak.test.ts

---

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

[claude]

LGTM — sound de-flake that preserves the original assertion strength.

What was reviewed:

• Same 6 passes and same 25 MB bound; only the sampling changed from last-of-window to max-of-window, which is stable across mimalloc decommit timing.
• Checked that a real leak (~17 MB/pass) still produces peakOver(4-6) − peakOver(1-3) ≈ 50 MB, well above the threshold — the property the test protects is intact.
• Considered whether including cold-start passes 1-2 in the before peak could mask a leak: it can only raise before, and the PR's own data shows pass 1 ≈ later peaks, so no loss of signal.

Extended reasoning...

Overview

Test-only change to test/js/workerd/html-rewriter-leak.test.ts. The RSS-based leak test was flaking on Windows x64 CI (~31 MB false-positive delta) because it took two point samples of RSS that could straddle a mimalloc segment decommit. The fix replaces before = pass 3; after = pass 6 with before = max(passes 1-3); after = max(passes 4-6), keeping the same six passes and the same 25 MB threshold. The comment block is updated to explain the bimodal RSS behavior.

Security risks

None. Pure test-fixture logic change; no production code, no new inputs, no external I/O.

Level of scrutiny

Low-to-moderate. This is a de-flake, so the key REVIEW.md concern is "keep asserting the property the original assertion protected — branch per-platform rather than dropping precision." The change does exactly that: threshold unchanged at 25 MB, and the PR description verifies a simulated leak still produces ~99 MB delta. Peak-over-window is strictly more robust than a single sample for a monotone-growth signal, and cannot produce a false negative that the old sampling wouldn't also produce (a leak raises every sample, so it raises the max).

Other factors

• The PR description includes empirical data (10-pass RSS traces on Windows) that directly demonstrates the bimodal ~30 MB swing and explains why the old point-sample approach was phase-dependent.
• Verification was run 20× on Windows release and 10× on Linux release with peak deltas ≤0.55 MB, plus a positive control (retaining rewriters → 98.9 MB delta) confirming the leak detector still fires.
• The first peakOver(3) window now includes the cold-start passes (previously discarded as warmup). I checked whether that could weaken the assertion: since we take the max, a lower cold-start RSS is ignored, and the PR's traces show pass 1 already at the ~60 MB plateau, so before is not inflated in the no-leak case.
• No prior human or bot review comments to address; no CODEOWNERS on this test file.

[claude]

LGTM — sound de-flake that swaps a bimodal point-sample for the kernel's monotone peak RSS.

What was reviewed:

• Confirmed process.resourceUsage().maxRSS on Windows goes through uv_getrusage (BunProcess.cpp:3487) → PeakWorkingSetSize / 1024, so the KB-on-Linux/Windows, bytes-on-macOS normalization is correct.
• Checked the assertion still protects the original property: same 6 passes, same 25 MB bound, and since maxRSS is a lifetime high-water mark, after - before can only be nonzero if passes 4-6 actually exceed the pass-1-3 peak — which a real leak does (~50 MB) and allocator decommit cannot.
• samples[] and the console.log are diagnostic-only, no assertion changes.

Extended reasoning...

Overview

Test-only change to test/js/workerd/html-rewriter-leak.test.ts. The RSS-based leak test was flaking on the 4-vCPU Windows lanes because it compared two process.memoryUsage.rss() point samples that could straddle a mimalloc segment decommit (~30 MB), producing a spurious +31 MB delta with nothing retained. The fix replaces the point samples with process.resourceUsage().maxRSS — the kernel's monotone lifetime peak (VmHWM / PeakWorkingSetSize) — so the delta between warmup and measured phases can never be inflated by decommit timing. The per-pass RSS samples are still collected and logged for future diagnostics.

Security risks

None. Test-only change to a memory-leak regression guard; no runtime code, no input handling.

Level of scrutiny

Low-to-moderate. It's a de-flake, so the key REVIEW.md rule is "keep asserting the property the original assertion protected." I verified:

• The workload (6 × 4000 × 64 handler registrations) and 25 MB threshold are unchanged.
• maxRSS is monotone, so after - before ≥ 0 always; a real leak (which grows ~17 MB/pass per the original #29879 regression) still produces ~50 MB and trips the bound. The PR description confirms this empirically (98.9 MB with simulated retention).
• The unit normalization (* 1024 except on darwin) matches Bun's implementation: POSIX getrusage returns KB on Linux, bytes on macOS; Windows uses uv_getrusage which returns PeakWorkingSetSize / 1024 (KB). Getting this wrong on Linux/Windows would silently make the test unable to fail, so I traced it through src/jsc/bindings/BunProcess.cpp:3487-3500.

Other factors

• No prior reviewer comments to address.
• The PR description includes 20-run Windows and 10-run Linux verification plus a positive control (simulated leak still caught), satisfying the "prove the test fails for the RIGHT reason" bar.
• The updated explanatory comment is longer than typical but carries durable non-obvious content (why maxRSS over rss()) that a future reader would need before touching this test again.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-20 and it conflicts with main. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.