From b40b4741b34d7ad3551259128da5d8a7e91b61ed Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 3 Oct 2025 14:52:35 +0100 Subject: [PATCH] fix(clock): make sure runTo is atomic Since `runTo` is asynchronous, we might end up in a reentrant call, for example when `updateRealTimeTimer` timeout happens between two tasks in `runTo`. This yields to breaking all invariants and results in `now` being reset to some old value. To prevent this, introduce a semaphore and only allow a single `runTo` at a time. Additionally, ensure we never rewind time when running our tests. --- packages/injected/src/clock.ts | 143 +++++++++++++++++++++++-------- tests/library/unit/clock.spec.ts | 38 ++++++++ 2 files changed, 147 insertions(+), 34 deletions(-) diff --git a/packages/injected/src/clock.ts b/packages/injected/src/clock.ts index 753996a36e9ff..792959739ca74 100644 --- a/packages/injected/src/clock.ts +++ b/packages/injected/src/clock.ts @@ -59,6 +59,26 @@ type Time = { type LogEntryType = 'fastForward' |'install' | 'pauseAt' | 'resume' | 'runFor' | 'setFixedTime' | 'setSystemTime'; +class Semaphore { + private _queue: (() => void)[] | undefined; + + acquire() { + if (!this._queue) { + this._queue = []; + return Promise.resolve(); + } + return new Promise(f => this._queue!.push(f)); + } + + release() { + const next = this._queue?.shift(); + if (next) + next(); + else + this._queue = undefined; + } +} + export class ClockController { readonly _now: Time; private _duringTick = false; @@ -69,11 +89,36 @@ export class ClockController { private _log: { type: LogEntryType, time: number, param?: number }[] = []; private _realTime: { startTicks: EmbedderTicks, lastSyncTicks: EmbedderTicks } | undefined; private _currentRealTimeTimer: { callAt: Ticks, dispose: () => void } | undefined; + private _runToSemaphore: Semaphore; + private _strictMode = false; constructor(embedder: Embedder) { this._timers = new Map(); this._now = { time: asWallTime(0), isFixedTime: false, ticks: 0 as Ticks, origin: asWallTime(-1) }; this._embedder = embedder; + this._runToSemaphore = new Semaphore(); + } + + debugDump(title?: string) { + const lines = [ + `=== ${title || 'Clock dump'} ===`, + `Now: ${this._now.ticks} ticks${this._now.isFixedTime ? ' [fixed]' : ''} date=${this._now.time}`, + `Timers:`, + ...Array.from(this._timers.values()).map(timer => { + return ` ${timer.type} created at ${timer.createdAt}, call at ${timer.callAt}, delay ${timer.delay}`; + }), + ]; + if (this._realTime) + lines.push(`Real time: started at ${this._realTime.startTicks}, last sync at ${this._realTime.lastSyncTicks}`); + if (this._currentRealTimeTimer) + lines.push(`Next real time sync at: ${this._currentRealTimeTimer.callAt}`); + lines.push(''); + // eslint-disable-next-line no-console + console.log(lines.join('\n')); + } + + setStrictModeForTests() { + this._strictMode = true; } uninstall() { @@ -83,7 +128,8 @@ export class ClockController { now(): number { this._replayLogOnce(); - this._syncRealTime(); + // Sync real time to support calling Date.now() in a loop. + this._advanceNow(this._syncRealTime() ?? this._now.ticks); return this._now.time; } @@ -104,18 +150,19 @@ export class ClockController { performanceNow(): DOMHighResTimeStamp { this._replayLogOnce(); - this._syncRealTime(); + // Sync real time to support calling performance.now() in a loop. + this._advanceNow(this._syncRealTime() ?? this._now.ticks); return this._now.ticks; } - private _syncRealTime() { + private _syncRealTime(): Ticks | undefined { if (!this._realTime) return; const now = this._embedder.performanceNow(); const sinceLastSync = now - this._realTime.lastSyncTicks; if (sinceLastSync > 0) { - this._advanceNow(shiftTicks(this._now.ticks, sinceLastSync)); this._realTime.lastSyncTicks = now; + return shiftTicks(this._now.ticks, sinceLastSync); } } @@ -132,6 +179,11 @@ export class ClockController { } private _advanceNow(to: Ticks) { + if (this._now.ticks > to) { + if (this._strictMode) + throw new Error(`Advancing to the past in strict mode ${this._now.ticks} -> ${to}`); + return; + } if (!this._now.isFixedTime) this._now.time = asWallTime(this._now.time + to - this._now.ticks); this._now.ticks = to; @@ -145,33 +197,45 @@ export class ClockController { this._replayLogOnce(); if (ticks < 0) throw new TypeError('Negative ticks are not supported'); - await this._runTo(shiftTicks(this._now.ticks, ticks)); + await this._runTo(() => shiftTicks(this._now.ticks, ticks)); } - private async _runTo(to: Ticks) { - to = Math.ceil(to) as Ticks; + private async _runTo(calculateTo: () => Ticks) { + // Since running timers is an async operation, avoid re-entrancy. + await this._runToSemaphore.acquire(); - if (this._now.ticks > to) - return; + try { + const to = Math.ceil(calculateTo()) as Ticks; + + let firstException: Error | undefined; + while (true) { + const result = await this._callFirstTimer(to); + if (!result.timerFound) + break; + firstException = firstException || result.error; + } - let firstException: Error | undefined; - while (true) { - const result = await this._callFirstTimer(to); - if (!result.timerFound) - break; - firstException = firstException || result.error; - } + // While running the timers, it is possible to advance past `to` + // by syncing with real time from within now() or performance.now(). + if (this._now.ticks < to) + this._advanceNow(to); + this._updateRealTimeTimer(); - this._advanceNow(to); - if (firstException) - throw firstException; + if (firstException) + throw firstException; + } finally { + this._runToSemaphore.release(); + } } async pauseAt(time: number): Promise { this._replayLogOnce(); this._innerPause(); - const toConsume = time - this._now.time; - await this._innerFastForwardTo(shiftTicks(this._now.ticks, toConsume)); + let toConsume = 0; + await this._innerFastForwardTo(() => { + toConsume = time - this._now.time; + return shiftTicks(this._now.ticks, toConsume); + }); return toConsume; } @@ -214,26 +278,30 @@ export class ClockController { callAt, dispose: this._embedder.setTimeout(() => { this._currentRealTimeTimer = undefined; - this._syncRealTime(); - // eslint-disable-next-line no-console - void this._runTo(this._now.ticks).catch(e => console.error(e)).then(() => this._updateRealTimeTimer()); + void this._runTo(() => this._syncRealTime() ?? this._now.ticks); }, callAt - this._now.ticks), }; } async fastForward(ticks: number) { this._replayLogOnce(); - await this._innerFastForwardTo(shiftTicks(this._now.ticks, ticks | 0)); + await this._innerFastForwardTo(() => { + this._advanceNow(this._syncRealTime() ?? this._now.ticks); + return shiftTicks(this._now.ticks, ticks | 0); + }); } - private async _innerFastForwardTo(to: Ticks) { - if (to < this._now.ticks) - throw new Error('Cannot fast-forward to the past'); - for (const timer of this._timers.values()) { - if (to > timer.callAt) - timer.callAt = to; - } - await this._runTo(to); + private async _innerFastForwardTo(calculateTo: () => Ticks) { + await this._runTo(() => { + const to = calculateTo(); + if (to < this._now.ticks) + throw new Error('Cannot fast-forward to the past'); + for (const timer of this._timers.values()) { + if (to > timer.callAt) + timer.callAt = to; + } + return to; + }); } addTimer(options: { func: TimerHandler, type: TimerType, delay?: number | string, args?: any[] }): number { @@ -288,7 +356,11 @@ export class ClockController { if (!timer) return null; - this._advanceNow(timer.callAt); + if (timer.callAt > this._now.ticks) { + // When the system is busy, a timer can be called late, which means we should not + // rewind back from |now| to |timer.callAt|. + this._advanceNow(timer.callAt); + } if (timer.type === TimerType.Interval) timer.callAt = shiftTicks(timer.callAt, timer.delay); @@ -339,6 +411,9 @@ export class ClockController { } getTimeToNextFrame() { + // When `window.requestAnimationFrame` is the first call in the page, + // this place is the first API call, so replay the log. + this._replayLogOnce(); return 16 - this._now.ticks % 16; } diff --git a/tests/library/unit/clock.spec.ts b/tests/library/unit/clock.spec.ts index a346fe3d52e6f..26d273db2ee10 100644 --- a/tests/library/unit/clock.spec.ts +++ b/tests/library/unit/clock.spec.ts @@ -21,6 +21,7 @@ import type { Builtins } from '../../../packages/injected/src/utilityScript'; const createClock = (now?: number): ClockController & Builtins => { const { clock, api } = rawCreateClock(globalThis); + clock.setStrictModeForTests(); clock.setSystemTime(now || 0); for (const key of Object.keys(api)) clock[key] = api[key]; @@ -46,6 +47,7 @@ const it = test.extend({ let clockObject: ClockController & Builtins; const install = (now?: number) => { const { clock, api } = rawInstall(globalThis); + clock.setStrictModeForTests(); if (now) clock.setSystemTime(now); for (const key of Object.keys(api)) @@ -62,6 +64,7 @@ const it = test.extend({ await use((config?: InstallConfig) => { const result = rawInstall(globalThis, config); clock = result.clock; + clock.setStrictModeForTests(); return result; }); clock?.uninstall(); @@ -1384,6 +1387,41 @@ it.describe('fastForward', () => { expect(shortTimers[1].callCount).toBe(1); expect(shortTimers[2].callCount).toBe(1); }); + + it('does not rewind back in time', async ({ clock }) => { + const stub = createStub(); + const gotTime = await new Promise(done => { + clock.setTimeout(() => { + stub(clock.Date.now()); + }, 10); + clock.setTimeout(() => { + stub(clock.Date.now()); + }, 10); + clock.resume(); + setTimeout(async () => { + // Call fast-forward right after the real time sync happens, + // but before all the callbacks are processed. + await clock.fastForward(1000); + setTimeout(() => { + done(clock.Date.now()); + }, 20); + }, 10); + }); + expect(stub.callCount).toBe(2); + expect(gotTime).toBeGreaterThan(1010); + }); + + it('error does not break the clock', async ({ clock }) => { + const stub = createStub(); + clock.setTimeout(() => { + stub(clock.Date.now()); + }, 1000); + const error = await clock.fastForward(-1000).catch(e => e); + expect(error.message).toContain('Cannot fast-forward to the past'); + await clock.fastForward(2000); + expect(stub.callCount).toBe(1); + expect(stub.calledWith(2000)).toBeTruthy(); + }); }); it.describe('pauseAt', () => {