chore: update termination tests to watch for promise cleanup

Two tests are updated to exercise the cleanup of promise c-list
entries during vat termination. `terminate.test.js` adds some promises
to the c-list and then checks their refcounts after termination, to
demonstrate that bug #10261 is leaking a refcount when it deletes the
dead vat's c-list entry without also decrementing the
refcount. `slow-termination.test.js` adds a number of promises to the
c-list, and expects the budget-limited cleanup to spend a phase on
promises.

Both tests are marked as failing until the code fix is landed in the
next commit.

packages/SwingSet/test/vat-admin/slow-termination/bootstrap-slow-terminate.js

@@ -1,4 +1,5 @@
 import { Far, E } from '@endo/far';
+import { makePromiseKit } from '@endo/promise-kit';
 
 export function buildRootObject(_vatPowers) {
   let root;
@@ -21,11 +22,22 @@ export function buildRootObject(_vatPowers) {
       }
 
       // set up 20 "dude exports, bootstrap imports" c-list entries
-
       for (let i = 0; i < 20; i += 1) {
         myImports.push(await E(root).sendExport());
       }
 
+      // also 10 imported promises
+      for (let i = 0; i < 10; i += 1) {
+        await E(root).acceptImports(makePromiseKit().promise);
+      }
+
+      // and 10 exported promises
+      for (let i = 0; i < 10; i += 1) {
+        const p = E(root).forever();
+        myImports.push(p);
+        p.catch(_err => 0); // hush
+      }
+
       // ask dude to creates 20 vatstore entries (in addition to the
       // built-in liveslots stuff)
       await E(root).makeVatstore(20);

packages/SwingSet/test/vat-admin/slow-termination/slow-termination.test.js

@@ -63,7 +63,7 @@ async function doSlowTerminate(t, mode) {
     defaultManagerType: 'xsnap',
     defaultReapInterval: 'never',
     snapshotInitial: 2, // same as the default
-    snapshotInterval: 10, // ensure multiple spans+snapshots
+    snapshotInterval: 11, // ensure multiple spans+snapshots
     bootstrap: 'bootstrap',
     bundles: {
       dude: {
@@ -117,13 +117,16 @@ async function doSlowTerminate(t, mode) {
   t.is(countCleanups, 0);
 
   // bootstrap adds a fair amount of vat-dude state:
-  // * we have c-list entries for 20 imports and 20 exports, each of
-  //   which need two kvStore entries, so 80 kvStore total
+  // * we have c-list entries for 20 object imports and 20 object
+  //   exports, each of which need two kvStore entries, so 80 kvStore
+  //   total
+  // * c-list entries for 10 promise imports and 10 promise exports,
+  // * so 40 kvStore total
   // * the vat has created 20 baggage entries, all of which go into
   //   the vatstore, adding 20 kvStore
   // * an empty vat has about 29 kvStore entries just to track
   //   counters, the built-in collection types, baggage itself, etc
-  // * by sending 40-plus deliveries into an xsnap vat with
+  // * by sending 60-plus deliveries into an xsnap vat with
   //   snapInterval=5, we get 8-ish transcript spans (7 old, 1
   //   current), and each old span generates a heap snapshot record
   // Slow vat termination means deleting these entries slowly.
@@ -148,30 +151,48 @@ async function doSlowTerminate(t, mode) {
       .pluck()
       .get(vatID);
 
-  // 20*2 for imports, 21*2 for exports, 20*1 for vatstore = 102.
-  // Plus 21 for the usual liveslots stuff, and 6 for kernel stuff
-  // like vNN.source/options
+  // 20*2 for imports, 21*2 for exports, 20*2 for promises, 20*1 for
+  // vatstore = 142.  Plus 21 for the usual liveslots stuff, and 6 for
+  // kernel stuff like vNN.source/options
+  const initialKVCount = 169;
 
-  t.is(remainingKV().length, 129);
+  t.is(remainingKV().length, initialKVCount);
   t.false(JSON.parse(kvStore.get('vats.terminated')).includes(vatID));
+
   // we get one span for snapshotInitial (=2), then a span every
-  // snapshotInterval (=10). Each non-current span creates a
+  // snapshotInterval (=11). Each non-current span creates a
   // snapshot.
-  t.is(remainingTranscriptSpans(), 6);
-  t.is(remainingTranscriptItems(), 59);
-  t.is(remainingSnapshots(), 5);
+  let expectedRemainingItems = 85;
+  let expectedRemainingSpans = 8;
+  let expectedRemainingSnapshots = 7;
+
+  const checkTS = () => {
+    t.is(remainingTranscriptSpans(), expectedRemainingSpans);
+    t.is(remainingTranscriptItems(), expectedRemainingItems);
+    t.is(remainingSnapshots(), expectedRemainingSnapshots);
+  };
+  checkTS();
+
   const remaining = () =>
     remainingKV().length +
     remainingSnapshots() +
     remainingTranscriptItems() +
     remainingTranscriptSpans();
 
   // note: mode=dieHappy means we send one extra message to the vat,
-  // which adds a single transcript item (but this doesn't happen to trigger an extra span)
+  // but we've tuned snapshotInterval to avoid this triggering a BOYD
+  // and save/load snapshot, so it increases our expected transcript
+  // items, but leaves the spans/snapshots alone
+  if (mode === 'dieHappy') {
+    expectedRemainingItems += 1;
+  }
 
   const kpid = controller.queueToVatRoot('bootstrap', 'kill', [mode]);
   await controller.run(noCleanupPolicy);
   await commit();
+
+  checkTS();
+
   t.is(controller.kpStatus(kpid), 'fulfilled');
   t.deepEqual(
     controller.kpResolution(kpid),
@@ -182,7 +203,7 @@ async function doSlowTerminate(t, mode) {
   t.true(JSON.parse(kvStore.get('vats.terminated')).includes(vatID));
   // no cleanups were allowed, so nothing should be removed yet
   t.truthy(kernelStorage.kvStore.get(`${vatID}.options`));
-  t.is(remainingKV().length, 129);
+  t.is(remainingKV().length, initialKVCount);
 
   // now do a series of cleanup runs, each with budget=5
   const clean = async (budget = 5) => {
@@ -223,8 +244,16 @@ async function doSlowTerminate(t, mode) {
   // the non-clist kvstore keys should still be present
   t.truthy(kernelStorage.kvStore.get(`${vatID}.options`));
 
-  // there are no imports, so this clean(budget.default=5) will delete
-  // the first five of our 47 other kv entries (20 vatstore plus 27
+  // there are no remaining imports, so this clean(budget.default=5)
+  // will delete the first five of our promise c-list entries, each
+  // with two kv entries
+  await cleanKV(10, 5); // 5 c-list promises
+  await cleanKV(10, 5); // 5 c-list promises
+  await cleanKV(10, 5); // 5 c-list promises
+  await cleanKV(10, 5); // 5 c-list promises
+
+  // that finishes the promises, so the next clean will delete the
+  // first five of our 47 other kv entries (20 vatstore plus 27
   // liveslots overhead
 
   await cleanKV(5, 5); // 5 other kv
@@ -241,54 +270,55 @@ async function doSlowTerminate(t, mode) {
   t.is(remainingKV().length, 22);
   await cleanKV(5, 5); // 5 other kv
   t.is(remainingKV().length, 17);
-  t.is(remainingSnapshots(), 5);
+  checkTS();
   await cleanKV(5, 5); // 5 other kv
   t.is(remainingKV().length, 12);
   await cleanKV(5, 5); // 5 other kv
   t.is(remainingKV().length, 7);
   await cleanKV(5, 5); // 5 other kv
   t.is(remainingKV().length, 2);
-  t.is(remainingSnapshots(), 5);
 
-  // there are two kv left, so this clean will delete those, then all 5 snapshots
+  checkTS();
+
+  // there are two kv left, so this clean will delete those, then 5 of
+  // the 7 snapshots
   await cleanKV(2, 7); // 2 final kv, and 5 snapshots
   t.deepEqual(remainingKV(), []);
   t.is(kernelStorage.kvStore.get(`${vatID}.options`), undefined);
-  t.is(remainingSnapshots(), 0);
-
-  t.is(remainingTranscriptSpans(), 6);
-  let ts = 59;
-  if (mode === 'dieHappy') {
-    ts = 60;
-  }
-  t.is(remainingTranscriptItems(), ts);
-
-  // the next clean (with the default budget of 5) will delete the
-  // five most recent transcript spans, starting with the isCurrent=1
-  // one (which had 9 or 10 items), leaving just the earliest (which
-  // had 4, due to `snapshotInitial`)
+  expectedRemainingSnapshots -= 5;
+  checkTS();
+
+  // the next clean gets the 2 remaining snapshots, and the five most
+  // recent transcript spans, starting with the isCurrent=1 one (which
+  // had 9 or 10 items), leaving the earliest (which had 4, due to
+  // `snapshotInitial`) and the next two (with 13 each, due to
+  // snapshotInterval plus 2 for BOYD/snapshot overhead ).
   let cleanups = await clean();
-  t.is(cleanups, 5);
-  t.is(remainingTranscriptSpans(), 1);
-  t.is(remainingTranscriptItems(), 4);
+
+  t.is(cleanups, expectedRemainingSnapshots + 5);
+  expectedRemainingSnapshots = 0;
+  expectedRemainingItems = 4 + 13 + 13;
+  expectedRemainingSpans = 3;
+  checkTS();
   // not quite done
   t.true(JSON.parse(kvStore.get('vats.terminated')).includes(vatID));
 
-  // the final clean deletes the remaining span, and finishes by
+  // the final clean deletes the remaining spans, and finishes by
   // removing the "still being deleted" bookkeeping, and the .options
 
   cleanups = await clean();
-  t.is(cleanups, 1);
-  t.is(remainingTranscriptSpans(), 0);
-  t.is(remainingTranscriptItems(), 0);
+  t.is(cleanups, 3);
+  expectedRemainingItems = 0;
+  expectedRemainingSpans = 0;
+  checkTS();
   t.is(remaining(), 0);
   t.false(JSON.parse(kvStore.get('vats.terminated')).includes(vatID));
 }
 
-test.serial('slow terminate (kill)', async t => {
+test.serial.failing('slow terminate (kill)', async t => {
   await doSlowTerminate(t, 'kill');
 });
 
-test.serial('slow terminate (die happy)', async t => {
+test.serial.failing('slow terminate (die happy)', async t => {
   await doSlowTerminate(t, 'dieHappy');
 });

packages/SwingSet/test/vat-admin/slow-termination/vat-slow-terminate.js

@@ -1,4 +1,5 @@
 import { Far } from '@endo/far';
+import { makePromiseKit } from '@endo/promise-kit';
 
 export function buildRootObject(vatPowers, _vatParameters, baggage) {
   const hold = [];
@@ -7,6 +8,7 @@ export function buildRootObject(vatPowers, _vatParameters, baggage) {
     dieHappy: completion => vatPowers.exitVat(completion),
     sendExport: () => Far('dude export', {}),
     acceptImports: imports => hold.push(imports),
+    forever: () => makePromiseKit().promise,
     makeVatstore: count => {
       for (let i = 0; i < count; i += 1) {
         baggage.init(`key-${i}`, i);

packages/SwingSet/test/vat-admin/terminate/bootstrap-die-cleanly.js

@@ -1,16 +1,24 @@
 import { Far, E } from '@endo/far';
+import { makePromiseKit } from '@endo/promise-kit';
 
 export function buildRootObject() {
   let dude;
+  const hold = [];
 
   const self = Far('root', {
     async bootstrap(vats, devices) {
       const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);
 
-      // create a dynamic vat, send it a message and let it respond, to make
-      // sure everything is working
+      // Create a dynamic vat, send it a message and let it respond,
+      // to make sure everything is working. Give them a promise to
+      // follow, to check that its refcount is cleaned up.
       dude = await E(vatMaker).createVatByName('dude');
       await E(dude.root).foo(1);
+      const p = makePromiseKit().promise;
+      await E(dude.root).holdPromise(p);
+      const p2 = E(dude.root).never();
+      p2.catch(_err => 'hush');
+      hold.push(p2);
       return 'bootstrap done';
     },
     async phase2() {

packages/SwingSet/test/vat-admin/terminate/terminate.test.js

@@ -449,7 +449,7 @@ test.serial('invalid criticalVatKey causes vat creation to fail', async t => {
   });
 });
 
-test.serial('dead vat state removed', async t => {
+test.serial.failing('dead vat state removed', async t => {
   const configPath = new URL('swingset-die-cleanly.json', import.meta.url)
     .pathname;
   const config = await loadSwingsetConfigFile(configPath);
@@ -471,6 +471,23 @@ test.serial('dead vat state removed', async t => {
   t.is(kvStore.get('vat.dynamicIDs'), '["v6"]');
   t.is(kvStore.get('ko26.owner'), 'v6');
   t.is(Array.from(enumeratePrefixedKeys(kvStore, 'v6.')).length > 10, true);
+  // find the two promises in the new vat's c-list, they should both
+  // have refCount=2 (one for bootstrap, one for the dynamic vat)
+  let decidedKPID;
+  let subscribedKPID;
+  for (const key of enumeratePrefixedKeys(kvStore, 'v6.')) {
+    if (key.startsWith('v6.c.kp')) {
+      const kpid = key.slice('v6.c.'.length);
+      const refCount = Number(kvStore.get(`${kpid}.refCount`));
+      const decider = kvStore.get(`${kpid}.decider`);
+      if (decider === 'v6') {
+        decidedKPID = kpid;
+      } else {
+        subscribedKPID = kpid;
+      }
+      t.is(refCount, 2, `${kpid}.refCount=${refCount}, not 2`);
+    }
+  }
   const beforeDump = debug.dump(true);
   t.truthy(beforeDump.transcripts.v6);
   t.truthy(beforeDump.snapshots.v6);
@@ -483,6 +500,16 @@ test.serial('dead vat state removed', async t => {
   const afterDump = debug.dump(true);
   t.falsy(afterDump.transcripts.v6);
   t.falsy(afterDump.snapshots.v6);
+  // the promise that v6 was deciding will be removed from v6's
+  // c-list, rejected by the kernel, and the reject notification to
+  // vat-bootstrap will remove it from that c-list, so the end state
+  // should be no references, and a completely deleted kernel promise
+  // table entry
+  t.is(kvStore.get(`${decidedKPID}.refCount`), undefined);
+  // the promise that v6 received from vat-bootstrap should be removed
+  // from v6's c-list, but it is still being exported by
+  // vat-bootstrap, so the refcount should finish at 1
+  t.is(kvStore.get(`${subscribedKPID}.refCount`), '1');
 });
 
 test.serial('terminate with presence', async t => {

packages/SwingSet/test/vat-admin/terminate/vat-dude-terminate.js

@@ -6,12 +6,18 @@ export function buildRootObject(vatPowers) {
   // to be cut off
   const { testLog } = vatPowers;
 
+  const hold = [];
+
   return Far('root', {
     foo(arg) {
       testLog(`FOO ${arg}`);
       return `FOO SAYS ${arg}`;
     },
 
+    holdPromise(p) {
+      hold.push(p);
+    },
+
     never() {
       return makePromiseKit().promise; // never fires
     },