Skip to content

Commit

Permalink
fix(swingset): retain vatParameters for vat creation and upgrade
Browse files Browse the repository at this point in the history
All vats have `vatParameters`, a mostly-arbitrary data record,
delivered as the second argument to their `buildRootObject()`
function. Dynamic vats get their vatParameters from the options bag
given to `E(vatAdminSvc).createVat()`. Static vats get them from the
kernel config record. When a vat is upgraded, the new incarnation gets
new vatParameters; these come from the options bag on
`E(adminNode).upgrade()`.

When received via `createVat()` or `upgrade()`, the vatParameters can
contain object and device references. VatParameters cannot include
promises.

Previously, the kernel delivered vatParameters to the vat, but did not
keep a copy. With this commit, the kernel retains a copy of
vatParameters (including a refcount on any kernel objects
therein). Internally, `vatKeeper.getVatParameters()` can be used to
retrieve this copy.  Only vats created or upgraded after this commit
lands will get retained vatParameters: for older vats this will return
`undefined`.

Retained vat parameters should make it easier to implement "upgrade
all vats", where the kernel perform a unilateral `upgrade()` on all
vats without userspace asking for it. When this is implemented, the
new incarnations will receive the same vatParameters as their
predecessors.

The slow-termination test was updated: it counts kvStore entries
precisely as we delete them all, so it requires an update each time we
add one.

fixes #8947
  • Loading branch information
warner committed Oct 27, 2024
1 parent 9c73ae4 commit c23ae84
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 37 deletions.
3 changes: 3 additions & 0 deletions packages/SwingSet/src/kernel/kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,8 @@ export default function buildKernel(
kdebug(`vat ${vatID} terminated before startVat delivered`);
return NO_DELIVERY_CRANK_RESULTS;
}
const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
vatKeeper.setVatParameters(vatParameters);
const { meterID } = vatInfo;
/** @type { KernelDeliveryStartVat } */
const kd = harden(['startVat', vatParameters]);
Expand Down Expand Up @@ -1052,6 +1054,7 @@ export default function buildKernel(
});
const vatOptions = harden({ ...origOptions, workerOptions });
vatKeeper.setSourceAndOptions(source, vatOptions);
vatKeeper.setVatParameters(vatParameters);
// TODO: decref the bundleID once setSourceAndOptions increfs it

// pause, take a deep breath, appreciate this moment of silence
Expand Down
2 changes: 2 additions & 0 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ export const CURRENT_SCHEMA_VERSION = 3;
// old (v0): v$NN.reapCountdown = $NN or 'never'
// v$NN.reapDirt = JSON({ deliveries, gcKrefs, computrons }) // missing keys treated as zero
// (leave room for v$NN.snapshotDirt and options.snapshotDirtThreshold for #6786)
// v$NN.vatParameters = JSON(capdata) // missing for vats created/upgraded before #8947
//
// exclude from consensus
// local.*

Expand Down
32 changes: 32 additions & 0 deletions packages/SwingSet/src/kernel/state/vatKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { isObject } from '@endo/marshal';
import { parseKernelSlot } from '../parseKernelSlots.js';
import { makeVatSlot, parseVatSlot } from '../../lib/parseVatSlots.js';
import { insistVatID } from '../../lib/id.js';
import { insistCapData } from '../../lib/capdata.js';
import { kdebug } from '../../lib/kdebug.js';
import {
parseReachableAndVatSlot,
Expand Down Expand Up @@ -173,6 +174,35 @@ export function makeVatKeeper(
return harden(options);
}

/**
* @param {SwingSetCapData} newVPCD
*/
function setVatParameters(newVPCD) {
insistCapData(newVPCD);
const key = `${vatID}.vatParameters`;
// increment-before-decrement to minimize spurious rc=0 checks
for (const kref of newVPCD.slots) {
incrementRefCount(kref, `${vatID}.vatParameters`);
}
const old = kvStore.get(key) || '{"slots":[]}';
for (const kref of JSON.parse(old).slots) {
decrementRefCount(kref, `${vatID}.vatParameters`);
}
kvStore.set(key, JSON.stringify(newVPCD));
}

/**
* @returns {SwingSetCapData | undefined} vpcd
*/
function getVatParameters() {
const key = `${vatID}.vatParameters`;
const old = kvStore.get(key);
if (old) {
return JSON.parse(old);
}
return undefined;
}

// This is named "addDirt" because it should increment all dirt
// counters (both for reap/BOYD and for heap snapshotting). We don't
// have `heapSnapshotDirt` yet, but when we do, it should get
Expand Down Expand Up @@ -768,6 +798,8 @@ export function makeVatKeeper(
setSourceAndOptions,
getSourceAndOptions,
getOptions,
setVatParameters,
getVatParameters,
addDirt,
getReapDirt,
clearReapDirt,
Expand Down
38 changes: 38 additions & 0 deletions packages/SwingSet/test/state.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,44 @@ test('vatKeeper.getOptions', async t => {
t.is(name, 'fred');
});

test('vatKeeper.setVatParameters', async t => {
const store = buildKeeperStorageInMemory();
const k = makeKernelKeeper(store, 'uninitialized');
k.createStartingKernelState({ defaultManagerType: 'local' });
k.setInitialized();
const v1 = k.allocateVatIDForNameIfNeeded('name1');
const bundleID =
'b1-00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000';
const source = { bundleID };
const workerOptions = { type: 'local' };
const options = { workerOptions, name: 'fred', reapDirtThreshold: {} };
k.createVatState(v1, source, options);

const vk = k.provideVatKeeper(v1);
const ko1 = kslot('ko1');
const ko2 = kslot('ko2');
const vp1 = kser({ foo: 1, bar: ko1, baz: ko1 });
vk.setVatParameters(vp1);
t.deepEqual(JSON.parse(store.kvStore.get('v1.vatParameters')), vp1);
t.is(store.kvStore.get('ko1.refCount'), '1,1');
t.deepEqual(vk.getVatParameters(), vp1);

const vp2 = kser({ foo: 1, bar: ko1, baz: ko2 });
vk.setVatParameters(vp2);
t.deepEqual(JSON.parse(store.kvStore.get('v1.vatParameters')), vp2);
t.is(store.kvStore.get('ko1.refCount'), '1,1');
t.is(store.kvStore.get('ko2.refCount'), '1,1');
t.deepEqual(vk.getVatParameters(), vp2);

const vp3 = kser({ foo: 1, baz: ko2 });
vk.setVatParameters(vp3);
t.deepEqual(JSON.parse(store.kvStore.get('v1.vatParameters')), vp3);
// this test doesn't do processRefcounts, which would remove the 0,0
t.is(store.kvStore.get('ko1.refCount'), '0,0');
t.is(store.kvStore.get('ko2.refCount'), '1,1');
t.deepEqual(vk.getVatParameters(), vp3);
});

test('XS vatKeeper defaultManagerType', async t => {
const store = buildKeeperStorageInMemory();
const k = makeKernelKeeper(store, 'uninitialized');
Expand Down
23 changes: 23 additions & 0 deletions packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export const buildRootObject = () => {
vatAdmin = await E(vats.vatAdmin).createVatAdminService(devices.vatAdmin);
},

nop: () => 0,

getMarker: () => marker,

getImportSensors: () => importSensors,
Expand Down Expand Up @@ -282,5 +284,26 @@ export const buildRootObject = () => {

return [paramA, paramB];
},

buildV1WithVatParameters: async () => {
const bcap1 = await E(vatAdmin).getNamedBundleCap('ulrik1');
const vp1 = { number: 1, marker };
const options1 = { vatParameters: vp1 };
const res = await E(vatAdmin).createVat(bcap1, options1);
ulrikAdmin = res.adminNode;
ulrikRoot = res.root;
const param1 = await E(ulrikRoot).getParameters();
return param1;
},

upgradeV2WithVatParameters: async () => {
const bcap2 = await E(vatAdmin).getNamedBundleCap('ulrik2');
const vp2 = { number: 2, marker };
const options2 = { vatParameters: vp2 };
await E(ulrikAdmin).upgrade(bcap2, options2);
const param2 = await E(ulrikRoot).getParameters();

return param2;
},
});
};
66 changes: 66 additions & 0 deletions packages/SwingSet/test/upgrade/upgrade.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -953,3 +953,69 @@ test('failed vatAdmin upgrade - bad replacement code', async t => {
// Just a taste to verify that the create went right; other tests check the rest
t.deepEqual(v1result.data, ['some', 'data']);
});

test('vat upgrade retains vatParameters', async t => {
const config = makeConfigFromPaths('bootstrap-scripted-upgrade.js', {
defaultManagerType: 'xs-worker',
defaultReapInterval: 'never',
bundlePaths: {
ulrik1: 'vat-ulrik-1.js',
ulrik2: 'vat-ulrik-2.js',
},
});
const kft = await initKernelForTest(t, t.context.data, config);
const { controller: c, kvStore } = kft;

// create initial version
const kpid1 = c.queueToVatRoot('bootstrap', 'buildV1WithVatParameters', []);
await c.run();
// incref=false to keep it from adding a refcount to the marker
const params1 = kunser(c.kpResolution(kpid1, { incref: false }));
// Free kpid1, else it will hold an extra refcount on the
// marker. The c.kpResolution() decremented the kpid1 refcount to 0,
// but we need to provoke a call to processRefcounts(), which only
// happens at end-of-crank
c.queueToVatRoot('bootstrap', 'nop', []);
// BOYD to get vat-admin to drop it's copy of the marker
c.reapAllVats();
await c.run();

t.is(params1.number, 1);
const marker = params1.marker;
t.deepEqual(params1, { number: 1, marker });
const kref = marker.getKref();

// confirm the vatParameters were recorded in kvStore
const vatID = JSON.parse(kvStore.get('vat.dynamicIDs'))[0];
const vp1cd = kvStore.get(`${vatID}.vatParameters`);
const vp1 = kunser(JSON.parse(vp1cd));
t.is(vp1.number, 1);
t.is(vp1.marker.getKref(), kref);

// Ideally, the refcount should be 2: one for the importing vat's
// c-list, and a second for the retained vNN.vatParameters. But it
// will also get a refcount from device-vat-admin, as a conduit for
// the upgrade() call, because devices don't do GC.

t.is(kvStore.get(`${kref}.refCount`), '3,3');

// upgrade
const kpid2 = c.queueToVatRoot('bootstrap', 'upgradeV2WithVatParameters', []);
await c.run();
const params2 = kunser(c.kpResolution(kpid2, { incref: false }));
c.queueToVatRoot('bootstrap', 'nop', []);
c.reapAllVats();
await c.run();

t.is(params2.number, 2);
t.is(params2.marker.getKref(), kref);
// kvStore should now hold the new vatParameters
const vp2cd = kvStore.get(`${vatID}.vatParameters`);
const vp2 = kunser(JSON.parse(vp2cd));
t.is(vp2.number, 2);
t.is(vp2.marker.getKref(), kref);

// same refcount: the old retained vatParameters should be swapped
// out
t.is(kvStore.get(`${kref}.refCount`), '3,3');
});
51 changes: 31 additions & 20 deletions packages/SwingSet/test/vat-admin/create-vat.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,20 @@ test('createVat holds refcount', async t => {
const { c, idRC, kernelStorage } = await doTestSetup(t, false, printSlog);
const { kvStore } = kernelStorage;

// we typicaly get: v1-bootstrap, v2-vatAdmin, v6-export-held
// for (const name of JSON.parse(kvStore.get('vat.names'))) {
// console.log(`${kvStore.get(`vat.name.${name}`)}: ${name}`);
// }
// and d7-vatAdmin

// The bootstrap vat starts by fetching 'held' from vat-export-held, during
// doTestSetup(), and retains it throughout the entire test. When we send
// it refcount(), it will send VatAdminService.getBundleCap(), wait for the
// response, then send VatAdminService.createVat(held). VAS will tell
// device-vat-admin to push a create-vat event (including 'held') on the
// run-queue. Some time later, the create-vat event reaches the front, and
// the new vat is created, receiving 'held' in vatParametesr.
// the new vat is created, receiving 'held' in vatParameters. The kernel
// retains a copy of those vatParameters for any future unilateral upgrade.

// We want to check the refcounts during this sequence, to confirm that the
// create-vat event holds a reference. Otherwise, 'held' might get GCed
Expand All @@ -292,7 +299,7 @@ test('createVat holds refcount', async t => {
// nothing ever decrements it: devices hold eternal refcounts on their
// c-list entries, and nothing ever removes a device c-list entry. But some
// day when we fix that, we'll rely upon the create-vat event's refcount to
// keep these things alive.
// keep these things alive.)

// We happen to know that 'held' will be allocated ko27, but we use
// `getHeld()` to obtain the real value in case e.g. a new built-in vat is
Expand Down Expand Up @@ -365,9 +372,11 @@ test('createVat holds refcount', async t => {
expectedRefcount -= 1; // kpid1 deleted, drops ref to 'held', now 2,2
// it also does syscall.send(createVat), which holds a new reference
expectedRefcount += 1; // arg to 'createVat'
// now we should see 3,3: v1-bootstrap, the kpResolution pin, and the
// send(createVat) arguments. Two of these are c-lists.
// now we should see 3,3: v1-bootstrap, the kpResolution pin, and
// the send(createVat) arguments. For c-lists, it is present in v1.c
// (import) and v6.c (export)
const r1 = findRefs(kvStore, held);
// console.log(`r1:`, JSON.stringify(r1));
t.deepEqual(r1.refcount, [expectedRefcount, expectedRefcount]);
t.is(r1.refs.length, expectedCLists);
// console.log(`---`);
Expand All @@ -384,13 +393,13 @@ test('createVat holds refcount', async t => {
await stepUntil(seeCreateVat);
// console.log(`---`);

// We should see 5,5: v2-bootstrap, the kpResolution pin, vat-vat-admin,
// device-vat-admin, and the create-vat run-queue event. Three of these are
// c-lists.
expectedRefcount += 1; // vat-vat-admin c-list
expectedCLists += 1; // vat-vat-admin c-list
expectedRefcount += 1; // device-vat-admin c-list
expectedCLists += 1; // device-vat-admin c-list
// We should see 5,5: v1-bootstrap, the kpResolution pin,
// v2-vatAdmin, d7-vatAdmin, and the create-vat run-queue
// event. c-lists: v1/v2/d7 imports, v6 export
expectedRefcount += 1; // v2-vatAdmin c-list
expectedCLists += 1; // v2-vatAdmin c-list
expectedRefcount += 1; // d7-vatAdmin c-list
expectedCLists += 1; // d7-vatAdmin c-list

const r2 = findRefs(kvStore, held);
// console.log(`r2:`, JSON.stringify(r2));
Expand All @@ -400,8 +409,8 @@ test('createVat holds refcount', async t => {
// Allow the vat-admin bringOutYourDead to be delivered, which
// allows it to drop its reference to 'held'.

expectedRefcount -= 1; // vat-vat-admin retires
expectedCLists -= 1; // vat-vat-admin retires
expectedRefcount -= 1; // v2-vatAdmin retires
expectedCLists -= 1; // v2-vatAdmin retires

// In addition, device-vat-admin does not yet participate in GC, and holds
// its references forever. So this -=1 remains commented out until we
Expand All @@ -415,25 +424,27 @@ test('createVat holds refcount', async t => {
t.deepEqual(c.dump().reapQueue, []);
// console.log(`---`);

// At this point we expected to see 5,5: v2-bootstrap, kpResolution pin,
// vat-vat-admin (because of the non-dropping bug), device-vat-admin
// (because of unimplemented GC), and the create-vat run-queue event. Two
// are c-lists.
// At this point we expected to see 4,4: v1-bootstrap, kpResolution
// pin, d7-vatAdmin (because of unimplemented GC), and the
// create-vat run-queue event. c-lists: v1/d7 imports, v6 export

const r3 = findRefs(kvStore, held);
// console.log(`r3:`, JSON.stringify(r3));
t.deepEqual(r3.refcount, [expectedRefcount, expectedRefcount]);
t.is(r3.refs.length, expectedCLists);

// Allow create-vat to be processed, removing the create-vat reference and
// adding a reference from the new vat's c-list
// Allow create-vat to be processed, removing the create-vat
// reference and adding a reference from the new vat's c-list, plus
// a second reference from the retained vatParameters
await c.step();
expectedRefcount -= 1; // remove send(createVat) argument
expectedRefcount += 1; // new-vat c-list
expectedRefcount += 1; // retained vatParameters
expectedCLists += 1; // new-vat c-list
// console.log(`---`);

// v2-bootstrap, kpResolution pin, device-vat-admin, new-vat
// v1-bootstrap, kpResolution pin, d7-vatAdmin, new-vat, retained
// vatParameters
const r4 = findRefs(kvStore, held);
// console.log(`r4:`, JSON.stringify(r4));
t.deepEqual(r4.refcount, [expectedRefcount, expectedRefcount]);
Expand Down
Loading

0 comments on commit c23ae84

Please sign in to comment.