diff --git a/test/js/web/timers/setInterval-fixture.js b/test/js/web/timers/setInterval-fixture.js index f9da0126868a..022860617cd7 100644 --- a/test/js/web/timers/setInterval-fixture.js +++ b/test/js/web/timers/setInterval-fixture.js @@ -1,15 +1,21 @@ -var lastCall = performance.now(); +const start = performance.now(); const delta = 16; -let tries = 100; +const total = 25; +let tries = total; setInterval(() => { const now = performance.now(); - console.log((now - lastCall) | 0, "ms since the last call"); - if (now - lastCall < ((delta / 2) | 0)) { + // 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"); 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..20702387eafa 100644 --- a/test/js/web/timers/setInterval-leak-fixture.js +++ b/test/js/web/timers/setInterval-leak-fixture.js @@ -1,23 +1,11 @@ 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"); 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 +23,7 @@ function iterate() { huge: { wow: { big: { - data: runs.toString().repeat(50), + data: Buffer.alloc(32 * 1024, runs & 0xff), }, }, }, @@ -66,15 +54,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 @@ -86,13 +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); + } } - if (delta > (isASAN ? 256 : 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 1dc6b7c98fcf..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 { isWindows } from "harness"; +import { bunEnv, bunExe } 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,110 @@ 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); }); -it("setInterval canceling with unref, close, _idleTimeout, and _onTimeout", () => { - expect([join(import.meta.dir, "timers-fixture-unref.js"), "setInterval"]).toRun(); +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( - "setInterval doesn't leak memory", - () => { - expect([`run`, join(import.meta.dir, "setInterval-leak-fixture.js")]).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); }, - !isWindows ? 30_000 : 90_000, + // 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, ); -// ✓ 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 leak memory", + 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); + }, + 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) {