Merge pull request #7130 from Agoric/6694-kernel-disconnect-promises

fix(SwingSet): move promise rejection from stopVat to the kernel

packages/SwingSet/src/kernel/kernel.js

@@ -257,8 +257,11 @@ export default function buildKernel(
     // check will report 'false'. That's fine, there's no state to
     // clean up.
     if (kernelKeeper.vatIsAlive(vatID)) {
-      const promisesToReject = kernelKeeper.cleanupAfterTerminatedVat(vatID);
-      for (const kpid of promisesToReject) {
+      // Reject all promises decided by the vat, making sure to capture the list
+      // of kpids before that data is deleted.
+      const deadPromises = [...kernelKeeper.enumeratePromisesByDecider(vatID)];
+      kernelKeeper.cleanupAfterTerminatedVat(vatID);
+      for (const kpid of deadPromises) {
         resolveToError(kpid, makeError('vat terminated'), vatID);
       }
     }
@@ -818,52 +821,58 @@ export default function buildKernel(
       upgradeMessage,
       incarnationNumber: vatKeeper.getIncarnationNumber(),
     };
+    const disconnectionCapData = kser(disconnectObject);
     /** @type { import('../types-external.js').KernelDeliveryStopVat } */
-    const kd1 = harden(['stopVat', kser(disconnectObject)]);
-    const vd1 = vatWarehouse.kernelDeliveryToVatDelivery(vatID, kd1);
-    const status1 = await deliverAndLogToVat(vatID, kd1, vd1);
+    const stopVatKD = harden(['stopVat', disconnectionCapData]);
+    const stopVatVD = vatWarehouse.kernelDeliveryToVatDelivery(
+      vatID,
+      stopVatKD,
+    );
+    const stopVatStatus = await deliverAndLogToVat(vatID, stopVatKD, stopVatVD);
+    const stopVatResults = deliveryCrankResults(vatID, stopVatStatus, false);
+
+    // We don't meter stopVat, since no user code is running, but we
+    // still report computrons to the runPolicy
+    let { computrons } = stopVatResults; // BigInt or undefined
+    if (computrons !== undefined) {
+      assert.typeof(computrons, 'bigint');
+    }
 
-    // make arguments for vat-vat-admin.js vatUpgradeCallback()
     /**
-     * @param {SwingSetCapData} _errorCD
+     * Make a method-arguments structure representing failure
+     * for vat-vat-admin.js vatUpgradeCallback().
+     *
+     * @param {SwingSetCapData} _errorCapData
      * @returns {RawMethargs}
      */
-    function makeFailure(_errorCD) {
-      insistCapData(_errorCD); // kser(Error)
+    const makeFailureMethargs = _errorCapData => {
+      insistCapData(_errorCapData); // kser(Error)
       // const error = kunser(_errorCD)
       // actually we shouldn't reveal the details, so instead we do:
       const error = Error('vat-upgrade failure');
       return ['vatUpgradeCallback', [upgradeID, false, error]];
-    }
-
-    // We use deliveryCrankResults to parse the stopVat status.
-    const results1 = deliveryCrankResults(vatID, status1, false);
-
-    // We don't meter stopVat, since no user code is running, but we
-    // still report computrons to the runPolicy
-    let { computrons } = results1; // BigInt or undefined
-    if (computrons !== undefined) {
-      assert.typeof(computrons, 'bigint');
-    }
+    };
 
     // TODO: if/when we implement vat pause/suspend, and if
     // deliveryCrankResults changes to not use .terminate to indicate
     // a problem, this should change to match: where we would normally
     // pause/suspend a vat for a delivery error, here we want to
     // unwind the upgrade.
 
-    if (results1.terminate) {
+    if (stopVatResults.terminate) {
       // get rid of the worker, so the next delivery to this vat will
       // re-create one from the previous state
       // eslint-disable-next-line @jessie.js/no-nested-await
       await vatWarehouse.stopWorker(vatID);
 
       // notify vat-admin of the failed upgrade
-      const vatAdminMethargs = makeFailure(results1.terminate.info);
+      const vatAdminMethargs = makeFailureMethargs(
+        stopVatResults.terminate.info,
+      );
 
       // we still report computrons to the runPolicy
       const results = harden({
-        ...results1,
+        ...stopVatResults,
         computrons,
         abort: true, // always unwind
         consumeMessage: true, // don't repeat the upgrade
@@ -873,8 +882,25 @@ export default function buildKernel(
       return results;
     }
 
-    // stopVat succeeded, so now we stop the worker, delete the
-    // transcript and any snapshot
+    // stopVat succeeded. finish cleanup on behalf of the worker.
+
+    // TODO: send BOYD to the vat, to give it one last chance to clean
+    // up, drop imports, and delete durable data. If we ever have a
+    // vat that is so broken it can't do BOYD, we can make that
+    // optional. #7001
+
+    // walk c-list for all decided promises, reject them all
+    for (const kpid of kernelKeeper.enumeratePromisesByDecider(vatID)) {
+      resolveToError(kpid, disconnectionCapData, vatID);
+    }
+
+    // TODO: getNonDurableObjectExports, synthesize abandonVSO,
+    // execute it as if it were a syscall. (maybe distinguish between
+    // reachable/recognizable exports, abandon the reachable, retire
+    // the recognizable) #6696
+
+    // cleanup done, now we stop the worker, delete the transcript and
+    // any snapshot
 
     await vatWarehouse.resetWorker(vatID);
     const source = { bundleID };
@@ -888,23 +914,32 @@ export default function buildKernel(
 
     // deliver a startVat with the new vatParameters
     /** @type { import('../types-external.js').KernelDeliveryStartVat } */
-    const kd2 = harden(['startVat', vatParameters]);
-    const vd2 = vatWarehouse.kernelDeliveryToVatDelivery(vatID, kd2);
+    const startVatKD = harden(['startVat', vatParameters]);
+    const startVatVD = vatWarehouse.kernelDeliveryToVatDelivery(
+      vatID,
+      startVatKD,
+    );
     // decref vatParameters now that translation did incref
     for (const kref of vatParameters.slots) {
       kernelKeeper.decrementRefCount(kref, 'upgrade-vat-event');
     }
-    const status2 = await deliverAndLogToVat(vatID, kd2, vd2);
-    const results2 = deliveryCrankResults(vatID, status2, false);
-    computrons = addComputrons(computrons, results2.computrons);
+    const startVatStatus = await deliverAndLogToVat(
+      vatID,
+      startVatKD,
+      startVatVD,
+    );
+    const startVatResults = deliveryCrankResults(vatID, startVatStatus, false);
+    computrons = addComputrons(computrons, startVatResults.computrons);
 
-    if (results2.terminate) {
+    if (startVatResults.terminate) {
       // unwind just like above
       // eslint-disable-next-line @jessie.js/no-nested-await
       await vatWarehouse.stopWorker(vatID);
-      const vatAdminMethargs = makeFailure(results2.terminate.info);
+      const vatAdminMethargs = makeFailureMethargs(
+        startVatResults.terminate.info,
+      );
       const results = harden({
-        ...results2,
+        ...startVatResults,
         computrons,
         abort: true, // always unwind
         consumeMessage: true, // don't repeat the upgrade

packages/SwingSet/src/kernel/kernelQueue.js

@@ -45,9 +45,7 @@ export function makeKernelQueueHandler(tools) {
       const p = kernelKeeper.getResolveablePromise(kpid, vatID);
       const { subscribers } = p;
       for (const subscriber of subscribers) {
-        if (subscriber !== vatID) {
-          notify(subscriber, kpid);
-        }
+        notify(subscriber, kpid);
       }
       kernelKeeper.resolveKernelPromise(kpid, rejected, data);
       const tag = rejected ? 'rejected' : 'fulfilled';

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

@@ -781,8 +781,6 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
     const vatKeeper = provideVatKeeper(vatID);
     const exportPrefix = `${vatID}.c.o+`;
     const importPrefix = `${vatID}.c.o-`;
-    const promisePrefix = `${vatID}.c.p`;
-    const kernelPromisesToReject = [];
 
     vatKeeper.deleteSnapshotsAndTranscript();
 
@@ -822,23 +820,8 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
       // that will also delete both db keys
     }
 
-    // now find all orphaned promises, which must be rejected
-    for (const k of enumeratePrefixedKeys(kvStore, promisePrefix)) {
-      // The vpid for a promise imported or exported by a vat (and thus
-      // potentially a promise for which the vat *might* be the decider) will
-      // always be of the form `p+NN` or `p-NN`.  The corresponding vpid->kpid
-      // c-list entry will thus always begin with `vMM.c.p`.  Decider-ship is
-      // independent of whether the promise was imported or exported, so we
-      // have to look up the corresponding kernel promise table entry to see
-      // whether the vat is the decider or not.  If it is, we add the promise
-      // to the list of promises that must be rejected because the dead vat
-      // will never be able to act upon them.
-      const kpid = kvStore.get(k);
-      const p = getKernelPromise(kpid);
-      if (p.state === 'unresolved' && p.decider === vatID) {
-        kernelPromisesToReject.push(kpid);
-      }
-    }
+    // the caller used enumeratePromisesByDecider() before calling us,
+    // so they already know the orphaned promises to reject
 
     // now loop back through everything and delete it all
     for (const k of enumeratePrefixedKeys(kvStore, `${vatID}.`)) {
@@ -870,8 +853,6 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
       }
       decStat('vats');
     }
-
-    return kernelPromisesToReject;
   }
 
   function addMessageToPromiseQueue(kernelSlot, msg) {
@@ -925,6 +906,27 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
     kvStore.set(`${kpid}.decider`, '');
   }
 
+  function* enumeratePromisesByDecider(vatID) {
+    insistVatID(vatID);
+    const promisePrefix = `${vatID}.c.p`;
+    for (const k of enumeratePrefixedKeys(kvStore, promisePrefix)) {
+      // The vpid for a promise imported or exported by a vat (and thus
+      // potentially a promise for which the vat *might* be the decider) will
+      // always be of the form `p+NN` or `p-NN`.  The corresponding vpid->kpid
+      // c-list entry will thus always begin with `vMM.c.p`.  Decider-ship is
+      // independent of whether the promise was imported or exported, so we
+      // have to look up the corresponding kernel promise table entry to see
+      // whether the vat is the decider or not.  If it is, we add the promise
+      // to the list of promises that must be rejected because the dead vat
+      // will never be able to act upon them.
+      const kpid = kvStore.get(k);
+      const p = getKernelPromise(kpid);
+      if (p.state === 'unresolved' && p.decider === vatID) {
+        yield kpid;
+      }
+    }
+  }
+
   function addSubscriberToPromise(kernelSlot, vatID) {
     insistKernelType('promise', kernelSlot);
     insistVatID(vatID);
@@ -1568,6 +1570,7 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
     addSubscriberToPromise,
     setDecider,
     clearDecider,
+    enumeratePromisesByDecider,
     incrementRefCount,
     decrementRefCount,
     getObjectRefCount,

packages/SwingSet/test/bootstrap-syscall-failure.js

@@ -34,7 +34,7 @@ export function buildRootObject(vatPowers, vatParameters) {
         () => testLog('p2 resolve (bad!)'),
         e => testLog(`p2 reject ${e}`),
       );
-      const p3 = E(badvat).begood(ourThing);
+      const p3 = E(badvat).begoodagain(ourThing);
       p3.then(
         () => testLog('p3 resolve (bad!)'),
         e => testLog(`p3 reject ${e}`),

packages/SwingSet/test/test-marshal.js

@@ -144,7 +144,7 @@ test('unserialize promise', async t => {
   t.truthy(p instanceof Promise);
 });
 
-test('kernel serialzation of errors', async t => {
+test('kernel serialization of errors', async t => {
   // The kernel synthesizes e.g. `Error('vat-upgrade failure')`, so we
   // need kmarshal to serialize those errors in a deterministic
   // way. This test checks that we don't get surprising things like

packages/SwingSet/test/test-promises.js

@@ -6,7 +6,9 @@ import {
   loadBasedir,
   buildKernelBundles,
 } from '../src/index.js';
-import { kser, kslot } from '../src/lib/kmarshal.js';
+import { kser, kslot, kunser } from '../src/lib/kmarshal.js';
+
+const bfile = name => new URL(name, import.meta.url).pathname;
 
 test.before(async t => {
   const kernelBundles = await buildKernelBundles();
@@ -212,3 +214,70 @@ test('refcount while queued', async t => {
   await c.run();
   t.deepEqual(c.kpResolution(kpid4), kser([true, 3]));
 });
+
+test('local promises are rejected by vat upgrade', async t => {
+  // TODO: Generalize packages/SwingSet/test/upgrade/test-upgrade.js
+  /** @type {SwingSetConfig} */
+  const config = {
+    includeDevDependencies: true, // for vat-data
+    defaultManagerType: 'xs-worker',
+    bootstrap: 'bootstrap',
+    defaultReapInterval: 'never',
+    vats: {
+      bootstrap: {
+        sourceSpec: bfile('./bootstrap-relay.js'),
+      },
+    },
+    bundles: {
+      watcher: { sourceSpec: bfile('./vat-durable-promise-watcher.js') },
+    },
+  };
+  const c = await buildVatController(config);
+  t.teardown(c.shutdown);
+  c.pinVatRoot('bootstrap');
+  await c.run();
+
+  const run = async (method, args = []) => {
+    assert(Array.isArray(args));
+    const kpid = c.queueToVatRoot('bootstrap', method, args);
+    await c.run();
+    const status = c.kpStatus(kpid);
+    if (status === 'fulfilled') {
+      const result = c.kpResolution(kpid);
+      return kunser(result);
+    }
+    assert(status === 'rejected');
+    const err = c.kpResolution(kpid);
+    throw kunser(err);
+  };
+  const messageVat = (name, methodName, args) =>
+    run('messageVat', [{ name, methodName, args }]);
+  // eslint-disable-next-line no-unused-vars
+  const messageObject = (presence, methodName, args) =>
+    run('messageVatObject', [{ presence, methodName, args }]);
+
+  const S = Symbol.for('passable');
+  await run('createVat', [{ name: 'watcher', bundleCapName: 'watcher' }]);
+  await messageVat('watcher', 'watchLocalPromise', ['orphaned']);
+  await messageVat('watcher', 'watchLocalPromise', ['fulfilled', S]);
+  await messageVat('watcher', 'watchLocalPromise', ['rejected', undefined, S]);
+  const v1Settlements = await messageVat('watcher', 'getSettlements');
+  t.deepEqual(v1Settlements, {
+    fulfilled: { status: 'fulfilled', value: S },
+    rejected: { status: 'rejected', reason: S },
+  });
+  await run('upgradeVat', [{ name: 'watcher', bundleCapName: 'watcher' }]);
+  const v2Settlements = await messageVat('watcher', 'getSettlements');
+  t.deepEqual(v2Settlements, {
+    fulfilled: { status: 'fulfilled', value: S },
+    rejected: { status: 'rejected', reason: S },
+    orphaned: {
+      status: 'rejected',
+      reason: {
+        name: 'vatUpgraded',
+        upgradeMessage: 'vat upgraded',
+        incarnationNumber: 1,
+      },
+    },
+  });
+});

packages/SwingSet/test/vat-durable-promise-watcher.js

@@ -0,0 +1,40 @@
+import { Far } from '@endo/marshal';
+import { getCopyMapEntries, M } from '@agoric/store';
+import { makePromiseKit } from '@endo/promise-kit';
+import {
+  prepareExo,
+  provideDurableMapStore,
+  watchPromise,
+} from '@agoric/vat-data';
+
+export function buildRootObject(_vatPowers, vatParameters, baggage) {
+  const settlements = provideDurableMapStore(baggage, 'settlements');
+  const PromiseWatcherI = M.interface('PromiseWatcher', {
+    onFulfilled: M.call(M.any(), M.string()).returns(),
+    onRejected: M.call(M.any(), M.string()).returns(),
+  });
+  const watcher = prepareExo(baggage, 'PromiseWatcher', PromiseWatcherI, {
+    onFulfilled(value, name) {
+      settlements.init(name, harden({ status: 'fulfilled', value }));
+    },
+    onRejected(reason, name) {
+      settlements.init(name, harden({ status: 'rejected', reason }));
+    },
+  });
+
+  return Far('root', {
+    watchLocalPromise: (name, fulfillment, rejection) => {
+      const { promise, resolve, reject } = makePromiseKit();
+      if (fulfillment !== undefined) {
+        resolve(fulfillment);
+      } else if (rejection !== undefined) {
+        reject(rejection);
+      }
+      watchPromise(promise, watcher, name);
+    },
+    getSettlements: () => {
+      const settlementsCopyMap = settlements.snapshot();
+      return Object.fromEntries(getCopyMapEntries(settlementsCopyMap));
+    },
+  });
+}