fix: do not advance real time while forwarding

packages/injected/src/clock.ts

@@ -59,6 +59,13 @@ type Time = {
 
 type LogEntryType = 'fastForward' |'install' | 'pauseAt' | 'resume' | 'runFor' | 'setFixedTime' | 'setSystemTime';
 
+type RealTimeTimer = {
+  callAt: Ticks;
+  cancel: () => void;
+  promise: Promise<void> | undefined;
+  dispose: () => Promise<void>;
+};
+
 export class ClockController {
   readonly _now: Time;
   private _duringTick = false;
@@ -68,7 +75,7 @@ export class ClockController {
   readonly disposables: (() => void)[] = [];
   private _log: { type: LogEntryType, time: number, param?: number }[] = [];
   private _realTime: { startTicks: EmbedderTicks, lastSyncTicks: EmbedderTicks } | undefined;
-  private _currentRealTimeTimer: { callAt: Ticks, dispose: () => void } | undefined;
+  private _currentRealTimeTimer: RealTimeTimer | undefined;
 
   constructor(embedder: Embedder) {
     this._timers = new Map();
@@ -145,7 +152,9 @@ 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._runWithDisabledRealTimeSync(async () => {
+      await this._runTo(shiftTicks(this._now.ticks, ticks));
+    });
   }
 
   private async _runTo(to: Ticks) {
@@ -169,15 +178,16 @@ export class ClockController {
 
   async pauseAt(time: number): Promise<number> {
     this._replayLogOnce();
-    this._innerPause();
+    await this._innerPause();
     const toConsume = time - this._now.time;
     await this._innerFastForwardTo(shiftTicks(this._now.ticks, toConsume));
     return toConsume;
   }
 
-  private _innerPause() {
+  private async _innerPause() {
     this._realTime = undefined;
-    this._updateRealTimeTimer();
+    await this._currentRealTimeTimer?.dispose();
+    this._currentRealTimeTimer = undefined;
   }
 
   resume() {
@@ -192,38 +202,64 @@ export class ClockController {
   }
 
   private _updateRealTimeTimer() {
-    if (!this._realTime) {
-      this._currentRealTimeTimer?.dispose();
-      this._currentRealTimeTimer = undefined;
+    if (this._currentRealTimeTimer?.promise) {
+      // In progress, safe to return as it will call itself once promise is resolved.
       return;
     }
 
     const firstTimer = this._firstTimer();
 
     // Either run the next timer or move time in 100ms chunks.
-    const callAt = Math.min(firstTimer ? firstTimer.callAt : this._now.ticks + maxTimeout, this._now.ticks + 100) as Ticks;
-    if (this._currentRealTimeTimer && this._currentRealTimeTimer.callAt < callAt)
-      return;
+    const nextTick = Math.min(firstTimer ? firstTimer.callAt : this._now.ticks + maxTimeout, this._now.ticks + 100) as Ticks;
+    const callAt = this._currentRealTimeTimer ? Math.min(this._currentRealTimeTimer.callAt, nextTick) as Ticks : nextTick;
 
     if (this._currentRealTimeTimer) {
-      this._currentRealTimeTimer.dispose();
+      // Cancel and reschedule.
+      this._currentRealTimeTimer.cancel();
       this._currentRealTimeTimer = undefined;
     }
 
-    this._currentRealTimeTimer = {
+    const realTimeTimer: RealTimeTimer = {
       callAt,
-      dispose: this._embedder.setTimeout(() => {
-        this._currentRealTimeTimer = undefined;
+      promise: undefined,
+      cancel: this._embedder.setTimeout(() => {
         this._syncRealTime();
         // eslint-disable-next-line no-console
-        void this._runTo(this._now.ticks).catch(e => console.error(e)).then(() => this._updateRealTimeTimer());
+        realTimeTimer.promise = this._runTo(this._now.ticks).catch(e => console.error(e));
+        void realTimeTimer.promise.then(() => {
+          this._currentRealTimeTimer = undefined;
+          if (this._realTime)
+            this._updateRealTimeTimer();
+        });
       }, callAt - this._now.ticks),
+      dispose: async () => {
+        realTimeTimer.cancel();
+        await realTimeTimer.promise;
+      }
     };
+
+    this._currentRealTimeTimer = realTimeTimer;
+  }
+
+  private async _runWithDisabledRealTimeSync(fn: () => Promise<void>) {
+    if (!this._realTime) {
+      await fn();
+      return;
+    }
+
+    await this._innerPause();
+    try {
+      await fn();
+    } finally {
+      this._innerResume();
+    }
   }
 
   async fastForward(ticks: number) {
     this._replayLogOnce();
-    await this._innerFastForwardTo(shiftTicks(this._now.ticks, ticks | 0));
+    await this._runWithDisabledRealTimeSync(async () => {
+      await this._innerFastForwardTo(shiftTicks(this._now.ticks, ticks | 0));
+    });
   }
 
   private async _innerFastForwardTo(to: Ticks) {
@@ -396,10 +432,8 @@ export class ClockController {
         this._advanceNow(shiftTicks(this._now.ticks, param!));
       } else if (type === 'pauseAt') {
         isPaused = true;
-        this._innerPause();
         this._innerSetTime(asWallTime(param!));
       } else if (type === 'resume') {
-        this._innerResume();
         isPaused = false;
       } else if (type === 'setFixedTime') {
         this._innerSetFixedTime(asWallTime(param!));
@@ -408,8 +442,13 @@ export class ClockController {
       }
     }
 
-    if (!isPaused && lastLogTime > 0)
-      this._advanceNow(shiftTicks(this._now.ticks, this._embedder.dateNow() - lastLogTime));
+    if (!isPaused) {
+      if (lastLogTime > 0)
+        this._advanceNow(shiftTicks(this._now.ticks, this._embedder.dateNow() - lastLogTime));
+      this._innerResume();
+    } else {
+      this._realTime = undefined;
+    }
 
     this._log.length = 0;
   }

tests/library/unit/clock.spec.ts

@@ -1384,6 +1384,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<number>(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', () => {
@@ -1595,37 +1630,6 @@ it.describe('Intl API', () => {
   });
 });
 
-it('works with concurrent runFor calls', async ({ clock }) => {
-  clock.setSystemTime(0);
-
-  const log: string[] = [];
-  for (let t = 500; t > 0; t -= 100) {
-    clock.setTimeout(() => {
-      log.push(`${t}: ${clock.Date.now()}`);
-      clock.setTimeout(() => {
-        log.push(`${t}+0: ${clock.Date.now()}`);
-      }, 0);
-    }, t);
-  }
-
-  await Promise.all([
-    clock.runFor(500),
-    clock.runFor(600),
-  ]);
-  expect(log).toEqual([
-    `100: 100`,
-    `100+0: 101`,
-    `200: 200`,
-    `200+0: 201`,
-    `300: 300`,
-    `300+0: 301`,
-    `400: 400`,
-    `400+0: 401`,
-    `500: 500`,
-    `500+0: 501`,
-  ]);
-});
-
 it('works with slow setTimeout in busy embedder', async ({ installEx }) => {
   const { originals, api, clock } = installEx({ now: 0 });
   await clock.pauseAt(0);