fix: tolerate decrefs that arrive after their vat has be deleted

Finalizer callbacks run at strange times, and can occur after the vat which
dropped the import has been terminated and cleaned up (so there are no c-list
entries left to remove). Ignore these.

The unit test is necessarily probablistic. This particular sequence appears
to exercise the previously-failing path about 50% of the time. The other 50%
of the time it does not, and the test passes without accomplishing anything
useful.

packages/SwingSet/src/kernel/kernel.js

@@ -146,11 +146,13 @@ export default function buildKernel(
 
   const pendingDecrefs = [];
   function decref(vatID, vref, count) {
-    assert(ephemeral.vats.has(vatID), `unknown vatID ${vatID}`);
+    if (!ephemeral.vats.has(vatID)) {
+      return; // vat was deleted already
+    }
     assert(count > 0, `bad count ${count}`);
     // TODO: decrement the clist import counter by 'count', then GC if zero
     if (testTrackDecref) {
-      console.log(`kernel decref [${vatID}].${vref} -= ${count}`);
+      // console.log(`kernel decref [${vatID}].${vref} -= ${count}`);
       pendingDecrefs.push({ vatID, vref, count });
     }
   }

packages/SwingSet/test/gc/boot-term.js

@@ -0,0 +1,24 @@
+import { E } from '@agoric/eventual-send';
+
+export function buildRootObject() {
+  let targetVat;
+  const theImport = harden({});
+  return harden({
+    async bootstrap(vats, devices) {
+      const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);
+      targetVat = await E(vatMaker).createVatByName('target');
+      // console.log(`target~.one(theImport)`);
+      // ignore() drops theImport immediately, making it available for GC
+    },
+
+    async ignore() {
+      await E(targetVat.root).ignore(theImport);
+      // console.log(`sent ignore() done`);
+    },
+
+    async terminate() {
+      await E(targetVat.adminNode).terminateWithFailure('bang');
+      // console.log(`sent terminateWithFailure() done`);
+    },
+  });
+}

packages/SwingSet/test/gc/test-gc.js

@@ -19,20 +19,26 @@ import { makePromiseKit } from '@agoric/promise-kit';
 import { buildVatController, buildKernelBundles } from '../../src/index';
 
 const haveGC = typeof gc === 'function' && typeof WeakRef === 'function';
-const testIfGC = haveGC ? test : test.skip;
+const testIfGC = haveGC ? test.serial : test.skip;
 
 test.before(async t => {
   const kernelBundles = await buildKernelBundles();
-  const bb = await bundleSource(path.resolve(__dirname, 'bootstrap.js'));
+  const bboot = await bundleSource(path.resolve(__dirname, 'boot.js'));
+  const bterm = await bundleSource(path.resolve(__dirname, 'boot-term.js'));
   const btarget = await bundleSource(path.resolve(__dirname, 'vat-target.js'));
-  const config = {
-    bootstrap: 'bootstrap',
+  const baseConfig = {
+    bundles: {
+      target: {
+        bundle: btarget,
+      },
+    },
     vats: {
-      bootstrap: { bundle: bb },
+      bootstrap: { bundle: bboot },
+      bootstrapTerminate: { bundle: bterm },
       target: { bundle: btarget },
     },
   };
-  t.context.data = { config, kernelBundles };
+  t.context.data = { baseConfig, kernelBundles };
 });
 
 function nextTick() {
@@ -48,7 +54,8 @@ function getDecrefSet(c) {
 }
 
 testIfGC('basic gc', async t => {
-  const { config, kernelBundles } = t.context.data;
+  const { baseConfig, kernelBundles } = t.context.data;
+  const config = { ...baseConfig, bootstrap: 'bootstrap' };
   const c = await buildVatController(config, [], {
     kernelBundles,
     testTrackDecref: true,
@@ -73,3 +80,44 @@ testIfGC('basic gc', async t => {
 
   t.pass();
 });
+
+testIfGC('decref on terminated vat', async t => {
+  const { baseConfig, kernelBundles } = t.context.data;
+  const config = { ...baseConfig, bootstrap: 'bootstrapTerminate' };
+  const c = await buildVatController(config, [], {
+    kernelBundles,
+    testTrackDecref: true,
+  });
+  // this creates a dynamic vat, with the same code as above
+  await c.run();
+
+  function capargs(args, slots = []) {
+    return harden({ body: JSON.stringify(args), slots });
+  }
+  function send(name) {
+    c.queueToVatExport('bootstrapTerminate', 'o+0', name, capargs([]), 'panic');
+  }
+
+  // We want to make sure the kernel decref() function can tolerate a late
+  // decref message that names a vat which is no longer present. If the test
+  // can finish without crashing, we're probably good. Like all
+  // non-deterministic things, this doesn't always exercise the bug:
+  // sometimes the decref arrives before the vat has been removed, and
+  // sometimes the vat (and the FinalizationRegistry) is destroyed before the
+  // decref is generated. This sequence seems to yield a 50% hit rate.
+
+  send('ignore');
+  await c.run();
+  gc();
+  send('terminate');
+  await c.run();
+  await nextTick();
+  await nextTick();
+
+  // The dynamic-vat decref (typically v8.o-50) only appears in decrefs if it
+  // arrives before the vat is terminated, which is the opposite of the case
+  // we're trying to test. In the ~50% of cases that decref() *after* the vat
+  // is terminated, decref() ignores the call, so it doesn't show up in the
+  // list. As a result, this tests passes if it completes without exceptions.
+  t.pass();
+});