Skip to content

Commit

Permalink
fix(swingset): initializeKindHandleKind early enough to support durables
Browse files Browse the repository at this point in the history
Previously, `kindIDID` (the ID of the virtual object Kind that is used
to hold the KindHandles we need for durable kinds) was allocated
on-demand, the first time `makeKindHandle()` was called. The ID it
received (and the ID of everything allocate afterwards) thus depended
upon if/when userspace decided to use `makeKindHandle()`.

In addition, `vrm.registerKind()` for the KindHandle kind was not
called until `kindIDID` itself was allocated. This doesn't necessarily
happen at all in the version-2 of a vat (i.e. if v2 doesn't define any
additional durable kinds), and can't be relied upon to happen before
v2 needs to deserialize the KindHandles that live in the 'baggage'.

So this commit changes liveslots to factor out the initialization and
registration into a new `initializeKindHandleKind` function, and
arranges to call it during `startVat()`. Several "fake" test harnesses
in `tools/` were updated to call it as they build their stuff.

test-gc-kernel.js was updated to sort the extracted vrefs numerically,
which was allowing the previous version of this test to pass despite
having the wrong object IDs.

refs #1848
  • Loading branch information
warner committed Mar 30, 2022
1 parent e897bff commit bcfd68d
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 70 deletions.
1 change: 1 addition & 0 deletions packages/SwingSet/src/liveslots/liveslots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,7 @@ function build(
}

initializeIDCounters();
vom.initializeKindHandleKind();
collectionManager.initializeStoreKindInfo();

const vatParameters = m.unserialize(vatParametersCapData);
Expand Down
16 changes: 11 additions & 5 deletions packages/SwingSet/src/liveslots/virtualObjectManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,15 @@ export function makeVirtualObjectManager(
let kindIDID;
const kindDescriptors = new WeakMap();

function initializeKindHandleKind() {
kindIDID = syscall.vatstoreGet('kindIDID');
if (!kindIDID) {
kindIDID = `${allocateExportID()}`;
syscall.vatstoreSet('kindIDID', kindIDID);
}
vrm.registerKind(kindIDID, reanimateDurableKindID, () => null, true);
}

function reanimateDurableKindID(vobjID, _proforma) {
const { subid: kindID } = parseVatSlot(vobjID);
const raw = syscall.vatstoreGet(`vom.kind.${kindID}`);
Expand All @@ -670,11 +679,7 @@ export function makeVirtualObjectManager(
}

const makeKindHandle = tag => {
if (!kindIDID) {
kindIDID = `${allocateExportID()}`;
syscall.vatstoreSet('kindIDID', kindIDID);
vrm.registerKind(kindIDID, reanimateDurableKindID, () => null, true);
}
assert(kindIDID, `initializeKindHandleKind not called yet`);
const kindID = `${allocateExportID()}`;
const kindIDvref = `o+${kindIDID}/${kindID}`;
const durableKindDescriptor = harden({ kindID, tag });
Expand Down Expand Up @@ -712,6 +717,7 @@ export function makeVirtualObjectManager(
};

return harden({
initializeKindHandleKind,
defineKind,
defineDurableKind,
makeKindHandle,
Expand Down
4 changes: 2 additions & 2 deletions packages/SwingSet/test/gc/test-gc-vat.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ test('forward to fake zoe', async t => {
console.log(`targetID: ${targetID}`);

// confirm that zoe is exporting it
t.is(findClist(c, zoeID, invitation), 'o+9');
t.true(dumpClist(c).includes(`${invitation}/${zoeID}/o+9`));
t.is(findClist(c, zoeID, invitation), 'o+10');
t.true(dumpClist(c).includes(`${invitation}/${zoeID}/o+10`));
// confirm that vat-target has not seen it yet
t.is(findClist(c, targetID, invitation), undefined);

Expand Down
20 changes: 11 additions & 9 deletions packages/SwingSet/test/stores/test-storeGC.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ function thingRefValString(vref) {
const nullValString = JSON.stringify({ body: 'null', slots: [] });

function mapRef(idx) {
return `o+1/${idx}`;
return `o+2/${idx}`; // see 'assert known scalarMapStore ID' below
}

function mapRefArg(idx) {
Expand Down Expand Up @@ -237,9 +237,9 @@ function validateCreateBaggage(v, idx) {
);
validate(v, matchVatstoreSet(`vc.${idx}.|schemata`, baggageSchema));
validate(v, matchVatstoreSet(`vc.${idx}.|label`, 'baggage'));
validate(v, matchVatstoreSet('baggageID', 'o+5/1'));
validate(v, matchVatstoreGet('vom.rc.o+5/1', NONE));
validate(v, matchVatstoreSet('vom.rc.o+5/1', '1'));
validate(v, matchVatstoreSet('baggageID', 'o+6/1'));
validate(v, matchVatstoreGet('vom.rc.o+6/1', NONE));
validate(v, matchVatstoreSet('vom.rc.o+6/1', '1'));
}

function validateCreate(v, idx, isWeak = false) {
Expand Down Expand Up @@ -466,12 +466,14 @@ function validateCreateHolder(v, idx) {

function validateInit(v) {
validate(v, matchVatstoreGet('idCounters', NONE));
validate(v, matchVatstoreGet('kindIDID', NONE));
validate(v, matchVatstoreSet('kindIDID', '1'));
validate(v, matchVatstoreGet('storeKindIDTable', NONE));
validate(
v,
matchVatstoreSet(
'storeKindIDTable',
'{"scalarMapStore":1,"scalarWeakMapStore":2,"scalarSetStore":3,"scalarWeakSetStore":4,"scalarDurableMapStore":5,"scalarDurableWeakMapStore":6,"scalarDurableSetStore":7,"scalarDurableWeakSetStore":8}',
'{"scalarMapStore":2,"scalarWeakMapStore":3,"scalarSetStore":4,"scalarWeakSetStore":5,"scalarDurableMapStore":6,"scalarDurableWeakMapStore":7,"scalarDurableSetStore":8,"scalarDurableWeakSetStore":9}',
),
);
validate(v, matchVatstoreGet('baggageID', NONE));
Expand Down Expand Up @@ -531,8 +533,8 @@ test.serial('assert known scalarMapStore ID', async t => {

const { testHooks } = await setupTestLiveslots(t, buildRootObject, 'bob', true);
const id = testHooks.obtainStoreKindID('scalarMapStore');
t.is(id, 1);
t.is(mapRef('INDEX'), 'o+1/INDEX');
t.is(id, 2);
t.is(mapRef('INDEX'), 'o+2/INDEX');
});

// test 1: lerv -> Lerv -> LerV -> Lerv -> lerv
Expand Down Expand Up @@ -1094,7 +1096,7 @@ test.serial('remotable refcount management 1', async t => {
);

const base = mainHeldIdx;
const remotableRef = 'o+9';
const remotableRef = 'o+10';

let rp = await dispatchMessage('makeAndHoldRemotable');
validateInit(v);
Expand Down Expand Up @@ -1126,7 +1128,7 @@ test.serial('remotable refcount management 2', async t => {
);

const base = mainHeldIdx;
const remotableRef = 'o+9';
const remotableRef = 'o+10';

let rp = await dispatchMessage('makeAndHoldRemotable');
validateInit(v);
Expand Down
10 changes: 6 additions & 4 deletions packages/SwingSet/test/test-baggage.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,21 @@ function validateCreateBaggage(v, idx) {
);
validate(v, matchVatstoreSet(`vc.${idx}.|schemata`, baggageSchema));
validate(v, matchVatstoreSet(`vc.${idx}.|label`, 'baggage'));
validate(v, matchVatstoreSet('baggageID', 'o+5/1'));
validate(v, matchVatstoreGet('vom.rc.o+5/1', NONE));
validate(v, matchVatstoreSet('vom.rc.o+5/1', '1'));
validate(v, matchVatstoreSet('baggageID', 'o+6/1'));
validate(v, matchVatstoreGet('vom.rc.o+6/1', NONE));
validate(v, matchVatstoreSet('vom.rc.o+6/1', '1'));
}

function validateSetup(v) {
validate(v, matchVatstoreGet('idCounters', NONE));
validate(v, matchVatstoreGet('kindIDID', NONE));
validate(v, matchVatstoreSet('kindIDID', '1'));
validate(v, matchVatstoreGet('storeKindIDTable', NONE));
validate(
v,
matchVatstoreSet(
'storeKindIDTable',
'{"scalarMapStore":1,"scalarWeakMapStore":2,"scalarSetStore":3,"scalarWeakSetStore":4,"scalarDurableMapStore":5,"scalarDurableWeakMapStore":6,"scalarDurableSetStore":7,"scalarDurableWeakSetStore":8}',
'{"scalarMapStore":2,"scalarWeakMapStore":3,"scalarSetStore":4,"scalarWeakSetStore":5,"scalarDurableMapStore":6,"scalarDurableWeakMapStore":7,"scalarDurableSetStore":8,"scalarDurableWeakSetStore":9}',
),
);
validate(v, matchVatstoreGet('baggageID', NONE));
Expand Down
31 changes: 26 additions & 5 deletions packages/SwingSet/test/test-gc-kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import anylogger from 'anylogger';
import { test } from '../tools/prepare-test-env-ava.js';

// eslint-disable-next-line import/order
import { assert } from '@agoric/assert';
import { waitUntilQuiescent } from '../src/lib-nodejs/waitUntilQuiescent.js';
import { createSHA256 } from '../src/lib-nodejs/hasher.js';

import { parseVatSlot } from '../src/lib/parseVatSlots.js';
import buildKernel from '../src/kernel/index.js';
import { initializeKernel } from '../src/controller/initializeKernel.js';
import {
Expand Down Expand Up @@ -1019,6 +1021,25 @@ test('message to self', async t => {
p.gcActionsAre([]);
});

function sortVrefs(vrefs) {
// returns ['o+8', 'o+9', 'o+10', 'o+11']
function num(vref) {
const p = parseVatSlot(vref);
assert.equal(p.type, 'object');
assert.equal(p.allocatedByVat, true);
assert.equal(p.virtual, false);
return p.id;
}
vrefs.sort((a, b) => Number(num(a) - num(b)));
}

test('sortVrefs', t => {
const v1 = ['o+11', 'o+9', 'o+10', 'o+8'];
const exp = ['o+8', 'o+9', 'o+10', 'o+11'];
sortVrefs(v1);
t.deepEqual(v1, exp);
});

test('terminated vat', async t => {
// when a vat is terminated, anything it imported should be dropped, and
// its exports should not cause problems when they are released
Expand Down Expand Up @@ -1082,9 +1103,9 @@ test('terminated vat', async t => {

// find the highest export: doomedExport1 / doomedExport1Presence
let exports = Object.keys(vrefs).filter(vref => vref.startsWith('o+'));
exports.sort();
sortVrefs(exports);
const doomedExport1Vref = exports[exports.length - 1];
t.is(doomedExport1Vref, 'o+9'); // arbitrary
t.is(doomedExport1Vref, 'o+10'); // arbitrary
const doomedExport1Kref = vrefs[doomedExport1Vref];
// this should also be deleted
// console.log(`doomedExport1Kref`, doomedExport1Kref);
Expand All @@ -1105,9 +1126,9 @@ test('terminated vat', async t => {
// now find the vref/kref for doomedExport2
vrefs = vrefsUsedByDoomed();
exports = Object.keys(vrefs).filter(vref => vref.startsWith('o+'));
exports.sort();
sortVrefs(exports);
const doomedExport2Vref = exports[exports.length - 1];
t.is(doomedExport2Vref, 'o+9'); // arbitrary
t.is(doomedExport2Vref, 'o+11'); // arbitrary
const doomedExport2Kref = vrefs[doomedExport2Vref];
[refcounts, owners] = getRefCountsAndOwners();
t.deepEqual(refcounts[doomedExport2Kref], [1, 1]); // from promise queue
Expand Down
6 changes: 3 additions & 3 deletions packages/SwingSet/test/test-liveslots.js
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ test('GC syscall.dropImports', async t => {
t.deepEqual(log.shift(), {
type: 'vatstoreSet',
key: 'idCounters',
value: '{"exportID":9,"collectionID":2,"promiseID":5}',
value: '{"exportID":10,"collectionID":2,"promiseID":5}',
});
const l2 = log.shift();
t.deepEqual(l2, {
Expand Down Expand Up @@ -1153,7 +1153,7 @@ test('GC dispatch.dropExports', async t => {
t.deepEqual(log.shift(), {
type: 'vatstoreSet',
key: 'idCounters',
value: '{"exportID":10,"collectionID":2,"promiseID":5}',
value: '{"exportID":11,"collectionID":2,"promiseID":5}',
});
t.deepEqual(log, []);

Expand Down Expand Up @@ -1220,7 +1220,7 @@ test('GC dispatch.retireExports inhibits syscall.retireExports', async t => {
t.deepEqual(log.shift(), {
type: 'vatstoreSet',
key: 'idCounters',
value: '{"exportID":10,"collectionID":2,"promiseID":5}',
value: '{"exportID":11,"collectionID":2,"promiseID":5}',
});
t.deepEqual(log, []);

Expand Down
4 changes: 2 additions & 2 deletions packages/SwingSet/test/test-vpid-liveslots.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ async function doVatResolveCase1(t, mode) {
const rootA = 'o+0';
const target1 = 'o-1';
const target2 = 'o-2';
const localTarget = 'o+9';
const localTarget = 'o+10';
const expectedP1 = 'p+5';
const expectedP2 = 'p+6';
const expectedP3 = 'p+7';
Expand Down Expand Up @@ -333,7 +333,7 @@ async function doVatResolveCase23(t, which, mode, stalls) {

const rootA = 'o+0';
const target1 = 'o-1';
const localTarget = 'o+9';
const localTarget = 'o+10';
const p1 = 'p-8';
const expectedP2 = 'p+5';
const expectedP3 = 'p+6';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ test('dead vat state removed', async t => {
const kvStore = hostStorage.kvStore;
t.is(kvStore.get('vat.dynamicIDs'), '["v6"]');
t.is(kvStore.get('ko26.owner'), 'v6');
t.is(Array.from(kvStore.getKeys('v6.', 'v6/')).length, 19);
t.is(Array.from(kvStore.getKeys('v6.', 'v6/')).length, 20);

controller.queueToVatRoot('bootstrap', 'phase2', capargs([]));
await controller.run();
Expand Down
25 changes: 13 additions & 12 deletions packages/SwingSet/test/virtualObjects/test-representatives.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ test.serial('exercise cache', async t => {
await doSimple('holdThing', what);
}
function dataKey(num) {
return `v1.vs.vom.o+9/${num}`;
return `v1.vs.vom.o+10/${num}`;
}
function esKey(num) {
return `v1.vs.vom.es.o+9/${num}`;
return `v1.vs.vom.es.o+10/${num}`;
}
function rcKey(num) {
return `v1.vs.vom.rc.o+9/${num}`;
return `v1.vs.vom.rc.o+10/${num}`;
}
function thingVal(name) {
return JSON.stringify({
Expand Down Expand Up @@ -381,21 +381,22 @@ test('virtual object gc', async t => {
remainingVOs[key] = hostStorage.kvStore.get(key);
}
t.deepEqual(remainingVOs, {
'v1.vs.baggageID': 'o+5/1',
'v1.vs.idCounters': '{"exportID":10,"collectionID":2,"promiseID":8}',
'v1.vs.baggageID': 'o+6/1',
'v1.vs.idCounters': '{"exportID":11,"collectionID":2,"promiseID":8}',
'v1.vs.kindIDID': '1',
'v1.vs.storeKindIDTable':
'{"scalarMapStore":1,"scalarWeakMapStore":2,"scalarSetStore":3,"scalarWeakSetStore":4,"scalarDurableMapStore":5,"scalarDurableWeakMapStore":6,"scalarDurableSetStore":7,"scalarDurableWeakSetStore":8}',
'{"scalarMapStore":2,"scalarWeakMapStore":3,"scalarSetStore":4,"scalarWeakSetStore":5,"scalarDurableMapStore":6,"scalarDurableWeakMapStore":7,"scalarDurableSetStore":8,"scalarDurableWeakSetStore":9}',
'v1.vs.vc.1.|entryCount': '0',
'v1.vs.vc.1.|label': 'baggage',
'v1.vs.vc.1.|nextOrdinal': '1',
'v1.vs.vc.1.|schemata':
'{"body":"[{\\"@qclass\\":\\"tagged\\",\\"tag\\":\\"match:kind\\",\\"payload\\":\\"string\\"}]","slots":[]}',
'v1.vs.vom.es.o+9/3': 'r',
'v1.vs.vom.o+9/2': '{"label":{"body":"\\"thing #2\\"","slots":[]}}',
'v1.vs.vom.o+9/3': '{"label":{"body":"\\"thing #3\\"","slots":[]}}',
'v1.vs.vom.o+9/8': '{"label":{"body":"\\"thing #8\\"","slots":[]}}',
'v1.vs.vom.o+9/9': '{"label":{"body":"\\"thing #9\\"","slots":[]}}',
'v1.vs.vom.rc.o+5/1': '1',
'v1.vs.vom.es.o+10/3': 'r',
'v1.vs.vom.o+10/2': '{"label":{"body":"\\"thing #2\\"","slots":[]}}',
'v1.vs.vom.o+10/3': '{"label":{"body":"\\"thing #3\\"","slots":[]}}',
'v1.vs.vom.o+10/8': '{"label":{"body":"\\"thing #8\\"","slots":[]}}',
'v1.vs.vom.o+10/9': '{"label":{"body":"\\"thing #9\\"","slots":[]}}',
'v1.vs.vom.rc.o+6/1': '1',
});
});

Expand Down
Loading

0 comments on commit bcfd68d

Please sign in to comment.