feat: vat-side promise ID retirement

packages/SwingSet/docs/vat-worker.md

@@ -23,18 +23,21 @@ If instead, the run-queue action is `notify`, the action will name a promise (wh
 The `Delivery` object is a hardened Array of data (Objects, Arrays, and Strings, all of which can be serialized as JSON), whose first element is a type. It will take one of the following shapes:
 
 * `['message', targetSlot, msg]`
-* `['notify', vpid, vp]`
+* `['notify', resolutions]`
 
 In the `message` form, `targetSlot` is a object/promise reference (a string like `o+13` or `p-24`), which identifies the object or promise to which the message is being sent. This target can be a promise if the message is being pipelined to the result of some earlier message.
 
 The `msg` field is an object shaped like `{ method, args, result }`, where `method` is a string (the method name being invoked), `args` is a "capdata" structure (an object `{ body, slots }`, where `body` is a JSON-formatted string, and `slots` is an array of object/promise references), and `result` is either `null` or a promise reference string (the promise that should be resolved by the executing vat if/when the message processing is complete).
 
-In the `notify` form, `vpid` is a promise reference that names the promise being resolved. `vp` is a record that describes the resolution. It will take one of the following forms:
+In the `notify` form, `resolutions` is an array of one or more resolution descriptors, each of which is an array of the form:
 
-* `{ state: 'fulfilledToPresence', slot }` (where `slot` is an object reference string)
-* `{ state: 'fulfilledToData', data }` (where `data` is a capdata structure)
-* `{ state: 'rejected', data }` (again `data` is a capdata structure)
-* (a future form might use `state: 'forwarded'`, but that is not implemented yet)
+* `[vpid, promiseDescriptor]`
+
+`vpid` is a promise reference that names the promise being resolved. `promiseDescriptor` is a record that describes the resolution, in the form:
+
+* `{ rejected, data }`
+
+`rejected` is a boolean value, where `true` indicates that the promise is being fulfilled and `false` indicates that `data` is a capdata structure that describes the value the promise is being fulfilled to or rejected as.
 
 This Delivery object begins life in the kernel as a `KernelDelivery` object, in which all of the object/promise references ("slots") are kernel-centric. Object references will look like `ko123`, and promise references will look like `kp456`.
 
@@ -49,18 +52,20 @@ The VatManager is given access to a `VatSyscallHandler` function. This takes a `
 * `['send', target, msg]`
 * `['callNow', target, method, args]`
 * `['subscribe', vpid]`
-* `['fulfillToPresence', vpid, slot]`
-* `['fulfillToData', vpid, data]`
-* `['reject', vpid, data]`
+* `['resolve', resolutions]`
+* `['vatstoreGet', key]`
+* `['vatstoreSet', key, data]`
+* `['vatstoreDelete', key]`
 
 As with deliveries (but in reverse), the translator converts this from vat-centric identifiers into kernel-centric ones, and emits a `KernelSyscall` object, with one of these forms:
 
 * `['send', target, msg]`
 * `['invoke', target, method, args]`
 * `['subscribe', vatid, kpid]`
-* `['fulfillToPresence', kpid, slot]`
-* `['fulfillToData', kpid, data]`
-* `['reject', kpid, data]`
+* `['resolve', vatid, resolutions]`
+* `['vatstoreGet', vatid, key]`
+* `['vatstoreSet', vatid, key, data]`
+* `['vatstoreDelete', vatid, key]`
 
 The `KernelSyscallHandler` accepts one of these objects and executes the syscall. Most syscalls modify kernel state (by appending an item to the run-queue, or modifying promise- and object- tables) and then return an empty result. `invoke` is special in that it will synchronously invoke some device and then return a result that contains arbitrary data. In any event, the KernelSyscallHandler returns a `KernelSyscallResult` object, which has one of the following forms:
 

packages/SwingSet/src/kernel/cleanup.js

@@ -1,67 +1,6 @@
 // import { kdebug } from './kdebug';
 import { parseKernelSlot } from './parseKernelSlots';
 
-// XXX temporary flags to control features during development
-const ENABLE_PROMISE_ANALYSIS = true; // flag to enable/disable check to see if delete clist entry is ok
-
-export function deleteCListEntryIfEasy(
-  vatID,
-  vatKeeper,
-  kernelKeeper,
-  kpid,
-  vpid,
-  kernelData,
-) {
-  if (ENABLE_PROMISE_ANALYSIS) {
-    const visited = new Set();
-    let sawPromise;
-
-    function scanKernelPromise(scanKPID, scanKernelData) {
-      visited.add(scanKPID);
-      // kdebug(`@@@ scan ${scanKPID} ${JSON.stringify(scanKernelData)}`);
-      if (scanKernelData) {
-        for (const slot of scanKernelData.slots) {
-          const { type } = parseKernelSlot(slot);
-          if (type === 'promise') {
-            sawPromise = slot;
-            if (visited.has(slot)) {
-              // kdebug(`@@@ ${slot} previously visited`);
-              return true;
-            } else {
-              const { data } = kernelKeeper.getKernelPromise(slot);
-              // const { data, state } = kernelKeeper.getKernelPromise(slot);
-              if (data) {
-                if (scanKernelPromise(slot, data)) {
-                  // kdebug(`@@@ scan ${slot} detects circularity`);
-                  return true;
-                }
-              } else {
-                // kdebug(`@@@ scan ${slot} state = ${state}`);
-              }
-            }
-          }
-        }
-      }
-      // kdebug(`@@@ scan ${scanKPID} detects no circularity`);
-      return false;
-    }
-
-    // kdebug(`@@ checking ${vatID} ${kpid} for circularity`);
-    if (scanKernelPromise(kpid, kernelData)) {
-      // kdebug(
-      //  `Unable to delete ${vatID} clist entry ${kpid}<=>${vpid} because it is indirectly self-referential`,
-      // );
-      return;
-    } else if (sawPromise) {
-      // kdebug(
-      //  `Unable to delete ${vatID} clist entry ${kpid}<=>${vpid} because there was a contained promise ${sawPromise}`,
-      // );
-      return;
-    }
-  }
-  vatKeeper.deleteCListEntry(kpid, vpid);
-}
-
 export function getKpidsToRetire(kernelKeeper, rootKPID, rootKernelData) {
   const seen = new Set();
   function scanKernelPromise(kpid, kernelData) {

packages/SwingSet/src/kernel/liveSlots.js

@@ -195,11 +195,15 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     return makeVatSlot('promise', true, promiseID);
   }
 
+  const knownResolutions = new WeakMap();
+
   function exportPromise(p) {
     const pid = allocatePromiseID();
     lsdebug(`Promise allocation ${forVatID}:${pid} in exportPromise`);
-    // eslint-disable-next-line no-use-before-define
-    p.then(thenResolve(pid), thenReject(pid));
+    if (!knownResolutions.has(p)) {
+      // eslint-disable-next-line no-use-before-define
+      p.then(thenResolve(p, pid), thenReject(p, pid));
+    }
     return pid;
   }
 
@@ -311,6 +315,49 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     return val;
   }
 
+  function resolutionCollector() {
+    const resolutions = [];
+    const doneResolutions = new Set();
+
+    function scanSlots(slots) {
+      for (const slot of slots) {
+        const { type } = parseVatSlot(slot);
+        if (type === 'promise') {
+          const p = slotToVal.get(slot);
+          assert(p, details`should have a value for ${slot} but didn't`);
+          const priorResolution = knownResolutions.get(p);
+          if (priorResolution && !doneResolutions.has(slot)) {
+            const [priorRejected, priorRes] = priorResolution;
+            // eslint-disable-next-line no-use-before-define
+            collect(slot, priorRejected, priorRes);
+          }
+        }
+      }
+    }
+
+    function collect(promiseID, rejected, value) {
+      doneResolutions.add(promiseID);
+      const valueSer = m.serialize(value);
+      resolutions.push([promiseID, rejected, valueSer]);
+      scanSlots(valueSer.slots);
+    }
+
+    function forPromise(promiseID, rejected, value) {
+      collect(promiseID, rejected, value);
+      return resolutions;
+    }
+
+    function forSlots(slots) {
+      scanSlots(slots);
+      return resolutions;
+    }
+
+    return {
+      forPromise,
+      forSlots,
+    };
+  }
+
   function queueMessage(targetSlot, prop, args, returnedP) {
     const serArgs = m.serialize(harden(args));
     const resultVPID = allocatePromiseID();
@@ -326,6 +373,10 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
       )}) -> ${resultVPID}`,
     );
     syscall.send(targetSlot, prop, serArgs, resultVPID);
+    const resolutions = resolutionCollector().forSlots(serArgs.slots);
+    if (resolutions.length > 0) {
+      syscall.resolve(resolutions);
+    }
 
     // ideally we'd wait until .then is called on p before subscribing, but
     // the current Promise API doesn't give us a way to discover this, so we
@@ -410,16 +461,6 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
 
     const args = m.unserialize(argsdata);
 
-    let notifySuccess = () => undefined;
-    let notifyFailure = () => undefined;
-    if (result) {
-      insistVatType('promise', result);
-      // eslint-disable-next-line no-use-before-define
-      notifySuccess = thenResolve(result);
-      // eslint-disable-next-line no-use-before-define
-      notifyFailure = thenReject(result);
-    }
-
     // If the method is missing, or is not a Function, or the method throws a
     // synchronous exception, we notify the caller (by rejecting the result
     // promise, if any). If the method returns an eventually-rejected
@@ -442,6 +483,15 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
       // TODO: untested, but in principle sound.
       res = HandledPromise.get(t, method);
     }
+    let notifySuccess = () => undefined;
+    let notifyFailure = () => undefined;
+    if (result) {
+      insistVatType('promise', result);
+      // eslint-disable-next-line no-use-before-define
+      notifySuccess = thenResolve(res, result);
+      // eslint-disable-next-line no-use-before-define
+      notifyFailure = thenReject(res, result);
+    }
     res.then(notifySuccess, notifyFailure);
   }
 
@@ -453,50 +503,38 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     slotToVal.delete(promiseID);
   }
 
-  const ENABLE_PROMISE_ANALYSIS = true;
-
-  function retirePromiseIDIfEasy(promiseID, data) {
-    if (ENABLE_PROMISE_ANALYSIS) {
-      for (const slot of data.slots) {
-        const { type } = parseVatSlot(slot);
-        if (type === 'promise') {
-          lsdebug(
-            `Unable to retire ${promiseID} because slot ${slot} is a promise`,
-          );
-          return;
-        }
-      }
-    }
-    retirePromiseID(promiseID);
-  }
-
-  function thenResolve(promiseID) {
+  function thenHandler(p, promiseID, rejected) {
     insistVatType('promise', promiseID);
-    return res => {
-      harden(res);
-      lsdebug(`ls.thenResolve fired`, res);
-      const ser = m.serialize(res);
-      syscall.resolve([[promiseID, false, ser]]);
+    return value => {
+      knownResolutions.set(p, harden([rejected, value]));
+      harden(value);
+      lsdebug(`ls.thenHandler fired`, value);
+      const resolutions = resolutionCollector().forPromise(
+        promiseID,
+        rejected,
+        value,
+      );
+
+      syscall.resolve(resolutions);
+
       const pRec = importedPromisesByPromiseID.get(promiseID);
       if (pRec) {
-        pRec.resolve(res);
+        if (rejected) {
+          pRec.reject(value);
+        } else {
+          pRec.resolve(value);
+        }
       }
-      retirePromiseIDIfEasy(promiseID, ser);
+      retirePromiseID(promiseID);
     };
   }
 
-  function thenReject(promiseID) {
-    return rej => {
-      harden(rej);
-      lsdebug(`ls thenReject fired`, rej);
-      const ser = m.serialize(rej);
-      syscall.resolve([[promiseID, true, ser]]);
-      const pRec = importedPromisesByPromiseID.get(promiseID);
-      if (pRec) {
-        pRec.reject(rej);
-      }
-      retirePromiseIDIfEasy(promiseID, ser);
-    };
+  function thenResolve(p, promiseID) {
+    return thenHandler(p, promiseID, false);
+  }
+
+  function thenReject(p, promiseID) {
+    return thenHandler(p, promiseID, true);
   }
 
   function notifyOnePromise(promiseID, rejected, data) {

packages/SwingSet/src/kernel/vatTranslator.js

@@ -4,7 +4,6 @@ import { insistKernelType, parseKernelSlot } from './parseKernelSlots';
 import { insistVatType, parseVatSlot } from '../parseVatSlots';
 import { insistCapData } from '../capdata';
 import { kdebug, legibilizeMessageArgs, legibilizeValue } from './kdebug';
-import { deleteCListEntryIfEasy } from './cleanup';
 
 /*
  * Return a function that converts KernelDelivery objects into VatDelivery
@@ -223,6 +222,7 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
 
   function translateResolve(vresolutions) {
     const kresolutions = [];
+    const kpidsResolved = [];
     let idx = 0;
     for (const resolution of vresolutions) {
       const [vpid, rejected, data] = resolution;
@@ -238,19 +238,9 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
       );
       idx += 1;
       kresolutions.push([kpid, rejected, kernelData]);
-      deleteCListEntryIfEasy(
-        vatID,
-        vatKeeper,
-        kernelKeeper,
-        kpid,
-        vpid,
-        kernelData,
-      );
+      kpidsResolved.push(kpid);
     }
-    // XXX TODO Once we get rid of the "if easy" logic, the above deletions
-    // should be collected and then processed in a batch here after all the
-    // translation is done, e.g., something like:
-    // vatKeeper.deleteCListEntriesForKernelSlots(targets);
+    vatKeeper.deleteCListEntriesForKernelSlots(kpidsResolved);
     return harden(['resolve', vatID, kresolutions]);
   }
 

packages/SwingSet/src/vats/comms/clist-kernel.js

@@ -47,6 +47,10 @@ export function makeKernel(state, syscall, stateKit) {
       // promise is retired (to remember the resolution).
       assert(p, `how did I forget about ${vpid}`);
 
+      if (p.kernelAwaitingResolve) {
+        return vpid;
+      }
+
       if (p.resolved) {
         // The vpid might have been retired, in which case we must not use it
         // when speaking to the kernel. It will only be retired if 1: it

packages/SwingSet/src/vats/comms/delivery.js

@@ -33,6 +33,7 @@ export function makeDeliveryKit(state, syscall, transmit, clistKit, stateKit) {
     changeDeciderFromRemoteToComms,
     getPromiseSubscribers,
     markPromiseAsResolved,
+    markPromiseAsResolvedInKernel,
   } = stateKit;
 
   function mapDataToKernel(data) {
@@ -340,6 +341,10 @@ export function makeDeliveryKit(state, syscall, transmit, clistKit, stateKit) {
       // promiseTable reminds provideKernelForLocal to use a fresh VPID if we
       // ever reference it again in the future
     }
+    for (const resolution of resolutions) {
+      const [vpid] = resolution;
+      markPromiseAsResolvedInKernel(vpid);
+    }
   }
 
   function resolveToRemote(remoteID, resolutions) {