fix(SwingSet): Don't send stopVat during upgrade

Fixes #6650

packages/SwingSet/docs/garbage-collection.md

@@ -360,9 +360,7 @@ If the last importing vat had previously called `syscall.retireImport`, there wi
 
 ## syscall.abandonExport processing
 
-During vat upgrade, the last delivery made to the old version is a special `dispatch.stopVat()`. This instructs the soon-to-be-deleted worker to destroy anything that will not survive the upgrade. All Remotables will be abandoned here, because they live solely in RAM, and the RAM heap does not survive. All non-durable virtual objects will also be abandoned, since only durable ones survive. We may also drop otherwise-durable objects which were only referenced by abandoned non-durable objects.
-
-During this phase, the vat performs a `syscall.abandonExports()` with all the abandoned vrefs. The kernel reacts to this in roughly the same way it reacts to the entire vat being terminated (which it is, sort of, at least the heap is being terminated). Each vref is deleted from the exporting vat's c-list, because *that* vat isn't going to be referencing it any more. The kernel object table is updated to clear the `owner` field: while the kernel object retains its identity, it is now orphaned, and any messages sent to it will be rejected (by the kernel) with a "vat terminated" error.
+Although not currently used (see [#6650](https://github.com/Agoric/agoric-sdk/issues/6650)), the kernel still supports vat issuance of `syscall.abandonExports()` with vrefs to be abandoned as part of vat upgrade (i.e., Remotables that live solely in the RAM heap and non-durable merely-virtual objects, and ideally also otherwise-durable objects not stored in baggage and referenced only by other abandoned objects). The kernel reacts to this in roughly the same way it reacts to the entire vat being terminated (which it is, sort of, at least the heap is being terminated). Each vref is deleted from the exporting vat's c-list, because *that* vat isn't going to be referencing it any more. The kernel object table is updated to clear the `owner` field: while the kernel object retains its identity, it is now orphaned, and any messages sent to it will be rejected (by the kernel) with a "vat terminated" error.
 
 No other work needs to be done. Any importing vats will continue to hold their reference as before. They can only tell that the object has been abandoned if they try to send it a message. Eventually, if all the importing vats drop their reference, and nothing else in the kernel is holding one, the kernel object entry will be deleted. In this case, no `dispatch.retireExports` is sent to the old exporting vat, since it's already been removed from their c-list.
 

packages/SwingSet/src/kernel/kernel.js

@@ -824,21 +824,6 @@ export default function buildKernel(
       incarnationNumber: vatKeeper.getIncarnationNumber(),
     };
     const disconnectionCapData = kser(disconnectObject);
-    /** @type { import('../types-external.js').KernelDeliveryStopVat } */
-    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 a method-arguments structure representing failure
@@ -855,36 +840,16 @@ export default function buildKernel(
       return ['vatUpgradeCallback', [upgradeID, false, error]];
     };
 
-    // 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 (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 = makeFailureMethargs(
-        stopVatResults.terminate.info,
-      );
-
-      // we still report computrons to the runPolicy
-      const results = harden({
-        ...stopVatResults,
-        computrons,
-        abort: true, // always unwind
-        consumeMessage: true, // don't repeat the upgrade
-        terminate: undefined, // do *not* terminate the vat
-        vatAdminMethargs,
-      });
-      return results;
-    }
-
-    // stopVat succeeded, now we finish cleanup on behalf of the worker
+    // cleanup on behalf of the worker
+    // This used to be handled by a stopVat delivery to the vat itself,
+    // but the implementation of that was cut to the bone
+    // in commits like 91480dee8e48ae26c39c420febf73b93deba6ea5
+    // basically reverting 1cfbeaa3c925d0f8502edfb313ecb12a1cab5eac
+    // and then ultimately moved to the kernel in a MUCH diminished form
+    // (see #5342 and #6650, and testUpgrade in
+    // {@link ../../test/upgrade/test-upgrade.js}).
+    // We hope to eventually add back correct sophisticated logic
+    // by e.g. having liveslots sweep the database when restoring a vat.
 
     // send BOYD so the terminating vat has one last chance to clean
     // up, drop imports, and delete durable data.
@@ -897,6 +862,15 @@ export default function buildKernel(
     (!boydResults.abort && !boydResults.terminate) ||
       Fail`Unexpected abort/terminate result from upgrade-internal bringOutYourDead: ${boydResults}`;
 
+    // we don't meter bringOutYourDead since no user code is running, but we
+    // still report computrons to the runPolicy
+    let { computrons } = boydResults;
+    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, we might want to unwind the upgrade here.
+
     // reject all promises for which the vat was decider
     for (const kpid of kernelKeeper.enumeratePromisesByDecider(vatID)) {
       resolveToError(kpid, disconnectionCapData, vatID);

packages/SwingSet/test/upgrade/test-upgrade.js

@@ -252,10 +252,9 @@ test('kernel sends bringOutYourDead for vat upgrade', async t => {
   );
   const expectedDeliveries = [
     messageDeliveryShape(1n, 'getVersion', []),
-    deliveryShape(2n, ['stopVat']),
-    deliveryShape(3n, ['bringOutYourDead']),
-    deliveryShape(4n, ['startVat']),
-    messageDeliveryShape(5n, 'getVersion', []),
+    deliveryShape(2n, ['bringOutYourDead']),
+    deliveryShape(3n, ['startVat']),
+    messageDeliveryShape(4n, 'getVersion', []),
   ];
   t.like(staticVatDeliveries, arrayShape(expectedDeliveries));
   t.is(staticVatDeliveries.length, expectedDeliveries.length);
@@ -746,7 +745,8 @@ test('non-durable exports are abandoned by upgrade of non-liveslots vat', async
   // const observerLog = await run('getDispatchLog', [], 'observer');
 });
 
-test('failed upgrade - relaxed durable rules', async t => {
+// No longer valid as of removing stopVat per #6650
+test.failing('failed upgrade - relaxed durable rules', async t => {
   const config = makeConfigFromPaths('bootstrap-scripted-upgrade.js', {
     relaxDurabilityRules: true,
     bundlePaths: {

packages/swingset-liveslots/src/liveslots.js

@@ -16,7 +16,6 @@ import { makeVirtualReferenceManager } from './virtualReferences.js';
 import { makeVirtualObjectManager } from './virtualObjectManager.js';
 import { makeCollectionManager } from './collectionManager.js';
 import { makeWatchedPromiseManager } from './watchedPromises.js';
-import { releaseOldState } from './stop-vat.js';
 
 const DEFAULT_VIRTUAL_OBJECT_CACHE_SIZE = 3; // XXX ridiculously small value to force churn for testing
 
@@ -66,7 +65,7 @@ function build(
   }
 
   let didStartVat = false;
-  let didStopVat = false;
+  const didStopVat = false;
 
   const outstandingProxies = new WeakSet();
 
@@ -1516,42 +1515,11 @@ function build(
   }
 
   /**
-   * @param { import('./types').SwingSetCapData } disconnectObjectCapData
+   * @param { import('./types').SwingSetCapData } _disconnectObjectCapData
    * @returns {Promise<void>}
    */
-  async function stopVat(disconnectObjectCapData) {
-    insistCapData(disconnectObjectCapData);
-    assert(disconnectObjectCapData.slots.length === 0);
-    assert(
-      !relaxDurabilityRules,
-      'stopVat not available when relaxDurabilityRules is true',
-    );
-
-    assert(didStartVat);
-    assert(!didStopVat);
-    didStopVat = true;
-
-    try {
-      // eslint-disable-next-line @jessie.js/no-nested-await
-      await releaseOldState({
-        m,
-        disconnectObjectCapData,
-        syscall,
-        exportedRemotables,
-        addToPossiblyDeadSet,
-        slotToVal,
-        valToSlot,
-        dropExports,
-        retireExports,
-        vrm,
-        collectionManager,
-        bringOutYourDead,
-        vreffedObjectRegistry,
-      });
-    } catch (e) {
-      console.warn(`-- error during stopVat()`, e);
-      throw e;
-    }
+  async function stopVat(_disconnectObjectCapData) {
+    Fail`stopVat is no longer supported as of #6650`;
   }
 
   /**