Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
149 changes: 146 additions & 3 deletions src/modules/import-map/preloader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number, () => void>();
return {
setTimer: (callback: () => void, _delayMs: number) => {
const handle = nextHandle++;
pending.set(handle, callback);
return handle as unknown as ReturnType<typeof setTimeout>;
},
cancelTimer: (handle: ReturnType<typeof setTimeout>) => {
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<T>(
promise: Promise<T>,
label: string,
turns = 200,
): Promise<T> {
let timer: ReturnType<typeof setTimeout> | undefined;
let cancelled = false;
const deadline = new Promise<never>((_, 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 () => {
Expand Down Expand Up @@ -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" } });
Expand Down Expand Up @@ -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<ReturnType<typeof createDeferred<ImportMapConfig>>> = [];
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<ImportMapConfig>();
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<void>((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 () => {
Expand Down
53 changes: 45 additions & 8 deletions src/modules/import-map/preloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof setTimeout>;
cancelTimer?: (handle: ReturnType<typeof setTimeout>) => 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;
}
Expand Down Expand Up @@ -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<typeof setTimeout>;
private readonly cancelTimer: (handle: ReturnType<typeof setTimeout>) => void;
private readonly monotonicNow: () => number;
private readonly loader: typeof loadImportMap;

constructor(options: ImportMapPreloaderOptions = {}) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -554,17 +591,17 @@ export class ImportMapPreloader {
if (!hasActiveWork) return resolvedPromise();
let timeoutId: ReturnType<typeof setTimeout> | undefined;
const timeout = new IntrinsicPromise<void>((_, 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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
throw error;
},
);
Expand Down Expand Up @@ -824,7 +861,7 @@ export class ImportMapPreloader {
this.trackActiveLoad(loaderPromise);
let timeoutId: ReturnType<typeof setTimeout> | undefined;
const timeoutPromise = new IntrinsicPromise<ImportMapConfig>((_, reject) => {
timeoutId = SetTimeout(() => {
timeoutId = this.setTimer(() => {
if (setDelete(this.activeLoads, loaderPromise)) {
this.trackOrphanedLoad(cacheKey, loaderPromise);
this.notifyCapacityChange();
Expand All @@ -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;
},
);
Expand All @@ -857,14 +894,14 @@ export class ImportMapPreloader {
projectId?: string,
context?: PreloadImportMapContext,
): Promise<ImportMapConfig> {
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);
}
Expand Down