From b4e6a48c6b0deadb60ae8a9df49196c1982785b9 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 16:21:09 +0000 Subject: [PATCH 1/3] test(timers): speed up setInterval.test.js and tighten its assertions The file was one of the slowest in the suite (24s on debian 13 x64-asan in CI, and the leak test actually times out at 30s on a local debug+ASAN build). The work was dominated by the leak fixture (350 batches of 1,000 timers with 10,000 ticks each) and four serial spawnSync subprocesses. Fixture changes (coverage preserved): - setInterval-leak-fixture.js: 50+300 batches of 1,000 timers -> 3+20 batches of 200 timers; attach a 32 KiB Buffer per timer instead of a ~200 byte string so a real leak is still >100 MB over baseline while the no-leak delta stays <20 MB. The protectedObjectTypeCounts.Timeout check is unchanged. Also detect ASAN via bun:internal-for-testing (the old execPath name check was false on bun-debug even though it is ASAN-instrumented). - setinterval-cancel-fixture.js: 50,000 -> 5,000 timers; 1M-element arg array -> 100k. A single stray callback after clearInterval still fails the test. - setInterval-fixture.js: 100 -> 25 ticks at 16ms; drop the per-tick log line, print PASS on success and the offending gap on failure. Test file changes: - Convert the four subprocess tests from the sync .toRun() matcher to it.concurrent with Bun.spawn, and assert stderr/stdout before exitCode. - Fix the 'refreshed setInterval should not reschedule again' test: it returned immediately without awaiting the interval, so its assertions never ran. It now awaits three fires and uses expect() instead of throwing from the callback. - Fix the 'clearInterval' test to clear its second interval instead of leaving it running for the rest of the file. - Add an explicit expect(remaining).toBe(0) to the async setInterval test. Local bun bd test timings (debug+ASAN, Linux x64): before: 53-57s (leak test times out at 30s) after: 8-11s, 8/8 pass over 10 consecutive runs --- test/js/web/timers/setInterval-fixture.js | 5 +- .../js/web/timers/setInterval-leak-fixture.js | 37 +++-- test/js/web/timers/setInterval.test.js | 149 ++++++++++++------ .../web/timers/setinterval-cancel-fixture.js | 5 +- 4 files changed, 121 insertions(+), 75 deletions(-) diff --git a/test/js/web/timers/setInterval-fixture.js b/test/js/web/timers/setInterval-fixture.js index f9da0126868a..38636042b91c 100644 --- a/test/js/web/timers/setInterval-fixture.js +++ b/test/js/web/timers/setInterval-fixture.js @@ -1,15 +1,16 @@ var lastCall = performance.now(); const delta = 16; -let tries = 100; +let tries = 25; setInterval(() => { const now = performance.now(); - console.log((now - lastCall) | 0, "ms since the last call"); if (now - lastCall < ((delta / 2) | 0)) { + console.error("fired after only", (now - lastCall) | 0, "ms (expected >=", delta, "ms)"); process.exit(1); } lastCall = now; if (--tries === 0) { + console.log("PASS"); process.exit(0); } }, delta); diff --git a/test/js/web/timers/setInterval-leak-fixture.js b/test/js/web/timers/setInterval-leak-fixture.js index d0b383523866..e54e978bcfe1 100644 --- a/test/js/web/timers/setInterval-leak-fixture.js +++ b/test/js/web/timers/setInterval-leak-fixture.js @@ -1,23 +1,20 @@ const delta = 1; -const initialRuns = 10_000; +const initialRuns = 1_000; let runs = initialRuns; // ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run far higher under bun-asan; widen the threshold to avoid false positives. -const isASAN = process.execPath.includes("bun-asan"); +// run higher under ASAN; widen the threshold to avoid false positives. Detect +// ASAN from the runtime (the debug build is ASAN-instrumented but named +// `bun-debug`, so the name check alone is wrong for local runs). +let isASAN = false; +try { + isASAN = require("bun:internal-for-testing").isASANEnabled(); +} catch {} +isASAN ||= process.execPath.includes("-asan"); function usage() { return process.memoryUsage.rss(); } -Promise.withResolvers ??= () => { - let promise, resolve, reject; - promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - return { promise, resolve, reject }; -}; - function gc() { if (typeof Bun !== "undefined") { Bun.gc(true); @@ -35,7 +32,7 @@ function iterate() { huge: { wow: { big: { - data: runs.toString().repeat(50), + data: Buffer.alloc(32 * 1024, runs & 0xff), }, }, }, @@ -66,15 +63,15 @@ async function batch(iterations) { { // Warmup - for (let i = 0; i < 50; i++) { - await batch(1_000); + for (let i = 0; i < 3; i++) { + await batch(200); } // Measure memory usage after the warmup const initial = usage(); - // Run batch 300 more times, each time creating 1,000 timers, waiting for them to finish, and + // Run batch 20 more times, each time creating 200 timers, waiting for them to finish, and // clearing them. - for (let i = 0; i < 300; i++) { - await batch(1_000); + for (let i = 0; i < 20; i++) { + await batch(200); } // Measure memory usage again, to check that cleared timers and the objects allocated inside each // callback have not bloated it @@ -92,7 +89,9 @@ async function batch(iterations) { } } - if (delta > (isASAN ? 256 : 20)) { + // With a 32 KiB payload pinned on each timer, a leaked batch of timers costs ~6 MB; 20 batches + // would add well over 100 MB, so both thresholds are comfortably below the leak signal. + if (delta > (isASAN ? 50 : 20)) { throw new Error("Memory leak detected"); } } diff --git a/test/js/web/timers/setInterval.test.js b/test/js/web/timers/setInterval.test.js index 1dc6b7c98fcf..77e32ada992d 100644 --- a/test/js/web/timers/setInterval.test.js +++ b/test/js/web/timers/setInterval.test.js @@ -1,5 +1,5 @@ import { expect, it } from "bun:test"; -import { isWindows } from "harness"; +import { bunEnv, bunExe, isWindows } from "harness"; import { join } from "path"; it("setInterval", async () => { @@ -28,7 +28,7 @@ it("setInterval", async () => { }); expect(result).toBe(10); - expect(performance.now() - start > 9).toBe(true); + expect(performance.now() - start).toBeGreaterThanOrEqual(9); }); it("clearInterval", async () => { @@ -38,8 +38,9 @@ it("clearInterval", async () => { expect.unreachable(); }, 1); clearInterval(id); - await new Promise((resolve, reject) => { - setInterval(() => { + await new Promise(resolve => { + const id2 = setInterval(() => { + clearInterval(id2); resolve(); }, 10); }); @@ -60,70 +61,116 @@ it("async setInterval", async () => { }, 1); }); }); + expect(remaining).toBe(0); }); it("refreshed setInterval should not reschedule again", async () => { let relative = performance.now(); let runCount = 0; - let timer = setInterval(() => { - let end = performance.now(); - - // loop for 100 - const spinloop = end; - while (performance.now() - spinloop < 100) { - end = performance.now(); - } - - timer.refresh(); - - const elapsed = Math.round(end - relative); - console.log("Time since last run", elapsed); - - runCount++; + await new Promise((resolve, reject) => { + const timer = setInterval(() => { + let end = performance.now(); - switch (runCount) { - case 1: { - if (elapsed < 180) { - throw new Error("Expected elapsed time to be greater than 180"); - } - break; + // spin for 100ms so the next scheduled tick is already due by the time we return + const spinloop = end; + while (performance.now() - spinloop < 100) { + end = performance.now(); } - case 3: - case 2: { - if (elapsed > 180) { - throw new Error("Expected elapsed time to be less than 180"); + + timer.refresh(); + + const elapsed = Math.round(end - relative); + runCount++; + + try { + switch (runCount) { + case 1: + // initial 100ms delay + 100ms spinloop + expect(elapsed).toBeGreaterThanOrEqual(180); + break; + case 2: + case 3: + // refresh() inside the callback must not push the next tick out: since the + // spinloop already consumed the interval, the next fire happens immediately + expect(elapsed).toBeLessThan(180); + break; } - break; + } catch (err) { + clearInterval(timer); + reject(err); + return; } - } - relative = end; + relative = end; - if (runCount === 3) { - clearInterval(timer); - } - }, 100); + if (runCount === 3) { + clearInterval(timer); + resolve(); + } + }, 100); + }); + expect(runCount).toBe(3); }); -it("setInterval runs with at least the delay time", () => { - expect([`run`, join(import.meta.dir, "setInterval-fixture.js")]).toRun(); -}); +async function runFixture(args) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env: bunEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +it.concurrent( + "setInterval runs with at least the delay time", + async () => { + const { stdout, stderr, exitCode } = await runFixture(["run", join(import.meta.dir, "setInterval-fixture.js")]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("PASS"); + expect(exitCode).toBe(0); + }, + 30_000, +); -it("setInterval canceling with unref, close, _idleTimeout, and _onTimeout", () => { - expect([join(import.meta.dir, "timers-fixture-unref.js"), "setInterval"]).toRun(); -}); +it.concurrent( + "setInterval canceling with unref, close, _idleTimeout, and _onTimeout", + async () => { + const { stdout, stderr, exitCode } = await runFixture([ + join(import.meta.dir, "timers-fixture-unref.js"), + "setInterval", + ]); + expect(stderr).toBe(""); + expect(stdout).toBe(""); + expect(exitCode).toBe(0); + }, + 30_000, +); -it( +it.concurrent( "setInterval doesn't leak memory", - () => { - expect([`run`, join(import.meta.dir, "setInterval-leak-fixture.js")]).toRun(); + async () => { + const { stdout, stderr, exitCode } = await runFixture([ + "run", + join(import.meta.dir, "setInterval-leak-fixture.js"), + ]); + expect(stderr).toBe(""); + expect(stdout).toMatch(/^RSS \d+ MB\nDelta -?\d+ MB\nTimeout object count: \d+\n$/); + expect(exitCode).toBe(0); }, !isWindows ? 30_000 : 90_000, ); -// ✓ setInterval doesn't leak memory [9930.00ms] -// ✓ setInterval doesn't leak memory [80188.00ms] -// TODO: investigate this discrepancy further -it("setInterval doesn't run when cancelled after being scheduled", () => { - expect([`run`, join(import.meta.dir, "setInterval-cancel-fixture.js")]).toRun(); -}, 30_000); +it.concurrent( + "setInterval doesn't run when cancelled after being scheduled", + async () => { + const { stdout, stderr, exitCode } = await runFixture([ + "run", + join(import.meta.dir, "setinterval-cancel-fixture.js"), + ]); + expect(stderr).toBe(""); + expect(stdout).toMatch(/^RSS: \d+ MB\n$/); + expect(exitCode).toBe(0); + }, + 30_000, +); diff --git a/test/js/web/timers/setinterval-cancel-fixture.js b/test/js/web/timers/setinterval-cancel-fixture.js index 4c5d58939ed3..64ad4901fa83 100644 --- a/test/js/web/timers/setinterval-cancel-fixture.js +++ b/test/js/web/timers/setinterval-cancel-fixture.js @@ -1,9 +1,8 @@ -const huge = Array.from({ length: 1000000 }, () => 0); -huge.fill(0); +const huge = new Array(100_000).fill(0); let hasRun = false; const gc = typeof Bun !== "undefined" ? Bun.gc : typeof globalThis.gc !== "undefined" ? globalThis.gc : () => {}; -var timers = new Array(50_000); +var timers = new Array(5_000); function fn(huge) { if (hasRun) { From 4632142a5f456f1670ddc86dbcf9c49fff4c133e Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 25 Jul 2026 18:21:44 +0000 Subject: [PATCH 2/3] test(timers): address review on setInterval.test.js speedup - setInterval-leak-fixture.js: assert on heapStats.objectTypeCounts.Timeout (201 in the no-leak case, thousands on leak) as the primary retention check. The RSS check becomes a backstop with a single 50 MB threshold, so the isASAN detection and dual threshold are no longer needed. - setInterval-fixture.js: check each tick against its scheduled time (N*delta from start) instead of the gap since the previous tick. If the event loop stalls past the interval, the catch-up tick legitimately fires with a near-zero gap; the old check reported that as an early fire. This was a pre-existing flake on main, not introduced here. - setInterval.test.js: drop the explicit 30s timeouts on the delay and cancel subprocess tests (both <2s), drop the Windows-only 90s leak timeout and the isWindows import (the 80s Windows runtime it guarded against no longer exists), and keep 30s only on the leak and unref fixtures which approach the default 5s under debug+ASAN with concurrent subprocesses. --- test/js/web/timers/setInterval-fixture.js | 15 ++++--- .../js/web/timers/setInterval-leak-fixture.js | 23 +++++----- test/js/web/timers/setInterval.test.js | 44 ++++++++----------- 3 files changed, 39 insertions(+), 43 deletions(-) diff --git a/test/js/web/timers/setInterval-fixture.js b/test/js/web/timers/setInterval-fixture.js index 38636042b91c..022860617cd7 100644 --- a/test/js/web/timers/setInterval-fixture.js +++ b/test/js/web/timers/setInterval-fixture.js @@ -1,13 +1,18 @@ -var lastCall = performance.now(); +const start = performance.now(); const delta = 16; -let tries = 25; +const total = 25; +let tries = total; setInterval(() => { const now = performance.now(); - if (now - lastCall < ((delta / 2) | 0)) { - console.error("fired after only", (now - lastCall) | 0, "ms (expected >=", delta, "ms)"); + // The Nth tick is not allowed to fire before N*delta ms have elapsed. Checking against the + // previous tick is wrong: if the event loop stalls for >delta, the catch-up tick correctly + // fires immediately and the gap between ticks can legitimately be ~0ms. + const ticks = total - tries + 1; + const earliest = ticks * delta; + if (now - start < earliest - 2) { + console.error("tick", ticks, "fired at", (now - start) | 0, "ms (expected >=", earliest, "ms)"); process.exit(1); } - lastCall = now; if (--tries === 0) { console.log("PASS"); diff --git a/test/js/web/timers/setInterval-leak-fixture.js b/test/js/web/timers/setInterval-leak-fixture.js index e54e978bcfe1..20702387eafa 100644 --- a/test/js/web/timers/setInterval-leak-fixture.js +++ b/test/js/web/timers/setInterval-leak-fixture.js @@ -1,15 +1,6 @@ const delta = 1; const initialRuns = 1_000; let runs = initialRuns; -// ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run higher under ASAN; widen the threshold to avoid false positives. Detect -// ASAN from the runtime (the debug build is ASAN-instrumented but named -// `bun-debug`, so the name check alone is wrong for local runs). -let isASAN = false; -try { - isASAN = require("bun:internal-for-testing").isASANEnabled(); -} catch {} -isASAN ||= process.execPath.includes("-asan"); function usage() { return process.memoryUsage.rss(); @@ -83,15 +74,21 @@ async function batch(iterations) { if (globalThis.Bun) { const heapStats = require("bun:jsc").heapStats(); - console.log("Timeout object count:", heapStats.objectTypeCounts.Timeout || 0); + const timeoutCount = heapStats.objectTypeCounts.Timeout || 0; + console.log("Timeout object count:", timeoutCount); if (heapStats.protectedObjectTypeCounts.Timeout) { throw new Error("Expected 0 protected Timeout but received " + heapStats.protectedObjectTypeCounts.Timeout); } + // One batch (200 timers) is live until the next GC; anything much larger means cleared + // timers from earlier batches are being retained. + if (timeoutCount > 500) { + throw new Error("Expected <= 500 live Timeout objects but received " + timeoutCount); + } } - // With a 32 KiB payload pinned on each timer, a leaked batch of timers costs ~6 MB; 20 batches - // would add well over 100 MB, so both thresholds are comfortably below the leak signal. - if (delta > (isASAN ? 50 : 20)) { + // With a 32 KiB payload pinned on each timer, 20 leaked batches would add well over 100 MB, + // so this backstop is well below the leak signal and well above allocator/heap growth noise. + if (delta > 50) { throw new Error("Memory leak detected"); } } diff --git a/test/js/web/timers/setInterval.test.js b/test/js/web/timers/setInterval.test.js index 77e32ada992d..554cec2484dc 100644 --- a/test/js/web/timers/setInterval.test.js +++ b/test/js/web/timers/setInterval.test.js @@ -1,5 +1,5 @@ import { expect, it } from "bun:test"; -import { bunEnv, bunExe, isWindows } from "harness"; +import { bunEnv, bunExe } from "harness"; import { join } from "path"; it("setInterval", async () => { @@ -122,16 +122,22 @@ async function runFixture(args) { return { stdout, stderr, exitCode }; } -it.concurrent( - "setInterval runs with at least the delay time", - async () => { - const { stdout, stderr, exitCode } = await runFixture(["run", join(import.meta.dir, "setInterval-fixture.js")]); - expect(stderr).toBe(""); - expect(stdout.trim()).toBe("PASS"); - expect(exitCode).toBe(0); - }, - 30_000, -); +it.concurrent("setInterval runs with at least the delay time", async () => { + const { stdout, stderr, exitCode } = await runFixture(["run", join(import.meta.dir, "setInterval-fixture.js")]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("PASS"); + expect(exitCode).toBe(0); +}); + +it.concurrent("setInterval doesn't run when cancelled after being scheduled", async () => { + const { stdout, stderr, exitCode } = await runFixture([ + "run", + join(import.meta.dir, "setinterval-cancel-fixture.js"), + ]); + expect(stderr).toBe(""); + expect(stdout).toMatch(/^RSS: \d+ MB\n$/); + expect(exitCode).toBe(0); +}); it.concurrent( "setInterval canceling with unref, close, _idleTimeout, and _onTimeout", @@ -144,6 +150,8 @@ it.concurrent( expect(stdout).toBe(""); expect(exitCode).toBe(0); }, + // the fixture spends ~2s under debug+ASAN just loading node/test/common; with the other + // fixtures running concurrently it can exceed the default 5s 30_000, ); @@ -158,19 +166,5 @@ it.concurrent( expect(stdout).toMatch(/^RSS \d+ MB\nDelta -?\d+ MB\nTimeout object count: \d+\n$/); expect(exitCode).toBe(0); }, - !isWindows ? 30_000 : 90_000, -); - -it.concurrent( - "setInterval doesn't run when cancelled after being scheduled", - async () => { - const { stdout, stderr, exitCode } = await runFixture([ - "run", - join(import.meta.dir, "setinterval-cancel-fixture.js"), - ]); - expect(stderr).toBe(""); - expect(stdout).toMatch(/^RSS: \d+ MB\n$/); - expect(exitCode).toBe(0); - }, 30_000, ); From a4e16ece51a65ce007c3b8ce2a0cf1652e19716f Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Jul 2026 01:14:59 +0000 Subject: [PATCH 3/3] ci: retrigger