feat(TimerService): add new delay method and protect device args

michaelfig · michaelfig · commit 7a2c830b6cde · 2021-09-19T21:40:17.000-06:00
diff --git a/packages/SwingSet/src/devices/timer-src.js b/packages/SwingSet/src/devices/timer-src.js
@@ -1,10 +1,10 @@
 /**
  * A Timer device that provides a capability that can be used to schedule wake()
  * calls at particular times. The simpler form is a handler object with a wake()
- * function can be passed to D(timer).setWakeup(delaySecs, handler) to be woken
- * after delaySecs.
+ * function can be passed to D(timer).setWakeup(baseTime, handler) to be woken
+ * after baseTime.
  *
- * The other form r = D(timer).makeRepeater(startTime, interval) allows one to
+ * The other form r = D(timer).makeRepeater(baseTime, interval) allows one to
  * create a repeater object which can be used to scheduled regular wakeups. Each
  * time D(timer).schedule(r, w) is called, w.wake(r) will be scheduled to be
  * called after the next multiple of interval since startTime. This doesn't
@@ -265,11 +265,11 @@ export function buildRootDeviceNode(tools) {
   // but the repeated calls won't accumulate timing drift, so the trigger
   // point will be reached at consistent intervals.
   return Far('root', {
-    setWakeup(delaySecs, handler) {
-      assert.typeof(delaySecs, 'bigint');
-      deadlines.add(lastPolled + Nat(delaySecs), handler);
+    setWakeup(baseTime, handler) {
+      baseTime = Nat(baseTime);
+      deadlines.add(baseTime, handler);
       saveState();
-      return lastPolled + Nat(delaySecs);
+      return baseTime;
     },
     removeWakeup(handler) {
       const times = deadlines.remove(handler);
diff --git a/packages/SwingSet/src/vats/types.js b/packages/SwingSet/src/vats/types.js
@@ -10,17 +10,19 @@
  * `repeater.schedule(h)`.
  * @property {(delay: RelativeTime, interval: RelativeTime) => TimerRepeater} createRepeater
  * DEPRECATED: use makeRepeater instead.
- * @property {(delaySecs: RelativeTime, interval: RelativeTime) => TimerRepeater} makeRepeater
+ * @property {(delay: RelativeTime, interval: RelativeTime) => TimerRepeater} makeRepeater
  * Create and return a repeater that will schedule `wake()` calls
  * repeatedly at times that are a multiple of interval following delay.
  * Interval is the difference between successive times at which wake will be
  * called.  When `schedule(w)` is called, `w.wake()` will be scheduled to be
  * called after the next multiple of interval from the base. Since times can be
  * coarse-grained, the actual call may occur later, but this won't change when
  * the next event will be called.
- * @property {(delaySecs: RelativeTime, interval: RelativeTime) => Notifier<Timestamp>} makeNotifier
+ * @property {(delay: RelativeTime, interval: RelativeTime) => Notifier<Timestamp>} makeNotifier
  * Create and return a Notifier that will deliver updates repeatedly at times
  * that are a multiple of interval following delay.
+ * @property {(delay: RelativeTime) => Promise<Timestamp>} delay
+ * Create and return a promise that will resolve after the relative time has passed.
  */
 
 /**
diff --git a/packages/SwingSet/src/vats/vat-timerWrapper.js b/packages/SwingSet/src/vats/vat-timerWrapper.js
@@ -1,7 +1,10 @@
+// @ts-check
+
 import { Nat } from '@agoric/nat';
 import { assert, details as X } from '@agoric/assert';
 import { Far } from '@agoric/marshal';
 import { makeNotifierKit } from '@agoric/notifier';
+import { makePromiseKit } from '@agoric/promise-kit';
 
 export function buildRootObject(vatPowers) {
   const { D } = vatPowers;
@@ -13,26 +16,30 @@ export function buildRootObject(vatPowers) {
       getCurrentTimestamp() {
         return Nat(D(timerNode).getLastPolled());
       },
-      setWakeup(delaySecs, handler) {
-        return D(timerNode).setWakeup(delaySecs, handler);
+      setWakeup(baseTime, handler) {
+        baseTime = Nat(baseTime);
+        return D(timerNode).setWakeup(baseTime, handler);
       },
       // can be used after setWakeup(h) or schedule(h)
       removeWakeup(handler) {
         return D(timerNode).removeWakeup(handler);
       },
       // deprecated in favor of makeRepeater().
       // TODO(#2164): remove before Beta
-      createRepeater(delaySecs, interval) {
-        return timerService.makeRepeater(delaySecs, interval);
+      createRepeater(delay, interval) {
+        delay = Nat(delay);
+        interval = Nat(interval);
+        return timerService.makeRepeater(delay, interval);
       },
-      makeRepeater(delaySecs, interval) {
-        Nat(delaySecs);
+      makeRepeater(delay, interval) {
+        delay = Nat(delay);
+        interval = Nat(interval);
         assert(
-          Nat(interval) > 0,
+          interval > 0,
           X`makeRepeater's second parameter must be a positive integer: ${interval}`,
         );
 
-        const index = D(timerNode).makeRepeater(delaySecs, interval);
+        const index = D(timerNode).makeRepeater(delay, interval);
 
         const vatRepeater = Far('vatRepeater', {
           schedule(h) {
@@ -46,22 +53,44 @@ export function buildRootObject(vatPowers) {
         repeaters.set(index, vatRepeater);
         return vatRepeater;
       },
-      makeNotifier(delaySecs, interval) {
-        Nat(delaySecs);
+      makeNotifier(delay, interval) {
+        delay = Nat(delay);
+        interval = Nat(interval);
         assert(
-          Nat(interval) > 0,
+          interval > 0,
           X`makeNotifier's second parameter must be a positive integer: ${interval}`,
         );
 
-        const index = D(timerNode).makeRepeater(delaySecs, interval);
+        const index = D(timerNode).makeRepeater(delay, interval);
         const { notifier, updater } = makeNotifierKit();
         const updateHandler = Far('updateHandler', {
           wake: updater.updateState,
         });
         D(timerNode).schedule(index, updateHandler);
 
+        // FIXME: The fact that we never delete the repeater (for the `index`)
+        // means that there is a resource leak and no way the repeater ever
+        // stops.
+        //
+        // This happens even if every recipient of the notifier permanently
+        // stops asking for updates, or equivalently, they drop all references
+        // to the notifier.
+        //
+        // To solve this problem, we could elegantly use something like #3854.
+
         return notifier;
       },
+      delay(delay) {
+        delay = Nat(delay);
+        const now = timerService.getCurrentTimestamp();
+        const baseTime = now + delay;
+        const promiseKit = makePromiseKit();
+        const delayHandler = Far('delayHandler', {
+          wake: promiseKit.resolve,
+        });
+        timerService.setWakeup(baseTime, delayHandler);
+        return promiseKit.promise;
+      },
     });
     return timerService;
   }
diff --git a/packages/zoe/tools/manualTimer.js b/packages/zoe/tools/manualTimer.js
@@ -9,6 +9,7 @@ import { Far } from '@agoric/marshal';
 import './types.js';
 import './internal-types.js';
 import { makeNotifierKit } from '@agoric/notifier';
+import { makePromiseKit } from '@agoric/promise-kit';
 
 /**
  * A fake clock that also logs progress.
@@ -25,11 +26,11 @@ export default function buildManualTimer(log, startValue = 0n, timeStep = 1n) {
   // Legacy because the value is mutated after it is stored.
   const schedule = makeLegacyMap('Timestamp');
 
-  const makeRepeater = (delaySecs, interval, timer) => {
-    assert.typeof(delaySecs, 'bigint');
+  const makeRepeater = (delay, interval, timer) => {
+    assert.typeof(delay, 'bigint');
     assert(
-      delaySecs % timeStep === 0n,
-      `timer has a resolution of ${timeStep}; ${delaySecs} is not divisible`,
+      delay % timeStep === 0n,
+      `timer has a resolution of ${timeStep}; ${delay} is not divisible`,
     );
     assert.typeof(interval, 'bigint');
     assert(
@@ -70,7 +71,7 @@ export default function buildManualTimer(log, startValue = 0n, timeStep = 1n) {
         timer.removeWakeup(repeaterWaker);
       },
     });
-    nextWakeup = ticks + delaySecs;
+    nextWakeup = ticks + delay;
     timer.setWakeup(nextWakeup, repeaterWaker);
     return repeater;
   };
@@ -135,14 +136,14 @@ export default function buildManualTimer(log, startValue = 0n, timeStep = 1n) {
     createRepeater(delay, interval) {
       return makeRepeater(delay, interval, timer);
     },
-    makeRepeater(delaySecs, interval) {
-      return makeRepeater(delaySecs, interval, timer);
+    makeRepeater(delay, interval) {
+      return makeRepeater(delay, interval, timer);
     },
-    makeNotifier(delaySecs, interval) {
-      assert.typeof(delaySecs, 'bigint');
+    makeNotifier(delay, interval) {
+      assert.typeof(delay, 'bigint');
       assert(
-        (delaySecs % timeStep) === 0n,
-        `timer has a resolution of ${timeStep}; ${delaySecs} is not divisible`,
+        (delay % timeStep) === 0n,
+        `timer has a resolution of ${timeStep}; ${delay} is not divisible`,
       );
       assert.typeof(interval, 'bigint');
       assert(
@@ -158,9 +159,24 @@ export default function buildManualTimer(log, startValue = 0n, timeStep = 1n) {
           timer.setWakeup(ticks + interval, repeaterWaker);
         },
       });
-      timer.setWakeup(ticks + delaySecs, repeaterWaker);
+      timer.setWakeup(ticks + delay, repeaterWaker);
       return notifier;
     },
+    delay(delay) {
+      assert.typeof(delay, 'bigint');
+      assert(
+        (delay % timeStep) === 0n,
+        `timer has a resolution of ${timeStep}; ${delay} is not divisible`,
+      );
+      const promiseKit = makePromiseKit();
+      const delayWaker = Far('delayWaker', {
+        wake(timestamp) {
+          promiseKit.resolve(timestamp);
+        },
+      });
+      timer.setWakeup(delay, delayWaker);
+      return promiseKit.promise;
+    },
   });
   return timer;
 }
