Skip to content

test: wait for what is asserted in the two memory tests of gc-controller-cadence.test.ts #45863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dylan-conway wants to merge 1 commit into from
Closed

test: wait for what is asserted in the two memory tests of gc-controller-cadence.test.ts #45863

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/js/bun/gc/gc-controller-cadence.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ describe("idle release", () => {
const full = resident();
const deadline = performance.now() + 8000;
const timer = setInterval(() => {
if (resident() > full - 80 && performance.now() < deadline) return;
if (resident() >= full - 80 && performance.now() < deadline) return;
console.log(JSON.stringify({ full, now: resident() }));
Comment on lines +231 to 232

@claude claude Bot Oct 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: CI can still see the Worker test fail with Received equal to full - 80 after this merges, the exact flake the PR says it removes. The poll at test/js/bun/gc/gc-controller-cadence.test.ts:231 reads resident() once below the limit, then line 232 reads it again for now. Between the two reads the main thread itself runs readFileSync, a regex and JSON.stringify, which can grow its own RSS by a few KB. When the first read sat just under a MB boundary, the second truncates one MB higher and expect(now).toBeLessThan(full - 80) at line 247 fails. Fix: report the value the poll accepted instead of re-reading; store the resident() result in a local in the setInterval callback and print that, and do the same single read at line 331/332 of the burst test.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

The trigger is RSS resting within a few KB below the full - 80 MB boundary when the poll's read at test/js/bun/gc/gc-controller-cadence.test.ts:231 exits; the test is reached by every Linux non-ASAN non-debug CI run. resident() at line 225 truncates kB to MB with >> 10, so one extra page of RSS can move the second read up by a whole unit. The reads at 231 and 232 are in the same process whose RSS is being measured: readFileSync of /proc/self/status, the regex exec and JSON.stringify at line 232 allocate on the main heap between them. On the base branch the same two reads exist; without the PR's strace harness, RSS steps by pages, so the second read can land one MB above the first. The user then gets Expected: < 47, Received: 47 from line 247 even though the poll had observed a value below the limit. Remedy: capture the polled resident() in a local and report that value, not a fresh read.

Verification: Line 225 resident() in test/js/bun/gc/gc-controller-cadence.test.ts truncates kB to MB with >> 10. Line 231 is one read; line 232 now: resident() is a second, independent read, and it is this now that line 247 asserts. Between the two reads the same process whose RSS is measured runs JS, so VmRSS can rise by a page. The base has the same double-read hazard; merging makes nothing worse.

process.exit(0);
}, 100);
Expand Down Expand Up @@ -328,7 +328,7 @@ describe("idle release", () => {
const full = resident();
expect(full).toBeGreaterThan(330);
const deadline = performance.now() + 6000;
while (resident() > full - 250 && performance.now() < deadline) await Bun.sleep(100);
while (resident() >= full - 250 && performance.now() < deadline) await Bun.sleep(100);
expect(resident()).toBeLessThan(full - 250);
proc.stdin.write("exit\n");
await proc.stdin.flush();
Expand Down
Loading