Skip to content

Commit

Permalink
rewrite scanForDeadObjects to avoid retire-without-drop (#9942)
Browse files Browse the repository at this point in the history
Rewrite scanForDeadObjects(), which is called during
dispatch.bringOutYourDead to process possiblyDeadSet and
possiblyRetiredSet. The new flow should be easier to review and
understand.

The main behavioral difference is to fix a bug (#9939) in which a vref
that appears in possiblyRetiredSet (because e.g. a weak collection was
deleted, which was using that vref as a key), but which 1: lacks a RAM
pillar (Presence object) and 2: was not dropped in this BOYD (e.g. it
has a vdata pillar), used to be sent to the kernel in a bogus
`syscall.retireImports()` call. Because this vref was not previously
dropped by the vat (syscall.dropImports()), this was a vat-fatal
error.

The new code will only retire such a Presence vref if it was not
reachable by the vat.

fixes #9939
  • Loading branch information
mergify[bot] authored Aug 30, 2024
2 parents c56c51e + c045163 commit ef9ed26
Show file tree
Hide file tree
Showing 9 changed files with 922 additions and 169 deletions.
5 changes: 5 additions & 0 deletions packages/SwingSet/test/gc/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,10 @@ export function buildRootObject() {
async makeInvitation0() {
await E(target).makeInvitationTarget(zoe);
},

// for #9939
async storePresenceInWeakSet() {
await E(target).store(A);
},
});
}
81 changes: 81 additions & 0 deletions packages/SwingSet/test/gc/gc-vat.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,84 @@ test('forward to fake zoe', async t => {
// 'makeInvitationTarget' result promise with it, then dropped it
t.is(findClist(c, targetID, invitation), undefined);
});

// see #9939
test('drop without retire', async t => {
let targetID;
let didBOYD = false;
// const msgs = ['deliver', 'deliver-result', 'syscall', 'syscall-result'];

function slogSender(slogObj) {
const {
time: _1,
replay: _2,
crankNum: _3,
deliveryNum: _4,
monotime: _5,
...o
} = slogObj;
if (o.vatID !== targetID) return;
if (o.type === 'deliver' && o.kd[0] === 'bringOutYourDead') {
didBOYD = true;
}
// if (msgs.includes(o.type)) console.log(JSON.stringify(o));
}
const config = {
bootstrap: 'bootstrap', // v6
vats: {
bootstrap: {
// v6
sourceSpec: new URL('bootstrap.js', import.meta.url).pathname,
},
target: {
// v1
sourceSpec: new URL('vat-target.js', import.meta.url).pathname,
// avoid V8's GC nondeterminism, only needed on the target vat
creationOptions: { managerType: 'xs-worker' },
},
},
};
const kernelStorage = initSwingStore().kernelStorage;
await initializeSwingset(config, [], kernelStorage);
const c = await makeSwingsetController(kernelStorage, {}, { slogSender });
t.teardown(c.shutdown);

c.pinVatRoot('bootstrap');
targetID = c.vatNameToID('target');
c.pinVatRoot('target');

await c.run();

c.queueToVatRoot('bootstrap', 'storePresenceInWeakSet', []);
await c.run();

// now do enough dummy messages to trigger a new BOYD
didBOYD = false;
while (!didBOYD) {
c.queueToVatRoot('target', 'dummy', []);
await c.run();
}

// now tell vat-target to drop its WeakSet
c.queueToVatRoot('target', 'drop', []);
await c.run();

// and trigger a second BOYD
didBOYD = false;
while (!didBOYD) {
// this will fail once the vat is terminated
try {
c.queueToVatRoot('target', 'dummy', []);
} catch (e) {
if (/vat name "target" must exist/.test(e.message)) {
t.fail('vat terminated, bug is present');
break;
}
}
await c.run();
}
t.true(didBOYD);

// the test passes if the vat survived
return t.pass();
});
14 changes: 13 additions & 1 deletion packages/SwingSet/test/gc/vat-target.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { Far, E } from '@endo/far';

export function buildRootObject() {
export function buildRootObject(_vatPowers, _vatParameters, baggage) {
/** @type { WeakSet | undefined } */
let ws = new WeakSet();

return Far('root', {
async two(A, B) {
// A=ko26 B=ko27
Expand All @@ -10,5 +13,14 @@ export function buildRootObject() {
makeInvitationTarget(zoe) {
return E(zoe).makeInvitationZoe();
},

// these three are for testing #9939
store: x => {
baggage.init('x', x);
assert(ws);
ws.add(x);
},
dummy: () => 0,
drop: () => (ws = undefined),
});
}
4 changes: 1 addition & 3 deletions packages/boot/test/orchestration/restart-contracts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ test.serial('stakeAtom', async t => {
// restarting that one. For them to share bootstrap they'll each need a unique
// instance name, which will require paramatizing the the two builders scripts
// and the two core-eval functions.
//
// TODO(#9939): Flaky under Node.js until liveslots problem exposed by vows is fixed.
test.serial.skip('basicFlows', async t => {
test.serial('basicFlows', async t => {
const {
walletFactoryDriver,
buildProposal,
Expand Down
Loading

0 comments on commit ef9ed26

Please sign in to comment.