From 5436e6783158c5b9a67cfd5bda58f924b18ff296 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 11 Jun 2021 13:34:36 -0700
Subject: [PATCH] fix(swingset): fix two tests which failed under XS GC

test-controller.js uses `c.step()` to examine the kernel one crank at a time,
but was written before some of those cranks are really GC actions. XS seems
to do a pretty thorough job of GC when asked, so the kernel has several to
do, and the test did not see the regular vat delivery progress it was
expecting. The fix is to step through all GC actions before taking the one
vat-delivery `c.step()` it intended to do.

test-promises.js "refcount while queued" failed because the bootstrap method
under test did not pin all vats in place. As a result, by the time the test
tried to `c.queueToVatExport()` a message to vat-right, vat-right's root
object had been collected. The fix is easy: make `bootstrap()` pin
`vats.right` in place.

The interesting question is why this did not fail under Node.js
(`managerType='local'`). Apparently Node.js is less inclined to release
objects, even when a full `gc()` is invoked. I suspect some pattern of
definition hoisting or optimization is keeping the `vats` variable around
longer than the source code would suggest. Might be related to #3240, but I
haven't been able to find a `gc()` mode that causes Node.js to collect it.
---
 .../test/basedir-promises-3/bootstrap.js      |  2 ++
 packages/SwingSet/test/test-controller.js     | 21 ++++++++++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/packages/SwingSet/test/basedir-promises-3/bootstrap.js b/packages/SwingSet/test/basedir-promises-3/bootstrap.js
index 6170887298b6..b6241847887c 100644
--- a/packages/SwingSet/test/basedir-promises-3/bootstrap.js
+++ b/packages/SwingSet/test/basedir-promises-3/bootstrap.js
@@ -4,8 +4,10 @@ import { Far } from '@agoric/marshal';
 
 export function buildRootObject() {
   const pk1 = makePromiseKit();
+  const pin = [];
   return Far('root', {
     bootstrap(vats) {
+      pin.push(vats.right); // pin so test can send 'three' to it later
       const p2 = E(vats.right).one(); // p2 is kp41
       E(p2).four(pk1.promise);
       // that puts an unresolved promise in the arguments of the promise
diff --git a/packages/SwingSet/test/test-controller.js b/packages/SwingSet/test/test-controller.js
index a9eaa0bf84b6..091045bca298 100644
--- a/packages/SwingSet/test/test-controller.js
+++ b/packages/SwingSet/test/test-controller.js
@@ -223,9 +223,20 @@ test('bootstrap export', async t => {
     },
   ]);
 
+  // this test was designed before GC, and wants to single-step the kernel,
+  // but doesn't care about the GC action steps, so we use this helper
+  // function
+  async function stepGC() {
+    while (c.dump().gcActions.length) {
+      // eslint-disable-next-line no-await-in-loop
+      await c.step();
+    }
+    await c.step(); // the non- GC action
+  }
+
   t.deepEqual(c.dump().log, []);
   // console.log('--- c.step() running bootstrap.obj0.bootstrap');
-  await c.step();
+  await stepGC();
   // kernel promise for result of the foo() that bootstrap sends to vat-left
   const fooP = 'kp41';
   t.deepEqual(c.dump().log, ['bootstrap.obj0.bootstrap()']);
@@ -254,7 +265,7 @@ test('bootstrap export', async t => {
     },
   ]);
 
-  await c.step();
+  await stepGC();
   const barP = 'kp42';
   t.deepEqual(c.dump().log, ['bootstrap.obj0.bootstrap()', 'left.foo 1']);
   kt.push([right0, leftVatID, 'o-50']);
@@ -277,7 +288,7 @@ test('bootstrap export', async t => {
     { type: 'notify', vatID: bootstrapVatID, kpid: fooP },
   ]);
 
-  await c.step();
+  await stepGC();
 
   t.deepEqual(c.dump().log, [
     'bootstrap.obj0.bootstrap()',
@@ -292,7 +303,7 @@ test('bootstrap export', async t => {
     { type: 'notify', vatID: leftVatID, kpid: barP },
   ]);
 
-  await c.step();
+  await stepGC();
 
   t.deepEqual(c.dump().log, [
     'bootstrap.obj0.bootstrap()',
@@ -306,7 +317,7 @@ test('bootstrap export', async t => {
     { type: 'notify', vatID: leftVatID, kpid: barP },
   ]);
 
-  await c.step();
+  await stepGC();
 
   t.deepEqual(c.dump().log, [
     'bootstrap.obj0.bootstrap()',