diff --git a/src/modules/import-map/preloader.test.ts b/src/modules/import-map/preloader.test.ts index 78625e91cc..d6e9ec99d9 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,84 @@ 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("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; + }, + ); + // 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. + 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 () => { diff --git a/src/modules/import-map/preloader.ts b/src/modules/import-map/preloader.ts index 158fdb26fe..23f4ad4632 100644 --- a/src/modules/import-map/preloader.ts +++ b/src/modules/import-map/preloader.ts @@ -207,6 +207,26 @@ 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; + /** + * 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; } @@ -368,6 +388,12 @@ 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 monotonicNow: () => number; private readonly loader: typeof loadImportMap; constructor(options: ImportMapPreloaderOptions = {}) { @@ -395,6 +421,17 @@ export class ImportMapPreloader { DEFAULT_IMPORT_MAP_LOAD_TIMEOUT_MS, ); this.now = options.now ?? DateNow; + // 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; } @@ -554,17 +591,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 +861,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 +873,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; }, ); @@ -857,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); }