From c7ec9a94c2fe5dc0d9459b3cf981f8c3c79755de Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Wed, 5 Aug 2026 09:31:33 +0200 Subject: [PATCH 1/3] fix(import-map): give the preloader a timer seam so deadlines follow its clock The preloader injected a clock but not its timers, so a caller supplying a virtual clock still had its deadlines measured against wall time. On a loaded CI worker that let a 600s load timeout fire during work that consumed no virtual time at all: the capacity-release test hung for ten minutes and then failed with 'Import-map preloader load timed out', taking a coverage shard and the release with it. It never reproduced in isolation, including under deliberate contention, because starvation is the trigger. setTimer/cancelTimer now pair with now(), defaulting to the host timers. The defaults are wrapped rather than stored directly, since calling them as this.setTimer would hand Deno's timers the preloader as their receiver. The test injects inert timers, so wall-clock starvation can no longer trip it, and awaits through a bounded settleWithin helper so a genuinely missed signal fails in under a second with a label instead of hanging. Verified by suppressing the capacity-release hook: 560ms to a labelled failure, where the same break previously cost ten minutes. --- src/modules/import-map/preloader.test.ts | 78 +++++++++++++++++++++++- src/modules/import-map/preloader.ts | 31 ++++++++-- 2 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/modules/import-map/preloader.test.ts b/src/modules/import-map/preloader.test.ts index 78625e91cc..9a7e1a5ffa 100644 --- a/src/modules/import-map/preloader.test.ts +++ b/src/modules/import-map/preloader.test.ts @@ -68,6 +68,72 @@ async function waitForLoadCount( assertEquals(loads.length, expected); } +/** + * Timer seam that never fires on its own. + * + * Tests that inject a virtual clock must inject timers too. Otherwise the + * preloader's deadline is measured against the wall clock while the rest of the + * test advances by fake ticks, so a starved CI worker can trip a timeout during + * work that consumed no virtual time at all. That turned a millisecond test into + * a ten-minute hang that only reproduced under full-shard load. + */ +function createInertTimers() { + let nextHandle = 1; + const pending = new Map void>(); + return { + setTimer: (callback: () => void, _delayMs: number) => { + const handle = nextHandle++; + pending.set(handle, callback); + return handle as unknown as ReturnType; + }, + cancelTimer: (handle: ReturnType) => { + pending.delete(handle as unknown as number); + }, + /** Fire a pending deadline explicitly when the timeout *is* under test. */ + fireAll: () => { + const callbacks = [...pending.values()]; + pending.clear(); + for (const callback of callbacks) callback(); + }, + }; +} + +/** + * Await `promise` within a bounded number of macrotask turns. + * + * With inert timers a missed signal would hang forever rather than fail, so the + * bound is what keeps a genuine regression fast and legible instead of silent. + */ +async function settleWithin( + promise: Promise, + label: string, + turns = 200, +): Promise { + let timer: ReturnType | undefined; + let cancelled = false; + const deadline = new Promise((_, reject) => { + let turn = 0; + const tick = () => { + if (cancelled) return; + if (++turn >= turns) { + reject(new Error(`${label} did not settle within ${turns} macrotask turns`)); + return; + } + timer = setTimeout(tick, 0); + }; + timer = setTimeout(tick, 0); + }); + try { + return await Promise.race([promise, deadline]); + } finally { + // The chain must be torn down explicitly: a pending tick would outlive the + // test and trip the runner's timer-leak detector. + cancelled = true; + if (timer !== undefined) clearTimeout(timer); + deadline.catch(() => {}); + } +} + describe("modules/import-map/preloader", () => { describe("preloadImportMap", () => { it("should return an import map config", async () => { @@ -1299,11 +1365,14 @@ describe("modules/import-map/preloader", () => { let releaseDuringAdmission = false; let admissionClockReads = 0; let clock = 0; + const timers = createInertTimers(); const preloader = new ImportMapPreloader({ maxProjects: 1, maxVariantsPerProject: 2, ttlMs: 1_000, loadTimeoutMs: TIMEOUT_NOT_UNDER_TEST_MS, + setTimer: timers.setTimer, + cancelTimer: timers.cancelTimer, now: () => { if (releaseDuringAdmission && admissionClockReads++ === 0) { loads[0]!.resolve({ imports: { source: "a" } }); @@ -1332,10 +1401,13 @@ describe("modules/import-map/preloader", () => { await waitForLoadCount(loads, 3); loads[2]!.resolve({ imports: { source: "c" } }); - assertEquals((await first).imports?.source, "a"); - assertEquals((await queued).imports?.source, "c"); + assertEquals((await settleWithin(first, "first preload")).imports?.source, "a"); + assertEquals((await settleWithin(queued, "queued preload")).imports?.source, "c"); loads[1]!.resolve({ imports: { source: "b" } }); - assertEquals((await unrelated).imports?.source, "b"); + assertEquals( + (await settleWithin(unrelated, "unrelated preload")).imports?.source, + "b", + ); }); it("keeps variant identity and capacity deterministic after primordial poisoning", async () => { diff --git a/src/modules/import-map/preloader.ts b/src/modules/import-map/preloader.ts index 158fdb26fe..4a05f1303c 100644 --- a/src/modules/import-map/preloader.ts +++ b/src/modules/import-map/preloader.ts @@ -207,6 +207,15 @@ export interface ImportMapPreloaderOptions { loadTimeoutMs?: number; /** Monotonic-enough clock seam; defaults to Date.now. */ now?: () => number; + /** + * Timer seam, paired with `now`. Defaults to the host timers. + * + * Without this, a caller that injects `now` still has its deadlines measured + * against the wall clock, so a starved process can trip a timeout during work + * that took no virtual time at all. Override both halves together. + */ + setTimer?: (callback: () => void, delayMs: number) => ReturnType; + cancelTimer?: (handle: ReturnType) => void; /** Loader seam for alternate runtimes and deterministic verification. */ loadImportMap?: typeof loadImportMap; } @@ -368,6 +377,11 @@ export class ImportMapPreloader { private readonly ttlMs: number; private readonly loadTimeoutMs: number; private readonly now: () => number; + private readonly setTimer: ( + callback: () => void, + delayMs: number, + ) => ReturnType; + private readonly cancelTimer: (handle: ReturnType) => void; private readonly loader: typeof loadImportMap; constructor(options: ImportMapPreloaderOptions = {}) { @@ -395,6 +409,11 @@ export class ImportMapPreloader { DEFAULT_IMPORT_MAP_LOAD_TIMEOUT_MS, ); this.now = options.now ?? DateNow; + // Wrap rather than store the host timers directly: called as `this.setTimer` + // they would receive the preloader as their receiver, and Deno's timers + // reject any receiver that is not the global object. + this.setTimer = options.setTimer ?? ((callback, delayMs) => SetTimeout(callback, delayMs)); + this.cancelTimer = options.cancelTimer ?? ((handle) => ClearTimeout(handle)); this.loader = options.loadImportMap ?? loadImportMap; } @@ -554,17 +573,17 @@ export class ImportMapPreloader { if (!hasActiveWork) return resolvedPromise(); let timeoutId: ReturnType | undefined; const timeout = new IntrinsicPromise((_, reject) => { - timeoutId = SetTimeout(() => { + timeoutId = this.setTimer(() => { reject(new IntrinsicRangeError("Import-map preloader capacity wait timed out")); }, timeoutMs); }); return promiseThen( raceTwo(capacityChange, timeout), () => { - if (timeoutId !== undefined) ClearTimeout(timeoutId); + if (timeoutId !== undefined) this.cancelTimer(timeoutId); }, (error) => { - if (timeoutId !== undefined) ClearTimeout(timeoutId); + if (timeoutId !== undefined) this.cancelTimer(timeoutId); throw error; }, ); @@ -824,7 +843,7 @@ export class ImportMapPreloader { this.trackActiveLoad(loaderPromise); let timeoutId: ReturnType | undefined; const timeoutPromise = new IntrinsicPromise((_, reject) => { - timeoutId = SetTimeout(() => { + timeoutId = this.setTimer(() => { if (setDelete(this.activeLoads, loaderPromise)) { this.trackOrphanedLoad(cacheKey, loaderPromise); this.notifyCapacityChange(); @@ -836,11 +855,11 @@ export class ImportMapPreloader { const boundedLoaderPromise = promiseThen( raceTwo(loaderPromise, timeoutPromise), (value) => { - if (timeoutId !== undefined) ClearTimeout(timeoutId); + if (timeoutId !== undefined) this.cancelTimer(timeoutId); return value; }, (error) => { - if (timeoutId !== undefined) ClearTimeout(timeoutId); + if (timeoutId !== undefined) this.cancelTimer(timeoutId); throw error; }, ); From 061ac3ff2e7bfd09e595542928929c8505616a38 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Wed, 5 Aug 2026 09:42:10 +0200 Subject: [PATCH 2/3] fix(import-map): wrap caller timers and put the capacity deadline on a seam Review feedback on #3377. Caller-supplied timers were stored raw while only the defaults were wrapped, so setTimer: setTimeout would have thrown Illegal invocation at the first load, which is the same trap the defaults were wrapped to avoid. Every timer is now wrapped. The capacity-retry deadline still read performance.now directly, so an injected clock did not control it: the same defect this pull request set out to fix, one call site over. It now goes through a monotonicNow seam. That seam is deliberately separate from now() rather than folded into it as suggested. now() defaults to Date.now and carries absolute time for TTL and expiry; NTP can move it backwards, and deadline arithmetic needs a monotonic source. Sharing one seam would either break expiry semantics or make timeouts clock-skew sensitive. Covered by a test that freezes the injected monotonic clock while host time runs: a capacity-blocked caller must never reach its deadline, and is released by capacity freeing rather than by time passing. Verified to discriminate by reverting the seam, which turns it red. --- src/modules/import-map/preloader.test.ts | 64 ++++++++++++++++++++++++ src/modules/import-map/preloader.ts | 32 +++++++++--- 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/modules/import-map/preloader.test.ts b/src/modules/import-map/preloader.test.ts index 9a7e1a5ffa..12affe70c1 100644 --- a/src/modules/import-map/preloader.test.ts +++ b/src/modules/import-map/preloader.test.ts @@ -1410,6 +1410,70 @@ describe("modules/import-map/preloader", () => { ); }); + it("measures the capacity deadline on the injected monotonic clock", async () => { + // The injected monotonic clock is frozen while host time keeps running. + // A capacity-blocked caller must therefore never reach its deadline: if + // the retry still consulted performance.now, real time would sail past + // loadTimeoutMs and reject it. Keeping this seam separate from `now` also + // matters, because `now` defaults to Date.now, which NTP can move + // backwards, and deadline arithmetic needs a monotonic source. + const adapter = createMinimalAdapter(); + const loads: Array>> = []; + const timers = createInertTimers(); + const preloader = new ImportMapPreloader({ + maxProjects: 1, + maxVariantsPerProject: 1, + ttlMs: TIMEOUT_NOT_UNDER_TEST_MS, + loadTimeoutMs: 50, + setTimer: timers.setTimer, + cancelTimer: timers.cancelTimer, + monotonicNow: () => 1_000, + now: () => 1, + loadImportMap: () => { + const load = createDeferred(); + loads.push(load); + return load.promise; + }, + }); + + const occupying = preloader.preload("/project", adapter, "project", { + contentSourceId: "source-a", + }); + await waitForLoadCount(loads, 1); + + const blocked = preloader.preload("/project", adapter, "project", { + contentSourceId: "source-b", + }); + + let settledEarly = false; + const observed = blocked.then( + () => { + settledEarly = true; + }, + () => { + settledEarly = true; + }, + ); + for (let turn = 0; turn < 50; turn++) { + await new Promise((resolve) => setTimeout(resolve, 0)); + } + assertEquals(settledEarly, false); + + // Releasing capacity, not the passage of host time, is what lets it run. + loads[0]!.resolve({ imports: { source: "a" } }); + assertEquals( + (await settleWithin(occupying, "occupying preload")).imports?.source, + "a", + ); + await waitForLoadCount(loads, 2); + loads[1]!.resolve({ imports: { source: "b" } }); + assertEquals( + (await settleWithin(blocked, "capacity-blocked preload")).imports?.source, + "b", + ); + await observed; + }); + it("keeps variant identity and capacity deterministic after primordial poisoning", async () => { const worker = new Worker( new URL("./preloader-primordial-poisoning.worker.ts", import.meta.url), diff --git a/src/modules/import-map/preloader.ts b/src/modules/import-map/preloader.ts index 4a05f1303c..23f4ad4632 100644 --- a/src/modules/import-map/preloader.ts +++ b/src/modules/import-map/preloader.ts @@ -216,6 +216,17 @@ export interface ImportMapPreloaderOptions { */ setTimer?: (callback: () => void, delayMs: number) => ReturnType; cancelTimer?: (handle: ReturnType) => void; + /** + * Elapsed-time seam for the capacity-retry deadline; defaults to + * performance.now. + * + * Deliberately separate from `now`: that clock carries absolute time for TTL + * and expiry, and defaults to Date.now, which can jump backwards under NTP + * correction. Deadline arithmetic needs a monotonic source, so the two cannot + * share one seam without either breaking expiry semantics or making timeouts + * clock-skew sensitive. + */ + monotonicNow?: () => number; /** Loader seam for alternate runtimes and deterministic verification. */ loadImportMap?: typeof loadImportMap; } @@ -382,6 +393,7 @@ export class ImportMapPreloader { delayMs: number, ) => ReturnType; private readonly cancelTimer: (handle: ReturnType) => void; + private readonly monotonicNow: () => number; private readonly loader: typeof loadImportMap; constructor(options: ImportMapPreloaderOptions = {}) { @@ -409,11 +421,17 @@ export class ImportMapPreloader { DEFAULT_IMPORT_MAP_LOAD_TIMEOUT_MS, ); this.now = options.now ?? DateNow; - // Wrap rather than store the host timers directly: called as `this.setTimer` - // they would receive the preloader as their receiver, and Deno's timers - // reject any receiver that is not the global object. - this.setTimer = options.setTimer ?? ((callback, delayMs) => SetTimeout(callback, delayMs)); - this.cancelTimer = options.cancelTimer ?? ((handle) => ClearTimeout(handle)); + // Wrap every timer, caller-supplied ones included, rather than storing them + // directly. Invoked as `this.setTimer` a stored function receives the + // preloader as its receiver, and Deno's timers reject any receiver that is + // not the global object, so `setTimer: setTimeout` would throw + // `Illegal invocation` at the first load. + const setTimer = options.setTimer ?? SetTimeout; + const cancelTimer = options.cancelTimer ?? ClearTimeout; + const monotonic = options.monotonicNow ?? monotonicNow; + this.setTimer = (callback, delayMs) => setTimer(callback, delayMs); + this.cancelTimer = (handle) => cancelTimer(handle); + this.monotonicNow = () => monotonic(); this.loader = options.loadImportMap ?? loadImportMap; } @@ -876,14 +894,14 @@ export class ImportMapPreloader { projectId?: string, context?: PreloadImportMapContext, ): Promise { - const capacityDeadline = monotonicNow() + this.loadTimeoutMs; + const capacityDeadline = this.monotonicNow() + this.loadTimeoutMs; for (;;) { const capacityChange = this.capacityChange.promise; try { return await this.preloadOnce(projectDir, adapter, projectId, context); } catch (error) { if (!this.isCapacityError(error)) throw error; - const remainingMs = capacityDeadline - monotonicNow(); + const remainingMs = capacityDeadline - this.monotonicNow(); if (remainingMs <= 0) throw error; await this.waitForActiveWork(capacityChange, remainingMs); } From d1e9a4b943e28044c2fa3ddd0bfb936e873d30dc Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Wed, 5 Aug 2026 09:46:38 +0200 Subject: [PATCH 3/3] test(import-map): prove host time passes the deadline in the monotonic test Review feedback on #3377. The test waited a fixed 50 event-loop turns and asserted the caller had not settled, but never proved 50ms of host time had elapsed. If the turns ran fast, a retry still reading performance.now would also be inside its deadline, so the test could pass with the bug present. It now waits on the real clock and asserts the host deadline was crossed before capacity is released. Yielding on timed sleeps rather than a turn budget avoids the inverse flake: a fixed turn cap can expire before the host deadline on a fast machine and fail an otherwise correct run. Still verified to discriminate: reverting the seam turns it red. --- src/modules/import-map/preloader.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/modules/import-map/preloader.test.ts b/src/modules/import-map/preloader.test.ts index 12affe70c1..d6e9ec99d9 100644 --- a/src/modules/import-map/preloader.test.ts +++ b/src/modules/import-map/preloader.test.ts @@ -1454,9 +1454,16 @@ describe("modules/import-map/preloader", () => { settledEarly = true; }, ); - for (let turn = 0; turn < 50; turn++) { - await new Promise((resolve) => setTimeout(resolve, 0)); + // Host time must genuinely pass loadTimeoutMs, otherwise this proves + // nothing: a retry still reading performance.now would also be inside its + // deadline, and the test would pass with the bug present. Yield on timed + // sleeps rather than a fixed turn count, so the wait is bounded by the + // real clock advancing instead of by a turn budget that can run out first. + const hostDeadline = performance.now() + 50; + while (performance.now() < hostDeadline) { + await new Promise((resolve) => setTimeout(resolve, 5)); } + assertEquals(performance.now() >= hostDeadline, true); assertEquals(settledEarly, false); // Releasing capacity, not the passage of host time, is what lets it run.