From 0f29938420483908493af5506887ab80424a9586 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Sat, 12 Oct 2024 18:45:29 -0700 Subject: [PATCH] fix(swingset): retain vatParameters for vat creation and upgrade 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(vatAdminSvc).upgradeVat()`. When received via `createVat()` or `upgradeVat()`, 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 `upgradeVat()` on all vats without userspace asking for it. When this is implemented, the new incarnations will receive the same vatParameters as their predecessors. fixes #8947 --- packages/SwingSet/src/kernel/kernel.js | 3 ++ .../SwingSet/src/kernel/state/kernelKeeper.js | 2 + .../SwingSet/src/kernel/state/vatKeeper.js | 32 ++++++++++++ packages/SwingSet/test/state.test.js | 38 ++++++++++++++ .../upgrade/bootstrap-scripted-upgrade.js | 21 ++++++++ .../SwingSet/test/upgrade/upgrade.test.js | 37 ++++++++++++++ .../test/vat-admin/create-vat.test.js | 51 +++++++++++-------- 7 files changed, 164 insertions(+), 20 deletions(-) diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index 9e91d9517843..680065b834a4 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/packages/SwingSet/src/kernel/kernel.js @@ -743,6 +743,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]); @@ -1022,6 +1024,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 diff --git a/packages/SwingSet/src/kernel/state/kernelKeeper.js b/packages/SwingSet/src/kernel/state/kernelKeeper.js index aabc853d16e1..e120833de69e 100644 --- a/packages/SwingSet/src/kernel/state/kernelKeeper.js +++ b/packages/SwingSet/src/kernel/state/kernelKeeper.js @@ -117,6 +117,8 @@ export const CURRENT_SCHEMA_VERSION = 2; // 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.* diff --git a/packages/SwingSet/src/kernel/state/vatKeeper.js b/packages/SwingSet/src/kernel/state/vatKeeper.js index 7322f34ee375..bd15830d7fb5 100644 --- a/packages/SwingSet/src/kernel/state/vatKeeper.js +++ b/packages/SwingSet/src/kernel/state/vatKeeper.js @@ -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, @@ -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}.vp`); + } + const old = kvStore.get(key) || '{"slots":[]}'; + for (const kref of JSON.parse(old).slots) { + decrementRefCount(kref, `${vatID}.vp`); + } + 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 @@ -768,6 +798,8 @@ export function makeVatKeeper( setSourceAndOptions, getSourceAndOptions, getOptions, + setVatParameters, + getVatParameters, addDirt, getReapDirt, clearReapDirt, diff --git a/packages/SwingSet/test/state.test.js b/packages/SwingSet/test/state.test.js index c60a01aa79c6..9661622d03f2 100644 --- a/packages/SwingSet/test/state.test.js +++ b/packages/SwingSet/test/state.test.js @@ -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'); diff --git a/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js b/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js index c93cfa6b5684..66b2f0447750 100644 --- a/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js +++ b/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js @@ -265,5 +265,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; + }, }); }; diff --git a/packages/SwingSet/test/upgrade/upgrade.test.js b/packages/SwingSet/test/upgrade/upgrade.test.js index aca1efbbd892..093f42655485 100644 --- a/packages/SwingSet/test/upgrade/upgrade.test.js +++ b/packages/SwingSet/test/upgrade/upgrade.test.js @@ -930,3 +930,40 @@ 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 { messageToVat, kvStore } = kft; + + // create initial version + const params1 = await messageToVat('bootstrap', 'buildV1WithVatParameters'); + t.is(params1.number, 1); + const marker = params1.marker; + t.deepEqual(params1, { number: 1, marker }); + const markerKref = 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(), markerKref); + + // upgrade + const params2 = await messageToVat('bootstrap', 'upgradeV2WithVatParameters'); + t.is(params2.number, 2); + t.is(params2.marker.getKref(), markerKref); + // 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(), markerKref); +}); diff --git a/packages/SwingSet/test/vat-admin/create-vat.test.js b/packages/SwingSet/test/vat-admin/create-vat.test.js index 40b3325cc582..0fe7547bd2d1 100644 --- a/packages/SwingSet/test/vat-admin/create-vat.test.js +++ b/packages/SwingSet/test/vat-admin/create-vat.test.js @@ -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 @@ -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 @@ -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(`---`); @@ -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)); @@ -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 @@ -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]);