Skip to content

Commit

Permalink
fix(swingset): fix two tests which failed under XS GC
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
warner committed Jun 11, 2021
1 parent 472caf8 commit 5436e67
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
2 changes: 2 additions & 0 deletions packages/SwingSet/test/basedir-promises-3/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions packages/SwingSet/test/test-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()']);
Expand Down Expand Up @@ -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']);
Expand All @@ -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()',
Expand All @@ -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()',
Expand All @@ -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()',
Expand Down

0 comments on commit 5436e67

Please sign in to comment.