fix: don't retire promises that resolve to data structures containing promises

Author: FUDCo

packages/SwingSet/src/kernel/liveSlots.js

@@ -393,6 +393,19 @@ function build(syscall, _state, makeRoot, forVatID) {
     slotToVal.delete(promiseID);
   }
 
+  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;
+      }
+    }
+    retirePromiseID(promiseID);
+  }
+
   function thenResolve(promiseID) {
     insistVatType('promise', promiseID);
     return res => {
@@ -406,9 +419,6 @@ function build(syscall, _state, makeRoot, forVatID) {
       lsdebug(` ser ${ser.body} ${JSON.stringify(ser.slots)}`);
       // find out what resolution category we're using
       const unser = JSON.parse(ser.body);
-      if (importedPromisesByPromiseID.has(promiseID)) {
-        importedPromisesByPromiseID.get(promiseID).res(res);
-      }
       if (
         Object(unser) === unser &&
         QCLASS in unser &&
@@ -417,12 +427,10 @@ function build(syscall, _state, makeRoot, forVatID) {
         const slot = ser.slots[unser.index];
         insistVatType('object', slot);
         syscall.fulfillToPresence(promiseID, slot);
-        retirePromiseID(promiseID);
       } else {
         // if it resolves to data, .thens fire but kernel-queued messages are
         // rejected, because you can't send messages to data
         syscall.fulfillToData(promiseID, ser);
-        retirePromiseID(promiseID);
       }
 
       // TODO (for chip): the kernel currently notifies all subscribers of a
@@ -445,6 +453,7 @@ function build(syscall, _state, makeRoot, forVatID) {
       if (pRec) {
         pRec.resolve(res);
       }
+      retirePromiseIDIfEasy(promiseID, ser);
     };
   }
 
@@ -453,17 +462,14 @@ function build(syscall, _state, makeRoot, forVatID) {
       harden(rej);
       lsdebug(`ls thenReject fired`, rej);
       const ser = m.serialize(rej);
-      if (importedPromisesByPromiseID.has(promiseID)) {
-        importedPromisesByPromiseID.get(promiseID).rej(rej);
-      }
       syscall.reject(promiseID, ser);
-      retirePromiseID(promiseID);
       // TODO (for chip): this is also a double-rejection until the
       // retire-promises branch lands. Delete this comment when that happens.
       const pRec = importedPromisesByPromiseID.get(promiseID);
       if (pRec) {
         pRec.reject(rej);
       }
+      retirePromiseIDIfEasy(promiseID, ser);
     };
   }
 
@@ -480,7 +486,7 @@ function build(syscall, _state, makeRoot, forVatID) {
     const pRec = importedPromisesByPromiseID.get(promiseID);
     const val = m.unserialize(data);
     pRec.resolve(val);
-    retirePromiseID(promiseID);
+    retirePromiseIDIfEasy(promiseID, data);
   }
 
   function notifyFulfillToPresence(promiseID, slot) {
@@ -514,7 +520,7 @@ function build(syscall, _state, makeRoot, forVatID) {
     const pRec = importedPromisesByPromiseID.get(promiseID);
     const val = m.unserialize(data);
     pRec.reject(val);
-    retirePromiseID(promiseID);
+    retirePromiseIDIfEasy(promiseID, data);
   }
 
   // here we finally invoke the vat code, and get back the root object

packages/SwingSet/src/kernel/vatManager.js

@@ -107,6 +107,19 @@ export default function makeVatManager(
     }
   }
 
+  function deleteCListEntryIfEasy(kpid, vpid, kernelData) {
+    for (const slot of kernelData.slots) {
+      const { type } = parseKernelSlot(slot);
+      if (type === 'promise') {
+        kdebug(
+          `Unable to delete clist entry ${kpid}<=>${vpid} because slot ${slot} is a promise`,
+        );
+        return;
+      }
+    }
+    vatKeeper.deleteCListEntry(kpid, vpid);
+  }
+
   // syscall handlers: these are wrapped by the 'syscall' object and made
   // available to userspace
 
@@ -182,7 +195,7 @@ export default function makeVatManager(
         data.body
       } ${JSON.stringify(data.slots)}/${JSON.stringify(kernelSlots)}`,
     );
-    vatKeeper.deleteCListEntry(kpid, promiseID);
+    deleteCListEntryIfEasy(kpid, promiseID, kernelData);
     syscallManager.fulfillToData(vatID, kpid, kernelData);
   }
 
@@ -197,7 +210,7 @@ export default function makeVatManager(
         data.body
       } ${JSON.stringify(data.slots)}/${JSON.stringify(kernelSlots)}`,
     );
-    vatKeeper.deleteCListEntry(kpid, promiseID);
+    deleteCListEntryIfEasy(kpid, promiseID, kernelData);
     syscallManager.reject(vatID, kpid, kernelData);
   }
 
@@ -378,7 +391,6 @@ export default function makeVatManager(
         ['notifyFulfillToPresence', vpid, slot],
         `vat[${vatID}].promise[${vpid}] fulfillToPresence failed`,
       );
-      vatKeeper.deleteCListEntry(kpid, vpid);
     } else if (kp.state === 'redirected') {
       throw new Error('not implemented yet');
     } else if (kp.state === 'fulfilledToData') {
@@ -387,24 +399,22 @@ export default function makeVatManager(
         ...kp.data,
         slots: kp.data.slots.map(slot => mapKernelSlotToVatSlot(slot)),
       });
-      vatKeeper.deleteCListEntry(kpid, vpid);
+      deleteCListEntryIfEasy(kpid, vpid, kp.data);
       await doProcess(
         ['notifyFulfillToData', vpid, vatData],
         `vat[${vatID}].promise[${vpid}] fulfillToData failed`,
       );
-      vatKeeper.deleteCListEntry(kpid, vpid);
     } else if (kp.state === 'rejected') {
       const vpid = mapKernelSlotToVatSlot(kpid);
       const vatData = harden({
         ...kp.data,
         slots: kp.data.slots.map(slot => mapKernelSlotToVatSlot(slot)),
       });
-      vatKeeper.deleteCListEntry(kpid, vpid);
+      deleteCListEntryIfEasy(kpid, vpid, kp.data);
       await doProcess(
         ['notifyReject', vpid, vatData],
         `vat[${vatID}].promise[${vpid}] reject failed`,
       );
-      vatKeeper.deleteCListEntry(kpid, vpid);
     } else {
       throw new Error(`unknown kernelPromise state '${kp.state}'`);
     }

packages/SwingSet/test/test-vpid-liveslots.js

@@ -243,11 +243,10 @@ async function doVatResolveCase1(t, mode) {
     type: 'send',
     targetSlot: target1,
     method: 'two',
-    args: capargs([slot0arg], [expectedP3]),
-    resultSlot: expectedP4,
+    args: capargs([slot0arg], [expectedP1]),
+    resultSlot: expectedP3,
   });
-  t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP4 });
-  t.deepEqual(log.shift(), resolutionOf(expectedP3, mode, target2));
+  t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP3 });
   t.deepEqual(log, []);
 
   t.end();
@@ -653,28 +652,27 @@ async function doVatResolveCase4(t, mode) {
   await endOfCrank();
 
   const expectedP4 = nextP();
-  const expectedP5 = nextP();
   t.deepEqual(log.shift(), {
     type: 'send',
     targetSlot: target1,
     method: 'three',
-    args: capargs([slot0arg], [expectedP4]),
-    resultSlot: expectedP5,
+    args: capargs([slot0arg], [p1]),
+    resultSlot: expectedP4,
   });
-  t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP5 });
+  t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP4 });
 
   if (mode === 'presence') {
     const expectedP5 = nextP();
     t.deepEqual(log.shift(), {
       type: 'send',
-      targetSlot: target2,
+      targetSlot: target2, // this depends on #823 being fixed
       method: 'four',
       args: capargs([], []),
-      resultSlot: expectedP6,
+      resultSlot: expectedP5,
     });
-    t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP6 });
+    t.deepEqual(log.shift(), { type: 'subscribe', target: expectedP5 });
   }
-  t.deepEqual(log.shift(), resolutionOf(expectedP4, mode, target2));
+  // if p1 rejects or resolves to data, the kernel never hears about four()
   t.deepEqual(log, []);
 
   t.end();