feat: clean up after dead vats

packages/SwingSet/src/kernel/dynamicVat.js

@@ -49,7 +49,7 @@ export function makeDynamicVatCreator(stuff) {
   function createVatDynamically(source, dynamicOptions = {}) {
     const vatID = allocateUnusedVatID();
     kernelKeeper.addDynamicVatID(vatID);
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+    const vatKeeper = kernelKeeper.allocateVatKeeper(vatID);
     vatKeeper.setSourceAndOptions(source, dynamicOptions);
     // eslint-disable-next-line no-use-before-define
     return create(vatID, source, dynamicOptions, true);

packages/SwingSet/src/kernel/id.js

@@ -26,10 +26,12 @@ export function insistVatID(s) {
     if (s !== `${s}`) {
       throw new Error(`not a string`);
     }
-    if (!s.startsWith(`v`)) {
-      throw new Error(`does not start with 'v'`);
+    if (s !== 'none') {
+      if (!s.startsWith(`v`)) {
+        throw new Error(`does not start with 'v'`);
+      }
+      Nat(Number(s.slice(1)));
     }
-    Nat(Number(s.slice(1)));
   } catch (e) {
     throw new Error(`'${s} is not a 'vNN'-style VatID: ${e.message}`);
   }

packages/SwingSet/src/kernel/kernel.js

@@ -39,6 +39,9 @@ function makeError(s) {
   return harden({ body: JSON.stringify(s), slots: [] });
 }
 
+const VAT_TERMINATION_ERROR = makeError('vat terminated');
+const UNKNOWN_VAT_ERROR = makeError('unknown vat');
+
 export default function buildKernel(kernelEndowments, kernelOptions = {}) {
   const {
     waitUntilQuiescent,
@@ -173,7 +176,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     insistVatID(forVatID);
     const kernelSlot = `${what}`;
     parseKernelSlot(what);
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(forVatID);
+    const vatKeeper = kernelKeeper.getVatKeeper(forVatID);
     return vatKeeper.mapKernelSlotToVatSlot(kernelSlot);
   }
 
@@ -184,7 +187,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     }
     insistVatID(fromVatID);
     assert(parseVatSlot(vatSlot).allocatedByVat);
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(fromVatID);
+    const vatKeeper = kernelKeeper.getVatKeeper(fromVatID);
     return vatKeeper.mapVatSlotToKernelSlot(vatSlot);
   }
 
@@ -309,8 +312,8 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     const vat = ephemeral.vats.get(vatID);
     kernelKeeper.incStat('dispatches');
     kernelKeeper.incStat('dispatchDeliver');
-    if (!vat || vat.dead) {
-      resolveToError(msg.result, makeError('unknown vat'));
+    if (!vat || vat.dead || vatID === 'none') {
+      resolveToError(msg.result, UNKNOWN_VAT_ERROR);
     } else {
       const kd = harden(['message', target, msg]);
       const vd = vat.translators.kernelDeliveryToVatDelivery(kd);
@@ -323,7 +326,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     const { type } = parseKernelSlot(target);
     if (type === 'object') {
       const vatID = kernelKeeper.ownerOfKernelObject(target);
-      insistVatID(vatID);
+      vatID === 'none' || insistVatID(vatID);
       await deliverToVat(vatID, target, msg);
     } else if (type === 'promise') {
       const kp = kernelKeeper.getKernelPromise(target);
@@ -553,7 +556,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
         },
       }); // marker
       vatObj0s[name] = vref;
-      const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+      const vatKeeper = kernelKeeper.getVatKeeper(vatID);
       const kernelSlot = vatKeeper.mapVatSlotToKernelSlot(vatSlot);
       vrefs.set(vref, kernelSlot);
       logStartup(`adding vref ${name} [${vatID}]`);
@@ -690,9 +693,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
       enablePipelining = false,
       notifyTermination = () => {},
     } = managerOptions;
-    // This should create the vatKeeper. Other users get it from the
-    // kernelKeeper, so we don't need a reference ourselves.
-    kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+    kernelKeeper.getVatKeeper(vatID);
     const translators = makeVatTranslators(vatID, kernelKeeper);
 
     ephemeral.vats.set(
@@ -788,7 +789,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
 
   function collectVatStats(vatID) {
     insistVatID(vatID);
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+    const vatKeeper = kernelKeeper.getVatKeeper(vatID);
     return vatKeeper.vatStats();
   }
 
@@ -810,6 +811,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
       const vatID = kernelKeeper.allocateVatIDForNameIfNeeded(name);
       logStartup(`Assigned VatID ${vatID} for genesis vat ${name}`);
       kernelSlog.addVat(vatID, false, name);
+      kernelKeeper.allocateVatKeeper(vatID);
       const managerOptions = harden({
         ...genesisVats.get(name),
         vatConsole: makeVatConsole(vatID),
@@ -824,33 +826,26 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     // instantiate all dynamic vats
     for (const vatID of kernelKeeper.getAllDynamicVatIDs()) {
       logStartup(`Loading dynamic vat ${vatID}`);
-      const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
-      if (vatKeeper.isDead()) {
-        kernelKeeper.forgetVat(vatID);
-      } else {
-        const {
-          source,
-          options: dynamicOptions,
-        } = vatKeeper.getSourceAndOptions();
-        // eslint-disable-next-line no-await-in-loop
-        await recreateVatDynamically(vatID, source, dynamicOptions);
-        // now the vatManager is attached and ready for transcript replay
-      }
+      const vatKeeper = kernelKeeper.allocateVatKeeper(vatID);
+      const {
+        source,
+        options: dynamicOptions,
+      } = vatKeeper.getSourceAndOptions();
+      // eslint-disable-next-line no-await-in-loop
+      await recreateVatDynamically(vatID, source, dynamicOptions);
+      // now the vatManager is attached and ready for transcript replay
     }
 
     function terminateVat(vatID) {
-      const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
-      if (!vatKeeper.isDead()) {
-        vatKeeper.markAsDead();
-        const err = makeError('vat terminated');
-        for (const kpid of kernelKeeper.findPromisesDecidedByVat(vatID)) {
-          resolveToError(kpid, err, vatID);
+      if (kernelKeeper.getVatKeeper(vatID)) {
+        const promisesToReject = kernelKeeper.cleanupAfterTerminatedVat(vatID);
+        for (const kpid of promisesToReject) {
+          resolveToError(kpid, VAT_TERMINATION_ERROR, vatID);
         }
         removeVatManager(vatID).then(
           () => kdebug(`terminated vat ${vatID}`),
           e => console.error(`problem terminating vat ${vatID}`, e),
         );
-        kernelKeeper.forgetVat(vatID);
       }
     }
 

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

@@ -432,21 +432,42 @@ export default function makeKernelKeeper(storage) {
     storage.set(`${kernelSlot}.data.slots`, capdata.slots.join(','));
   }
 
-  function findPromisesDecidedByVat(vatID) {
-    const prefixKey = `${vatID}.c.p`;
-    const endKey = `${vatID}.c.q`;
-    const result = [];
-    for (const k of storage.getKeys(prefixKey, endKey)) {
+  function cleanupAfterTerminatedVat(vatID) {
+    insistVatID(vatID);
+    ephemeral.vatKeepers.delete(vatID);
+    const koPrefix = `${vatID}.c.o+`;
+    const kpPrefix = `${vatID}.c.p`;
+    const kernelPromisesToReject = [];
+    for (const k of storage.getKeys(`${vatID}.`, `${vatID}/`)) {
       // The store semantics ensure this iteration is lexicographic.  Any
       // changes to the creation of the list of promises need to preserve this
       // in order to preserve determinism.
-      const kpid = storage.get(k);
-      const p = getKernelPromise(kpid);
-      if (p.state === 'unresolved' && p.decider === vatID) {
-        result.push(kpid);
+      if (k.startsWith(koPrefix)) {
+        const koid = storage.get(k);
+        const ownerKey = `${koid}.owner`;
+        const ownerVat = storage.get(ownerKey);
+        if (ownerVat === vatID) {
+          storage.set(ownerKey, 'none');
+        }
+      } else if (k.startsWith(kpPrefix)) {
+        const kpid = storage.get(k);
+        const p = getKernelPromise(kpid);
+        if (p.state === 'unresolved' && p.decider === vatID) {
+          kernelPromisesToReject.push(kpid);
+        }
       }
+      storage.delete(k);
     }
-    return result;
+    // TODO: deleting entries from the dynamic vat IDs list requires a linear
+    // scan; this collection ought to be represented in a way that makes it
+    // efficient to remove an entry from it, though this should be OK as long as
+    // we keep the list short.
+    const DYNAMIC_IDS_KEY = 'vat.dynamicIDs';
+    const oldDynamicVatIDs = JSON.parse(getRequired(DYNAMIC_IDS_KEY));
+    const newDynamicVatIDs = oldDynamicVatIDs.filter(v => v !== vatID);
+    storage.set(DYNAMIC_IDS_KEY, JSON.stringify(newDynamicVatIDs));
+
+    return kernelPromisesToReject;
   }
 
   function addMessageToPromiseQueue(kernelSlot, msg) {
@@ -622,30 +643,32 @@ export default function makeKernelKeeper(storage) {
     deadKernelPromises.clear();
   }
 
-  function allocateVatKeeperIfNeeded(vatID) {
+  function getVatKeeper(vatID) {
     insistVatID(vatID);
-    if (!storage.has(`${vatID}.o.nextID`)) {
-      initializeVatState(storage, vatID);
-    }
-    if (!ephemeral.vatKeepers.has(vatID)) {
-      const vk = makeVatKeeper(
-        storage,
-        vatID,
-        addKernelObject,
-        addKernelPromiseForVat,
-        incrementRefCount,
-        decrementRefCount,
-        incStat,
-        decStat,
-      );
-      ephemeral.vatKeepers.set(vatID, vk);
-    }
     return ephemeral.vatKeepers.get(vatID);
   }
 
-  function forgetVat(vatID) {
+  function allocateVatKeeper(vatID) {
     insistVatID(vatID);
-    ephemeral.vatKeepers.delete(vatID);
+    if (!storage.has(`${vatID}.o.nextID`)) {
+      initializeVatState(storage, vatID);
+    }
+    assert(
+      !ephemeral.vatKeepers.has(vatID),
+      details`vatID ${vatID} already defined`,
+    );
+    const vk = makeVatKeeper(
+      storage,
+      vatID,
+      addKernelObject,
+      addKernelPromiseForVat,
+      incrementRefCount,
+      decrementRefCount,
+      incStat,
+      decStat,
+    );
+    ephemeral.vatKeepers.set(vatID, vk);
+    return vk;
   }
 
   function getAllVatIDs() {
@@ -726,16 +749,17 @@ export default function makeKernelKeeper(storage) {
     const kernelTable = [];
 
     for (const vatID of getAllVatIDs()) {
-      const vk = allocateVatKeeperIfNeeded(vatID);
-
-      // TODO: find some way to expose the liveSlots internal tables, the
-      // kernel doesn't see them
-      const vatTable = {
-        vatID,
-        state: { transcript: Array.from(vk.getTranscript()) },
-      };
-      vatTables.push(vatTable);
-      vk.dumpState().forEach(e => kernelTable.push(e));
+      const vk = getVatKeeper(vatID);
+      if (vk) {
+        // TODO: find some way to expose the liveSlots internal tables, the
+        // kernel doesn't see them
+        const vatTable = {
+          vatID,
+          state: { transcript: Array.from(vk.getTranscript()) },
+        };
+        vatTables.push(vatTable);
+        vk.dumpState().forEach(e => kernelTable.push(e));
+      }
     }
 
     for (const deviceID of getAllDeviceIDs()) {
@@ -815,7 +839,6 @@ export default function makeKernelKeeper(storage) {
     fulfillKernelPromiseToPresence,
     fulfillKernelPromiseToData,
     rejectKernelPromise,
-    findPromisesDecidedByVat,
     addMessageToPromiseQueue,
     addSubscriberToPromise,
     setDecider,
@@ -831,8 +854,9 @@ export default function makeKernelKeeper(storage) {
     getVatIDForName,
     allocateVatIDForNameIfNeeded,
     allocateUnusedVatID,
-    allocateVatKeeperIfNeeded,
-    forgetVat,
+    allocateVatKeeper,
+    getVatKeeper,
+    cleanupAfterTerminatedVat,
     getAllVatNames,
     addDynamicVatID,
     getAllDynamicVatIDs,

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

@@ -179,14 +179,6 @@ export function makeVatKeeper(
     }
   }
 
-  function markAsDead() {
-    storage.set(`${vatID}.dead`, 'true');
-  }
-
-  function isDead() {
-    return !!storage.get(`${vatID}.dead`);
-  }
-
   /**
    * Generator function to return the vat's transcript, one entry at a time.
    */
@@ -252,8 +244,6 @@ export function makeVatKeeper(
     mapVatSlotToKernelSlot,
     mapKernelSlotToVatSlot,
     deleteCListEntry,
-    markAsDead,
-    isDead,
     getTranscript,
     addToTranscript,
     vatStats,

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

@@ -30,7 +30,7 @@ export function makeLocalVatManagerFactory(tools) {
 
   function prepare(vatID, managerOptions = {}) {
     const { notifyTermination = undefined } = managerOptions;
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+    const vatKeeper = kernelKeeper.getVatKeeper(vatID);
     const transcriptManager = makeTranscriptManager(
       kernelKeeper,
       vatKeeper,

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

@@ -38,7 +38,7 @@ export function makeNodeWorkerVatManagerFactory(tools) {
       // stops doing that, turn this into a regular assert
       console.log(`node-worker does not support enableInternalMetering`);
     }
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+    const vatKeeper = kernelKeeper.getVatKeeper(vatID);
     const transcriptManager = makeTranscriptManager(
       kernelKeeper,
       vatKeeper,

packages/SwingSet/src/kernel/vatManager/worker-subprocess-node.js

@@ -39,7 +39,7 @@ export function makeNodeSubprocessFactory(tools) {
       // stops doing that, turn this into a regular assert
       console.log(`node-worker does not support enableInternalMetering`);
     }
-    const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+    const vatKeeper = kernelKeeper.getVatKeeper(vatID);
     const transcriptManager = makeTranscriptManager(
       kernelKeeper,
       vatKeeper,

packages/SwingSet/src/kernel/vatTranslator.js

@@ -12,7 +12,7 @@ import { deleteCListEntryIfEasy } from './cleanup';
  * objects
  */
 function makeTranslateKernelDeliveryToVatDelivery(vatID, kernelKeeper) {
-  const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+  const vatKeeper = kernelKeeper.getVatKeeper(vatID);
   const { mapKernelSlotToVatSlot } = vatKeeper;
 
   // msg is { method, args, result }, all slots are kernel-centric
@@ -96,7 +96,7 @@ function makeTranslateKernelDeliveryToVatDelivery(vatID, kernelKeeper) {
  * objects
  */
 function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
-  const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+  const vatKeeper = kernelKeeper.getVatKeeper(vatID);
   const { mapVatSlotToKernelSlot } = vatKeeper;
 
   function translateSend(targetSlot, method, args, resultSlot) {
@@ -240,7 +240,7 @@ function makeTranslateKernelSyscallResultToVatSyscallResult(
   vatID,
   kernelKeeper,
 ) {
-  const vatKeeper = kernelKeeper.allocateVatKeeperIfNeeded(vatID);
+  const vatKeeper = kernelKeeper.getVatKeeper(vatID);
 
   const { mapKernelSlotToVatSlot } = vatKeeper;