test(swingset): improve/re-enable gc tests

This adds a new (failing) test of #7212, enhances some other tests to
cover the same thing, and uncomments a portion of upgrade.test.js
which was commented out when we discovered the bug.

These will only pass when the kernel properly retires unreachable
objects that have just been abandoned by their owning vat.

The new test (gc-kernel-orphan.test.js) also checks that vat
termination on the same crank that retires an object will not cause a
panic.

packages/SwingSet/test/abandon-export.test.js

@@ -5,15 +5,19 @@ import { test } from '../tools/prepare-test-env-ava.js';
 import buildKernel from '../src/kernel/index.js';
 import { initializeKernel } from '../src/controller/initializeKernel.js';
 import { extractMethod } from '../src/lib/kdebug.js';
+import makeKernelKeeper from '../src/kernel/state/kernelKeeper.js';
 import { makeKernelEndowments, buildDispatch } from './util.js';
 import { kser, kunser, kslot } from '@agoric/kmarshal';
 
 const makeKernel = async () => {
   const endowments = makeKernelEndowments();
-  const { kvStore } = endowments.kernelStorage;
-  await initializeKernel({}, endowments.kernelStorage);
+  const { kernelStorage } = endowments;
+  const { kvStore } = kernelStorage;
+  await initializeKernel({}, kernelStorage);
   const kernel = buildKernel(endowments, {}, {});
-  return { kernel, kvStore };
+  const kernelKeeper = makeKernelKeeper(kernelStorage, null);
+  kernelKeeper.loadStats();
+  return { kernel, kvStore, kernelKeeper };
 };
 
 /**
@@ -43,7 +47,7 @@ const assertSingleEntryLog = (t, log, type, details = {}) => {
   return entry;
 };
 
-const makeTestVat = async (t, kernel, name) => {
+const makeTestVat = async (t, kernel, name, kernelKeeper) => {
   const { log, dispatch } = buildDispatch();
   let syscall;
   const setup = providedSyscall => {
@@ -56,19 +60,29 @@ const makeTestVat = async (t, kernel, name) => {
   const kref = kernel.getRootObject(vatID);
   kernel.pinObject(kref);
   const flushDeliveries = async () => {
-    // Make a dummy delivery so the kernel will call processRefcounts().
-    // This isn't normally needed, but we're directly making syscalls
-    // outside of a delivery.
-    // If that ever stops working, we can update `makeTestVat` to return
-    // functions supporting more true-to-production vat behavior like
-    // ```js
-    // enqueueSyscall('send', target, kser([...args]), resultVPID);
-    // enqueueSyscall('subscribe', resultVPID);
-    // flushSyscalls(); // issues queued syscalls inside a dummy delivery
-    // ```
-    kernel.queueToKref(kref, 'flush', []);
+    // This test uses a really lazy+sketchy approach to triggering
+    // syscalls: it just grabs the vat's `syscall` object and invokes
+    // it, from outside a crank. We're exercising refcount-changing
+    // syscalls, but the kernel only runs processRefcounts() after a
+    // real crank (i.e. inside processDeliveryMessage and
+    // processAcceptanceMessage). We need to trigger this, but we want
+    // to avoid making a real delivery to one of our vats, so their
+    // delivery logs won't be cluttered with our dummy
+    // processRefcounts-triggering activity
+    //
+    // The previous hack was to do a queueToKref(), and then try to
+    // remove the resulting delivery from the stream.
+    //
+    // The new hack is to inject a `{ type: 'negated-gc-action' }`
+    // onto the run-queue, which is handled by processDeliveryMessage
+    // but doesn't actually delivery anything.
+    //
+    // A safer approach would be to define the vat's dispatch() to
+    // listen for some messages that trigger each of the syscalls we
+    // want to invoke
+    //
+    kernelKeeper.addToRunQueue({ type: 'negated-gc-action' });
     await kernel.run();
-    assertFirstLogEntry(t, log, 'deliver', { method: 'flush' });
   };
   return { vatID, kref, dispatch, log, syscall, flushDeliveries };
 };
@@ -78,7 +92,7 @@ async function doAbandon(t, reachable) {
   // vatB abandons it
   // vatA should retain the object
   // sending to the abandoned object should get an error
-  const { kernel, kvStore } = await makeKernel();
+  const { kernel, kvStore, kernelKeeper } = await makeKernel();
   await kernel.start();
 
   const {
@@ -87,13 +101,14 @@ async function doAbandon(t, reachable) {
     log: logA,
     syscall: syscallA,
     flushDeliveries: flushDeliveriesA,
-  } = await makeTestVat(t, kernel, 'vatA');
+  } = await makeTestVat(t, kernel, 'vatA', kernelKeeper);
   const {
     vatID: vatB,
     kref: bobKref,
     log: logB,
     syscall: syscallB,
-  } = await makeTestVat(t, kernel, 'vatB');
+    flushDeliveries: flushDeliveriesB,
+  } = await makeTestVat(t, kernel, 'vatB', kernelKeeper);
   await kernel.run();
 
   // introduce B to A, so it can send 'holdThis' later
@@ -142,16 +157,24 @@ async function doAbandon(t, reachable) {
 
   // now have vatB abandon the export
   syscallB.abandonExports([targetForBob]);
-  await flushDeliveriesA();
-
-  // no GC messages for either vat
-  t.deepEqual(logA, []);
-  t.deepEqual(logB, []);
-
+  await flushDeliveriesB();
   targetOwner = kvStore.get(`${targetKref}.owner`);
   targetRefCount = kvStore.get(`${targetKref}.refCount`);
-  t.is(targetOwner, undefined);
-  t.is(targetRefCount, expectedRefCount); // unchanged
+
+  if (reachable) {
+    // no GC messages for either vat
+    t.deepEqual(logA, []);
+    t.deepEqual(logB, []);
+    t.is(targetOwner, undefined);
+    t.is(targetRefCount, expectedRefCount); // unchanged
+  } else {
+    // 'target' was orphaned and unreachable, kernel will delete it,
+    // so A will get a retireImports now
+    assertSingleEntryLog(t, logA, 'retireImports', { vrefs: [targetForAlice] });
+    t.deepEqual(logB, []);
+    t.is(targetOwner, undefined);
+    t.is(targetRefCount, undefined);
+  }
 
   if (reachable) {
     // vatA can send a message, but it will reject
@@ -180,17 +203,11 @@ async function doAbandon(t, reachable) {
     await flushDeliveriesA();
     // vatB should not get a dispatch.dropImports
     t.deepEqual(logB, []);
-    // the object still exists, now only recognizable
-    targetRefCount = kvStore.get(`${targetKref}.refCount`);
-    expectedRefCount = '0,1';
-    t.is(targetRefCount, expectedRefCount); // merely recognizable
+    // the kernel automatically retires the orphaned
+    // now-merely-recognizable object
+    t.deepEqual(logA, [{ type: 'retireImports', vrefs: [targetForAlice] }]);
+    // which deletes it
   }
-
-  // now vatA retires the object too
-  syscallA.retireImports([targetForAlice]);
-  await flushDeliveriesA();
-  // vatB should not get a dispatch.retireImports
-  t.deepEqual(logB, []);
   // the object no longer exists
   targetRefCount = kvStore.get(`${targetKref}.refCount`);
   expectedRefCount = undefined;

packages/SwingSet/test/gc-dead-vat/bootstrap.js

@@ -1,5 +1,6 @@
 import { Far, E } from '@endo/far';
 import { makePromiseKit } from '@endo/promise-kit';
+import { makeScalarBigWeakSetStore } from '@agoric/vat-data';
 
 async function sendExport(doomedRoot) {
   const exportToDoomed = Far('exportToDoomed', {});
@@ -11,6 +12,7 @@ export function buildRootObject() {
   let doomedRoot;
   const pin = [];
   const pk1 = makePromiseKit();
+  const wh = makeScalarBigWeakSetStore('weak-holder');
   return Far('root', {
     async bootstrap(vats, devices) {
       const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);
@@ -19,6 +21,8 @@ export function buildRootObject() {
       await sendExport(doomedRoot);
       const doomedExport1Presence = await E(doomedRoot).getDoomedExport1();
       pin.push(doomedExport1Presence);
+      const doomedExport3Presence = await E(doomedRoot).getDoomedExport3();
+      wh.add(doomedExport3Presence);
     },
     async stash() {
       // Give vat-doomed a target that doesn't resolve one() right away.

packages/SwingSet/test/gc-dead-vat/vat-doomed.js

@@ -4,6 +4,7 @@ export function buildRootObject(vatPowers) {
   const pin = [];
   const doomedExport1 = Far('doomedExport1', {});
   const doomedExport2 = Far('doomedExport2', {});
+  const doomedExport3 = Far('doomedExport3', {});
   return Far('root', {
     accept(exportToDoomedPresence) {
       pin.push(exportToDoomedPresence);
@@ -14,6 +15,9 @@ export function buildRootObject(vatPowers) {
     stashDoomedExport2(target) {
       E(E(target).one()).neverCalled(doomedExport2);
     },
+    getDoomedExport3() {
+      return doomedExport3;
+    },
     terminate() {
       vatPowers.exitVat('completion');
     },