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.

Many unit tests were updated to deal with the change in allocated IDs,
but they ought to be somewhat more stable now. Two of these updates
puzzled me:

* In `test-gc-kernel.js`, I understand why doomedExport1Vref changed
  from o+9 to o+10, and I'm not surprised that doomedExport2Vref is
  now o+11, but I don't understand why doomedExport2Vref was o+9
  before instead of o+10.
* In test-virtualObjectGC.js line 1297, why did the single- and
  multi- facet test cases start behaving the same way? Why were
  they different before?

refs #1848
  • Loading branch information
warner committed Mar 29, 2022
1 parent 104fbfe commit 3e5b090
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 67 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 @@ -658,6 +658,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 @@ -669,11 +678,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 @@ -711,6 +716,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
4 changes: 2 additions & 2 deletions packages/SwingSet/test/test-gc-kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ test('terminated vat', async t => {
let exports = Object.keys(vrefs).filter(vref => vref.startsWith('o+'));
exports.sort();
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 @@ -1107,7 +1107,7 @@ test('terminated vat', async t => {
exports = Object.keys(vrefs).filter(vref => vref.startsWith('o+'));
exports.sort();
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
34 changes: 15 additions & 19 deletions packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ function stateKey(vref) {
return `vom.${base(vref)}`;
}

const unfacetedThingKindID = 'o+9';
const facetedThingKindID = 'o+10';
const holderKindID = 'o+11';
const unfacetedThingKindID = 'o+10';
const facetedThingKindID = 'o+11';
const holderKindID = 'o+12';

const remotableID = 'o+14';

function thingVref(isf, instance) {
return `${isf ? facetedThingKindID : unfacetedThingKindID}/${instance}`;
Expand Down Expand Up @@ -454,19 +456,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(rcKey('o+5/1'), NONE));
validate(v, matchVatstoreSet(rcKey('o+5/1'), '1'));
validate(v, matchVatstoreSet('baggageID', 'o+6/1'));
validate(v, matchVatstoreGet(rcKey('o+6/1'), NONE));
validate(v, matchVatstoreSet(rcKey('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 Expand Up @@ -1157,7 +1161,7 @@ test.serial('VO multifacet markers only', async t => {
buildRootObject,
'bob',
);
const thing = 'o+12/1';
const thing = 'o+13/1';
const thingCapdata = JSON.stringify({ unused: capdata('uncared for') });

// lerv -> Lerv Create facets
Expand Down Expand Up @@ -1290,18 +1294,13 @@ async function voRefcountManagementTest3(t, isf) {
validate(v, matchVatstoreSet(stateKey(`${holderKindID}/4`), heldHolderValue(`${holderKindID}/3`)));
validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue));
validateReturned(v, rp);
if (isf) {
validate(v, matchVatstoreGet(rcKey(thing), '1'));
validate(v, matchVatstoreGet(esKey(thing), NONE));
}
validate(v, matchVatstoreGet(rcKey(thing), '1'));
validate(v, matchVatstoreGet(esKey(thing), NONE));

validate(v, matchVatstoreGet(rcKey(`${holderKindID}/2`), '1'));
validate(v, matchVatstoreGet(esKey(`${holderKindID}/2`), NONE));
validate(v, matchVatstoreGet(rcKey(`${holderKindID}/3`), '1'));
validate(v, matchVatstoreGet(esKey(`${holderKindID}/3`), NONE));
if (!isf) {
validate(v, matchVatstoreGet(rcKey(thing), '1'));
validate(v, matchVatstoreGet(esKey(thing), NONE));
}
validateDone(v);

rp = await dispatchMessage('finishDropHolders');
Expand Down Expand Up @@ -1432,7 +1431,6 @@ test.serial('presence refcount management 2', async t => {
// prettier-ignore
test.serial('remotable refcount management 1', async t => {
const { v, dispatchMessage } = await setupTestLiveslots(t, buildRootObject, 'bob');
const remotableID = 'o+13';

let rp = await dispatchMessage('makeAndHoldRemotable');
validateSetup(v);
Expand Down Expand Up @@ -1465,7 +1463,6 @@ test.serial('remotable refcount management 1', async t => {
// prettier-ignore
test.serial('remotable refcount management 2', async t => {
const { v, dispatchMessage } = await setupTestLiveslots(t, buildRootObject, 'bob');
const remotableID = 'o+13';

let rp = await dispatchMessage('makeAndHoldRemotable');
validateSetup(v);
Expand Down Expand Up @@ -1567,7 +1564,6 @@ test.serial('verify presence weak key GC', async t => {
test.serial('VO holding non-VO', async t => {
const { v, dispatchMessage, dispatchDropExports, dispatchRetireExports } =
await setupTestLiveslots(t, buildRootObject, 'bob');
const remotableID = 'o+13';

// lerv -> Lerv Create non-VO
let rp = await dispatchMessage('makeAndHoldRemotable');
Expand Down
Loading

0 comments on commit 3e5b090

Please sign in to comment.