feat: properly terminate & clean up after failed vats

Author: FUDCo

packages/SwingSet/src/kernel/dynamicVat.js

@@ -71,8 +71,16 @@ export function makeDynamicVatCreator(stuff) {
       ? '(source bundle)'
       : `from: ${source.bundleName}`;
 
-    assertKnownOptions(dynamicOptions, ['metered', 'vatParameters']);
-    const { metered = true, vatParameters = {} } = dynamicOptions;
+    assertKnownOptions(dynamicOptions, [
+      'metered',
+      'vatParameters',
+      'enableSetup',
+    ]);
+    const {
+      metered = true,
+      vatParameters = {},
+      enableSetup = false,
+    } = dynamicOptions;
     let terminated = false;
 
     function notifyTermination(error) {
@@ -113,7 +121,7 @@ export function makeDynamicVatCreator(stuff) {
       const managerOptions = {
         bundle: vatSourceBundle,
         metered,
-        enableSetup: false,
+        enableSetup,
         enableInternalMetering: false,
         notifyTermination: metered ? notifyTermination : undefined,
         vatConsole: makeVatConsole(vatID),

packages/SwingSet/src/kernel/kernel.js

@@ -60,7 +60,9 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
   const logStartup = verbose ? console.debug : () => 0;
 
   insistStorageAPI(hostStorage);
-  const { enhancedCrankBuffer, commitCrank } = wrapStorage(hostStorage);
+  const { enhancedCrankBuffer, abortCrank, commitCrank } = wrapStorage(
+    hostStorage,
+  );
 
   const kernelSlog = writeSlogObject
     ? makeSlogger(writeSlogObject)
@@ -283,6 +285,36 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     }
   }
 
+  function removeVatManager(vatID, reason) {
+    const old = ephemeral.vats.get(vatID);
+    ephemeral.vats.delete(vatID);
+    const err = reason ? Error(reason) : undefined;
+    old.notifyTermination(err); // XXX TODO: most of the places where notifyTermination gets passed don't need it
+    return old.manager.shutdown();
+  }
+
+  function terminateVat(vatID, reason) {
+    if (kernelKeeper.getVatKeeper(vatID)) {
+      const promisesToReject = kernelKeeper.cleanupAfterTerminatedVat(vatID);
+      const err = reason ? makeError(reason) : VAT_TERMINATION_ERROR;
+      for (const kpid of promisesToReject) {
+        resolveToError(kpid, err, vatID);
+      }
+      removeVatManager(vatID, reason).then(
+        () => kdebug(`terminated vat ${vatID}`),
+        e => console.error(`problem terminating vat ${vatID}`, e),
+      );
+    }
+  }
+
+  let deliveryProblem;
+
+  function registerDeliveryProblem(vatID, problem, resultKP) {
+    if (!deliveryProblem) {
+      deliveryProblem = { vatID, problem, resultKP };
+    }
+  }
+
   async function deliverAndLogToVat(vatID, kernelDelivery, vatDelivery) {
     const vat = ephemeral.vats.get(vatID);
     const crankNum = kernelKeeper.getCrankNumber();
@@ -295,10 +327,10 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     try {
       const deliveryResult = await vat.manager.deliver(vatDelivery);
       finish(deliveryResult);
-      // TODO: eventually
-      // if (deliveryResult === death) {
-      //   vat.notifyTermination(deliveryResult.causeOfDeath);
-      // }
+      const [status, problem] = deliveryResult;
+      if (status !== 'ok') {
+        registerDeliveryProblem(vatID, problem);
+      }
     } catch (e) {
       // log so we get a stack trace
       console.error(`error in kernel.deliver:`, e);
@@ -426,6 +458,7 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     }
     try {
       processQueueRunning = Error('here');
+      deliveryProblem = null;
       if (message.type === 'send') {
         kernelKeeper.decrementRefCount(message.target, `deq|msg|t`);
         kernelKeeper.decrementRefCount(message.msg.result, `deq|msg|r`);
@@ -441,8 +474,15 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
       } else {
         throw Error(`unable to process message.type ${message.type}`);
       }
-      kernelKeeper.purgeDeadKernelPromises();
-      kernelKeeper.saveStats();
+      if (deliveryProblem) {
+        abortCrank();
+        const { vatID, problem } = deliveryProblem;
+        terminateVat(vatID, problem);
+        kdebug(`vat abnormally terminated: ${problem}`);
+      } else {
+        kernelKeeper.purgeDeadKernelPromises();
+        kernelKeeper.saveStats();
+      }
       commitCrank();
       kernelKeeper.incrementCrankNumber();
     } finally {
@@ -639,19 +679,20 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
         // This is a safety check -- this case should never happen unless the
         // vatManager is somehow confused.
         console.error(`vatSyscallHandler invoked on dead vat ${vatID}`);
-        return harden(['error', 'vat is dead']);
+        const problem = 'vat is dead';
+        registerDeliveryProblem(vatID, problem);
+        return harden(['error', problem]);
       }
       let ksc;
       try {
         // this can fail if the vat asks for something not on their clist,
         // which is fatal to the vat
         ksc = translators.vatSyscallToKernelSyscall(vatSyscallObject);
       } catch (vaterr) {
-        console.error(`vat ${vatID} error during translation: ${vaterr}`);
-        console.error(`vat terminated`);
-        // TODO: mark the vat as dead, reject subsequent syscalls, withhold
-        // deliveries, notify adminvat
-        return harden(['error', 'clist violation: prepare to die']);
+        kdebug(`vat ${vatID} terminated: error during translation: ${vaterr}`);
+        const problem = 'clist violation: prepare to die';
+        registerDeliveryProblem(vatID, problem);
+        return harden(['error', problem]);
       }
 
       const finish = kernelSlog.syscall(vatID, ksc, vatSyscallObject);
@@ -674,7 +715,9 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
         panic(`error during syscall/device.invoke: ${err}`, err);
         // the kernel is now in a shutdown state, but it may take a while to
         // grind to a halt
-        return harden(['error', 'you killed my kernel. prepare to die']);
+        const problem = 'you killed my kernel. prepare to die';
+        registerDeliveryProblem(vatID, problem);
+        return harden(['error', problem]);
       }
 
       return vres;
@@ -717,13 +760,6 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
     manager.setVatSyscallHandler(vatSyscallHandler);
   }
 
-  function removeVatManager(vatID) {
-    const old = ephemeral.vats.get(vatID);
-    ephemeral.vats.delete(vatID);
-    old.notifyTermination(null);
-    return old.manager.shutdown();
-  }
-
   const knownBundles = new Map();
 
   function hasBundle(name) {
@@ -843,19 +879,6 @@ export default function buildKernel(kernelEndowments, kernelOptions = {}) {
       // now the vatManager is attached and ready for transcript replay
     }
 
-    function terminateVat(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),
-        );
-      }
-    }
-
     if (vatAdminDeviceBundle) {
       // if we have a device bundle, then vats[vatAdmin] will be present too
       const endowments = {

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

@@ -487,7 +487,22 @@ export default function makeKernelKeeper(storage) {
     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));
+    if (newDynamicVatIDs.length !== oldDynamicVatIDs.length) {
+      storage.set(DYNAMIC_IDS_KEY, JSON.stringify(newDynamicVatIDs));
+    } else {
+      console.log(`removing static vat ${vatID}`);
+      for (const k of storage.getKeys('vat.name.', 'vat.name/')) {
+        if (storage.get(k) === vatID) {
+          storage.delete(k);
+          const VAT_NAMES_KEY = 'vat.names';
+          const name = k.slice('vat.name.'.length);
+          const oldStaticVatNames = JSON.parse(getRequired(VAT_NAMES_KEY));
+          const newStaticVatNames = oldStaticVatNames.filter(v => v !== name);
+          storage.set(VAT_NAMES_KEY, JSON.stringify(newStaticVatNames));
+          break;
+        }
+      }
+    }
 
     return kernelPromisesToReject;
   }
@@ -608,10 +623,10 @@ export default function makeKernelKeeper(storage) {
    *
    * @param kernelSlot  The kernel slot whose refcount is to be incremented.
    */
-  function incrementRefCount(kernelSlot, tag) {
+  function incrementRefCount(kernelSlot, _tag) {
     if (kernelSlot && parseKernelSlot(kernelSlot).type === 'promise') {
       const refCount = Nat(Number(storage.get(`${kernelSlot}.refCount`))) + 1;
-      kdebug(`++ ${kernelSlot}  ${tag} ${refCount}`);
+      // kdebug(`++ ${kernelSlot}  ${tag} ${refCount}`);
       storage.set(`${kernelSlot}.refCount`, `${refCount}`);
     }
   }
@@ -633,7 +648,7 @@ export default function makeKernelKeeper(storage) {
       let refCount = Nat(Number(storage.get(`${kernelSlot}.refCount`)));
       assert(refCount > 0, details`refCount underflow {kernelSlot} ${tag}`);
       refCount -= 1;
-      kdebug(`-- ${kernelSlot}  ${tag} ${refCount}`);
+      // kdebug(`-- ${kernelSlot}  ${tag} ${refCount}`);
       storage.set(`${kernelSlot}.refCount`, `${refCount}`);
       if (refCount === 0) {
         deadKernelPromises.add(kernelSlot);

packages/SwingSet/src/kernel/vatAdmin/vatAdminWrapper.js

@@ -25,11 +25,11 @@ export function buildRootObject(vatPowers) {
 
     const [doneP, doneRR] = producePRR();
     running.set(vatID, doneRR);
+    doneP.catch(() => {}); // shut up false whine about unhandled rejection
 
     const adminNode = harden({
       terminate() {
         D(vatAdminNode).terminate(vatID);
-        // TODO(hibbert): cleanup admin vat data structures
       },
       adminData() {
         return D(vatAdminNode).adminStats(vatID);

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

@@ -4,7 +4,6 @@ import { insistMessage } from '../../message';
 export function makeDeliver(tools, dispatch) {
   const {
     meterRecord,
-    notifyTermination,
     refillAllMeters,
     stopGlobalMeter,
     transcriptManager,
@@ -31,9 +30,13 @@ export function makeDeliver(tools, dispatch) {
    * so, and the kernel must be defensive against this.
    */
   function runAndWait(f, errmsg) {
+    // prettier-ignore
     Promise.resolve()
       .then(f)
-      .then(undefined, err => console.log(`doProcess: ${errmsg}:`, err));
+      .then(
+        undefined,
+        err => console.log(`doProcess: ${errmsg}: ${err.message}`),
+      );
     return waitUntilQuiescent();
   }
 
@@ -48,17 +51,14 @@ export function makeDeliver(tools, dispatch) {
     await runAndWait(() => dispatch[dispatchOp](...dispatchArgs), errmsg);
     stopGlobalMeter();
 
+    let status = ['ok'];
     // refill this vat's meter, if any, accumulating its usage for stats
     if (meterRecord) {
       // note that refill() won't actually refill an exhausted meter
       const used = meterRecord.refill();
       const exhaustionError = meterRecord.isExhausted();
       if (exhaustionError) {
-        // TODO: if the vat requested death-before-confusion, unwind this
-        // crank and pretend all its syscalls never happened
-        if (notifyTermination) {
-          notifyTermination(exhaustionError);
-        }
+        status = ['error', exhaustionError.message];
       } else {
         updateStats(used);
       }
@@ -70,12 +70,13 @@ export function makeDeliver(tools, dispatch) {
     // TODO: if the dispatch failed, and we choose to destroy the vat, change
     // what we do with the transcript here.
     transcriptManager.finishDispatch();
+    return status;
   }
 
   async function deliverOneMessage(targetSlot, msg) {
     insistMessage(msg);
     const errmsg = `vat[${vatID}][${targetSlot}].${msg.method} dispatch failed`;
-    await doProcess(
+    return doProcess(
       ['deliver', targetSlot, msg.method, msg.args, msg.result],
       errmsg,
     );

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

@@ -53,23 +53,13 @@ export function makeVatManagerFactory({
       'vatParameters',
       'vatConsole',
     ]);
-    const {
-      setup,
-      bundle,
-      enableSetup = false,
-      metered = false,
-      notifyTermination,
-    } = managerOptions;
+    const { setup, bundle, enableSetup = false } = managerOptions;
     assert(setup || bundle);
     assert(
       !bundle || typeof bundle === 'object',
       `bundle must be object, not a plain string`,
     );
     assert(!(setup && !enableSetup), `setup() provided, but not enabled`); // todo maybe useless
-    assert(
-      !(notifyTermination && !metered),
-      `notifyTermination is currently useless without metered:true`,
-    ); // explicit termination will change that
   }
 
   // returns promise for new vatManager

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

@@ -28,8 +28,7 @@ export function makeLocalVatManagerFactory(tools) {
   };
   // testLog is also a vatPower, only for unit tests
 
-  function prepare(vatID, managerOptions = {}) {
-    const { notifyTermination = undefined } = managerOptions;
+  function prepare(vatID) {
     const vatKeeper = kernelKeeper.getVatKeeper(vatID);
     const transcriptManager = makeTranscriptManager(
       kernelKeeper,
@@ -46,7 +45,6 @@ export function makeLocalVatManagerFactory(tools) {
         {
           vatID,
           stopGlobalMeter,
-          notifyTermination,
           meterRecord,
           refillAllMeters,
           transcriptManager,
@@ -65,7 +63,6 @@ export function makeLocalVatManagerFactory(tools) {
         setVatSyscallHandler,
         deliver,
         shutdown,
-        notifyTermination,
       });
       return manager;
     }
@@ -75,7 +72,6 @@ export function makeLocalVatManagerFactory(tools) {
   function createFromSetup(vatID, setup, managerOptions) {
     assert(!managerOptions.metered, `unsupported`);
     assert(!managerOptions.enableInternalMetering, `unsupported`);
-    assert(!managerOptions.notifyTermination, `unsupported`);
     assert(setup instanceof Function, 'setup is not an in-realm function');
     const { syscall, finish } = prepare(vatID, managerOptions);
 

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

@@ -30,7 +30,6 @@ export function makeNodeWorkerVatManagerFactory(tools) {
   function createFromBundle(vatID, bundle, managerOptions) {
     const { vatParameters } = managerOptions;
     assert(!managerOptions.metered, 'not supported yet');
-    assert(!managerOptions.notifyTermination, 'not supported yet');
     assert(!managerOptions.enableSetup, 'not supported at all');
     if (managerOptions.enableInternalMetering) {
       // TODO: warn+ignore, rather than throw, because the kernel enables it

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

@@ -87,7 +87,6 @@ export function createSyscall(transcriptManager) {
       // (and we'll be terminated, but the kernel and all other vats will
       // continue). Emit enough of an error message to explain the errors
       // that are about to ensue on our way down.
-      console.error(`syscall suffered error, shutdown commencing`);
       throw Error(`syscall suffered error, shutdown commencing`);
     }
     // otherwise vres is ['ok', null] or ['ok', capdata]

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

@@ -31,7 +31,6 @@ export function makeNodeSubprocessFactory(tools) {
   function createFromBundle(vatID, bundle, managerOptions) {
     const { vatParameters } = managerOptions;
     assert(!managerOptions.metered, 'not supported yet');
-    assert(!managerOptions.notifyTermination, 'not supported yet');
     assert(!managerOptions.enableSetup, 'not supported at all');
     if (managerOptions.enableInternalMetering) {
       // TODO: warn+ignore, rather than throw, because the kernel enables it