feat: promise resolution notification refactoring

packages/SwingSet/src/kernel/cleanup.js

@@ -1,23 +1,62 @@
 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,
 ) {
-  let idx = 0;
-  for (const slot of kernelData.slots) {
-    const { type } = parseKernelSlot(slot);
-    if (type === 'promise') {
+  if (ENABLE_PROMISE_ANALYSIS) {
+    const visited = new Set();
+    let sawPromise;
+
+    function scanKernelPromise(scanKPID, scanKernelData) {
+      visited.add(scanKPID);
+      kdebug(`@@@ scan ${scanKPID}`);
+      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, 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 slot[${idx}]===${slot} is a promise`,
+        `Unable to delete ${vatID} clist entry ${kpid}<=>${vpid} because there was a contained promise ${sawPromise}`,
       );
       return;
     }
-    idx += 1;
   }
   vatKeeper.deleteCListEntry(kpid, vpid);
 }

packages/SwingSet/src/kernel/kernel.js

@@ -157,8 +157,7 @@ export default function buildKernel(
 
   // runQueue entries are {type, vatID, more..}. 'more' depends on type:
   // * deliver: target, msg
-  // * notifyFulfillToData/notifyFulfillToPresence/notifyReject:
-  //   kernelPromiseID
+  // * notify: kernelPromiseID
 
   // in the kernel table, promises and resolvers are both indexed by the same
   // value. kernelPromises[promiseID] = { decider, subscribers }
@@ -454,7 +453,7 @@ export default function buildKernel(
       case 'fulfilledToData':
         return 'dispatchNotifyFulfillToData';
       case 'rejected':
-        return 'dispatchReject';
+        return 'dispatchNotifyReject';
       default:
         throw Error(`unknown promise state ${state}`);
     }

packages/SwingSet/src/kernel/liveSlots.js

@@ -123,11 +123,11 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     insistVatType('promise', vpid);
     lsdebug(`makeImportedPromise(${vpid})`);
 
-    // The Promise will we associated with a handler that converts p~.foo()
-    // into a syscall.send() that targets the vpid. When the Promise is
-    // resolved (during receipt of a dispatch.notifyFulfill* or
-    // notifyReject), this Promise's handler will be replaced by the handler
-    // of the resolution, which might be a Presence or a local object.
+    // The Promise will we associated with a handler that converts p~.foo() into
+    // a syscall.send() that targets the vpid. When the Promise is resolved
+    // (during receipt of a dispatch.notify), this Promise's handler will be
+    // replaced by the handler of the resolution, which might be a Presence or a
+    // local object.
 
     // for safety as we shake out bugs in HandledPromise, we guard against
     // this handler being used after it was supposed to be resolved
@@ -382,7 +382,7 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     }
     // TODO: if we acquire new decision-making authority over a promise that
     // we already knew about ('result' is already in slotToVal), we should no
-    // longer accept dispatch.notifyFulfill from the kernel. We currently use
+    // longer accept dispatch.notify from the kernel. We currently use
     // importedPromisesByPromiseID to track a combination of "we care about
     // when this promise resolves" and "we are listening for the kernel to
     // resolve it". We should split that into two tables or something. And we
@@ -435,14 +435,18 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     slotToVal.delete(promiseID);
   }
 
+  const ENABLE_PROMISE_ANALYSIS = true;
+
   function retirePromiseIDIfEasy(promiseID, data) {
-    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;
+    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);
@@ -501,11 +505,11 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     };
   }
 
-  function notifyFulfillToData(promiseID, data) {
+  function notify(promiseID, rejected, data) {
     assert(didRoot);
     insistCapData(data);
     lsdebug(
-      `ls.dispatch.notifyFulfillToData(${promiseID}, ${data.body}, ${data.slots})`,
+      `ls.dispatch.notify(${promiseID}, ${rejected}, ${data.body}, [${data.slots}])`,
     );
     insistVatType('promise', promiseID);
     // TODO: insist that we do not have decider authority for promiseID
@@ -514,46 +518,16 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     }
     const pRec = importedPromisesByPromiseID.get(promiseID);
     const val = m.unserialize(data);
-    pRec.resolve(val);
-    retirePromiseIDIfEasy(promiseID, data);
-  }
-
-  function notifyFulfillToPresence(promiseID, slot) {
-    assert(didRoot);
-    lsdebug(`ls.dispatch.notifyFulfillToPresence(${promiseID}, ${slot})`);
-    insistVatType('promise', promiseID);
-    // TODO: insist that we do not have decider authority for promiseID
-    insistVatType('object', slot);
-    if (!importedPromisesByPromiseID.has(promiseID)) {
-      throw new Error(`unknown promiseID '${promiseID}'`);
+    if (rejected) {
+      pRec.reject(val);
+    } else {
+      pRec.resolve(val);
     }
-    const pRec = importedPromisesByPromiseID.get(promiseID);
-    const val = convertSlotToVal(slot);
-    // val is either a local pass-by-presence object, or a Presence (which
-    // points at some remote pass-by-presence object).
-    pRec.resolve(val);
-    retirePromiseID(promiseID);
+    retirePromiseIDIfEasy(promiseID, data);
   }
 
   // TODO: when we add notifyForward, guard against cycles
 
-  function notifyReject(promiseID, data) {
-    assert(didRoot);
-    insistCapData(data);
-    lsdebug(
-      `ls.dispatch.notifyReject(${promiseID}, ${data.body}, ${data.slots})`,
-    );
-    insistVatType('promise', promiseID);
-    // TODO: insist that we do not have decider authority for promiseID
-    if (!importedPromisesByPromiseID.has(promiseID)) {
-      throw new Error(`unknown promiseID '${promiseID}'`);
-    }
-    const pRec = importedPromisesByPromiseID.get(promiseID);
-    const val = m.unserialize(data);
-    pRec.reject(val);
-    retirePromiseIDIfEasy(promiseID, data);
-  }
-
   function exitVat(completion) {
     syscall.exit(false, m.serialize(harden(completion)));
   }
@@ -586,12 +560,7 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     slotToVal.set(rootSlot, rootObject);
   }
 
-  const dispatch = harden({
-    deliver,
-    notifyFulfillToData,
-    notifyFulfillToPresence,
-    notifyReject,
-  });
+  const dispatch = harden({ deliver, notify });
   return harden({ vatGlobals, setBuildRootObject, dispatch, m });
 }
 

packages/SwingSet/src/kernel/state/kernelKeeper.js

@@ -161,9 +161,10 @@ export default function makeKernelKeeper(storage, kernelSlog) {
     syscallCallNow: 0,
     dispatches: 0,
     dispatchDeliver: 0,
+    dispatchNotify: 0,
     dispatchNotifyFulfillToData: 0,
     dispatchNotifyFulfillToPresence: 0,
-    dispatchReject: 0,
+    dispatchNotifyReject: 0,
     clistEntries: 0,
     clistEntriesUp: 0,
     clistEntriesDown: 0,

packages/SwingSet/src/kernel/vatManager/deliver.js

@@ -81,29 +81,26 @@ export function makeDeliver(tools, dispatch) {
     );
   }
 
-  async function deliverOneNotification(vpid, vp) {
-    const errmsg = `vat[${vatID}].promise[${vpid}] ${vp.state} failed`;
-    switch (vp.state) {
-      case 'fulfilledToPresence':
-        return doProcess(['notifyFulfillToPresence', vpid, vp.slot], errmsg);
-      case 'redirected':
-        // TODO unimplemented
-        throw new Error('not implemented yet');
-      case 'fulfilledToData':
-        return doProcess(['notifyFulfillToData', vpid, vp.data], errmsg);
-      case 'rejected':
-        return doProcess(['notifyReject', vpid, vp.data], errmsg);
-      default:
-        throw Error(`unknown promise state '${vp.state}'`);
+  async function deliverOneNotification(primaryVpid, resolutions) {
+    for (const vpid of Object.keys(resolutions)) {
+      // XXX return inside loop is wrong once `resolutions` has more than 1 element
+      const vp = resolutions[vpid];
+      const errmsg = `vat[${vatID}].promise[${vpid}] ${vp.rejected} failed`;
+      return doProcess(['notify', vpid, vp.rejected, vp.data], errmsg);
     }
+    // XXX placeholder to make lint shut up until we're done implementing things
+    return ['error', 'incomplete code, this should never happen'];
   }
 
   // vatDeliverObject is:
   //  ['message', target, msg]
   //   target is vid
   //   msg is: { method, args (capdata), result }
-  //  ['notify', vpid, vp]
-  //   vp is the current (final) promise data, rendered in vat form
+  //  ['notify', vpid, resolutions]
+  //   vpid is the id of the primary promise being resolved
+  //   resolutions is an object mapping vpid's to the final promise data,
+  //     rendered in vat form, for both the primary promise and any collateral
+  //     promises it references whose resolution has been newly discovered
   async function deliver(vatDeliverObject) {
     const [type, ...args] = vatDeliverObject;
     switch (type) {

packages/SwingSet/src/kernel/vatManager/nodeWorkerSupervisor.js

@@ -59,21 +59,15 @@ function doMessage(targetSlot, msg) {
   );
 }
 
-function doNotify(vpid, vp) {
-  const errmsg = `vat.promise[${vpid}] ${vp.state} failed`;
-  switch (vp.state) {
-    case 'fulfilledToPresence':
-      return doProcess(['notifyFulfillToPresence', vpid, vp.slot], errmsg);
-    case 'redirected':
-      // TODO unimplemented
-      throw new Error('not implemented yet');
-    case 'fulfilledToData':
-      return doProcess(['notifyFulfillToData', vpid, vp.data], errmsg);
-    case 'rejected':
-      return doProcess(['notifyReject', vpid, vp.data], errmsg);
-    default:
-      throw Error(`unknown promise state '${vp.state}'`);
+function doNotify(primaryVpid, resolutions) {
+  for (const vpid of Object.keys(resolutions)) {
+    // XXX return inside loop is wrong once `resolutions` has more than 1 element
+    const vp = resolutions[vpid];
+    const errmsg = `vat.promise[${vpid}] ${vp.rejected} failed`;
+    return doProcess(['notify', vpid, vp.rejected, vp.data], errmsg);
   }
+  // XXX placeholder to make lint shut up until we're done implementing things
+  return ['error', 'incomplete code, this should never happen'];
 }
 
 parentPort.on('message', ([type, ...margs]) => {

packages/SwingSet/src/kernel/vatManager/subprocessSupervisor.js

@@ -59,21 +59,15 @@ function doMessage(targetSlot, msg) {
   );
 }
 
-function doNotify(vpid, vp) {
-  const errmsg = `vat.promise[${vpid}] ${vp.state} failed`;
-  switch (vp.state) {
-    case 'fulfilledToPresence':
-      return doProcess(['notifyFulfillToPresence', vpid, vp.slot], errmsg);
-    case 'redirected':
-      // TODO unimplemented
-      throw new Error('not implemented yet');
-    case 'fulfilledToData':
-      return doProcess(['notifyFulfillToData', vpid, vp.data], errmsg);
-    case 'rejected':
-      return doProcess(['notifyReject', vpid, vp.data], errmsg);
-    default:
-      throw Error(`unknown promise state '${vp.state}'`);
+function doNotify(primaryVpid, resolutions) {
+  for (const vpid of Object.keys(resolutions)) {
+    // XXX return inside loop is wrong once `resolutions` has more than 1 element
+    const vp = resolutions[vpid];
+    const errmsg = `vat.promise[${vpid}] ${vp.rejected} failed`;
+    return doProcess(['notify', vpid, vp.rejected, vp.data], errmsg);
   }
+  // XXX placeholder to make lint shut up until we're done implementing things
+  return ['error', 'incomplete code, this should never happen'];
 }
 
 const toParent = arrayEncoderStream();