From b3d07a622d79131afa3f3c953f375906ca0c719a Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 6 Apr 2023 22:43:48 -0700 Subject: [PATCH 1/2] collectionManager: refcount remotables in keyShape/valueShape The collection schema (keyShape/valueShape) can refer to specific Remotable instances (e.g. Presence, or a durable object), rather than defining a constraint to be a whole category of objects. The collection schema metadata is thus a form of write-once state storage, like a tiny virtual object. Previously, defineKind failed to increment the refcount on the objects' vrefs, so when the collection falls out of memory, the object might be deleted, and then we would be unable to revive the constraint when loading the schema back in later. This changes the code to increment the refcounts of the vrefs in keyShape and valueShape when the collection is created, and decrement them when the collection is deleted. It handles ephemeral Remotables, as well as virtual/durable object Representatives. For durable collections, it also enforces the same `isDurable` check on the schema as it would on keys and values of the collection itself, so you cannot use a `Far()` or a merely-virtual object in the valueShape of a durable collection. closes #7321 --- .../src/collectionManager.js | 15 ++- .../test/test-collection-schema-refcount.js | 106 ++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 packages/swingset-liveslots/test/test-collection-schema-refcount.js diff --git a/packages/swingset-liveslots/src/collectionManager.js b/packages/swingset-liveslots/src/collectionManager.js index fb63f280ff4..4bbc74b21f0 100644 --- a/packages/swingset-liveslots/src/collectionManager.js +++ b/packages/swingset-liveslots/src/collectionManager.js @@ -685,7 +685,11 @@ export function makeCollectionManager( const collectionID = `${subid}`; const collection = summonCollectionInternal(false, collectionID, kindName); - const doMoreGC = collection.clearInternal(true); + let doMoreGC = collection.clearInternal(true); + const { schemataCapData } = schemaCache.get(collectionID); + doMoreGC = + schemataCapData.slots.map(vrm.removeReachableVref).some(b => b) || + doMoreGC; for (const dbKey of enumerateKeysWithPrefix( syscall, prefixc(collectionID, '|'), @@ -728,6 +732,15 @@ export function makeCollectionManager( schemata.valueShape = valueShape; } const schemataCapData = serialize(harden(schemata)); + if (isDurable) { + schemataCapData.slots.forEach((vref, slotIndex) => { + if (!vrm.isDurable(vref)) { + throwNotDurable(vref, slotIndex, schemataCapData); + } + }); + } + schemataCapData.slots.forEach(vrm.addReachableVref); + schemaCache.set( collectionID, harden({ keyShape, valueShape, label, schemataCapData }), diff --git a/packages/swingset-liveslots/test/test-collection-schema-refcount.js b/packages/swingset-liveslots/test/test-collection-schema-refcount.js new file mode 100644 index 00000000000..03db7d350a4 --- /dev/null +++ b/packages/swingset-liveslots/test/test-collection-schema-refcount.js @@ -0,0 +1,106 @@ +import test from 'ava'; +import '@endo/init/debug.js'; + +import { Far } from '@endo/marshal'; +import { makeLiveSlots } from '../src/liveslots.js'; +import { parseVatSlot } from '../src/parseVatSlots.js'; +import { kser } from './kmarshal.js'; +import { buildSyscall } from './liveslots-helpers.js'; +import { makeStartVat, makeBringOutYourDead } from './util.js'; +import { makeMockGC } from './mock-gc.js'; + +// virtual/durable collections serialize their keyShape/valueShape, +// any Remotables therein must be compatible, and we should have +// refcounts on them + +const shapetest = test.macro(async (t, collectionType, remotableType) => { + const { syscall, fakestore } = buildSyscall(); + const gcTools = makeMockGC(); + let remotable; + let map; + + function buildRootObject(vatPowers) { + const { VatData } = vatPowers; + const initData = () => ({}); + const handle = VatData.makeKindHandle('thing'); + const makeVirtualThing = VatData.defineKind('thing', initData, {}); + const makeDurableThing = VatData.defineDurableKind(handle, initData, {}); + switch (remotableType) { + case 'ephemeral': + remotable = Far('thing', {}); + break; + case 'virtual': + remotable = makeVirtualThing(); + break; + case 'durable': + remotable = makeDurableThing(); + break; + default: + throw Error(`unhandled ${remotableType}`); + } + t.truthy(remotable); + const valueShape = remotable; + const durable = collectionType === 'durable'; + map = VatData.makeScalarBigMapStore('map', { valueShape, durable }); + return Far('root', {}); + } + + const makeNS = () => ({ buildRootObject }); + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, makeNS); + const { dispatch, testHooks } = ls; + const startVat = makeStartVat(kser()); + if (collectionType === 'durable' && remotableType !== 'durable') { + await t.throwsAsync(() => dispatch(startVat), { + message: /value is not durable/, + }); + // TODO: unhandled rejection interferes with the test + return; + } + await dispatch(startVat); + + // the object we used in the valueShape schema .. + const vref = testHooks.valToSlot.get(remotable); + t.truthy(vref); + + // .. should appear in the serialized schema + const mapVref = testHooks.valToSlot.get(map); + const collectionID = parseVatSlot(mapVref).subid; + const schemaKey = `vc.${collectionID}.|schemata`; + const schemataData = JSON.parse(fakestore.get(schemaKey)); + t.deepEqual(schemataData.slots, [vref]); + + // and it should hold a refcount + t.is(testHooks.getReachableRefCount(vref), 1); + + // now pretend the collection is deleted, and do a BOYD + gcTools.kill(map); + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + // the refcount should be gone + t.falsy(testHooks.getReachableRefCount(vref)); + // so should the schema (and all other keys) + t.falsy(fakestore.has(schemaKey)); +}); + +test( + 'virtual collection shape holds ephmeral', + shapetest, + 'virtual', + 'ephemeral', +); +test('virtual collection shape holds virtual', shapetest, 'virtual', 'virtual'); +test('virtual collection shape holds durable', shapetest, 'virtual', 'durable'); + +test.skip( + 'durable collection shape holds ephmeral', + shapetest, + 'durable', + 'ephemeral', +); +test.skip( + 'durable collection shape holds virtual', + shapetest, + 'durable', + 'virtual', +); +test('durable collection shape holds durable', shapetest, 'durable', 'durable'); From 5af1b316f2c61af30d7e6fed3d2be78f56e11248 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 5 Apr 2023 09:29:37 -0700 Subject: [PATCH 2/2] liveslots: add new test of durable collections across upgrades Check that the refcounts are maintained correctly --- .../test/test-collection-upgrade.js | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 packages/swingset-liveslots/test/test-collection-upgrade.js diff --git a/packages/swingset-liveslots/test/test-collection-upgrade.js b/packages/swingset-liveslots/test/test-collection-upgrade.js new file mode 100644 index 00000000000..25c5d52a15e --- /dev/null +++ b/packages/swingset-liveslots/test/test-collection-upgrade.js @@ -0,0 +1,135 @@ +/* eslint-disable no-await-in-loop, @jessie.js/no-nested-await, no-shadow */ +import test from 'ava'; +import '@endo/init/debug.js'; + +import { Far } from '@endo/marshal'; +import { M } from '@agoric/store'; +import { makeLiveSlots } from '../src/liveslots.js'; +import { parseVatSlot } from '../src/parseVatSlots.js'; +import { kser } from './kmarshal.js'; +import { buildSyscall } from './liveslots-helpers.js'; +import { makeStartVat } from './util.js'; +import { makeMockGC } from './mock-gc.js'; + +test('durable collections survive upgrade', async t => { + let map1; + let set1; + let thing1; + let thing2; + let thing3; + + function build1(vatPowers, _vatParameters, baggage) { + const { VatData } = vatPowers; + const handle = VatData.makeKindHandle('thing'); + baggage.init('handle', handle); + const initData = name => ({ name }); + const how = { name: ({ state }) => state.name }; + const makeDurableThing = VatData.defineDurableKind(handle, initData, how); + thing1 = makeDurableThing('thing1'); + thing2 = makeDurableThing('thing2'); + thing3 = makeDurableThing('thing3'); + const valueShape = [thing1, M.any()]; + const durable = true; + map1 = VatData.makeScalarBigMapStore('map', { valueShape, durable }); + baggage.init('map', map1); + map1.init('string', harden([thing1, 'string'])); + map1.init('number', harden([thing1, 123])); + map1.init('thing2', harden([thing1, thing2])); + // thing3 is only used as the valueShape + set1 = VatData.makeScalarBigSetStore('set', { + valueShape: thing3, + durable, + }); + baggage.init('set', set1); + return Far('root', {}); + } + const make1 = () => ({ buildRootObject: build1 }); + + const kvStore = new Map(); + const { syscall: sc1 } = buildSyscall({ kvStore }); + const gcTools1 = makeMockGC(); + const ls1 = makeLiveSlots(sc1, 'vatA', {}, {}, gcTools1, undefined, make1); + + const startVat = makeStartVat(kser()); + await ls1.dispatch(startVat); + + const mapVref = ls1.testHooks.valToSlot.get(map1); + const setVref = ls1.testHooks.valToSlot.get(set1); + const thing1Vref = ls1.testHooks.valToSlot.get(thing1); + const thing2Vref = ls1.testHooks.valToSlot.get(thing2); + const thing3Vref = ls1.testHooks.valToSlot.get(thing3); + + // console.log(`-- map=${mapVref}, thing1=${thing1Vref}, thing2=${thing2Vref}`); + + // map and set should have refcount 1, from baggage + t.is(ls1.testHooks.getReachableRefCount(mapVref), 1); + t.is(ls1.testHooks.getReachableRefCount(setVref), 1); + // thing1 should have a refcount of 4: one for valueShape, plus one + // for each entry + t.is(ls1.testHooks.getReachableRefCount(thing1Vref), 4); + // thing2 should have 1, only the 'thing2' entry + t.is(ls1.testHooks.getReachableRefCount(thing2Vref), 1); + // thing3 should have 1, just the Set's valueShape + t.is(ls1.testHooks.getReachableRefCount(thing3Vref), 1); + + const collectionID = parseVatSlot(mapVref).subid; + const schemaKey = `vc.${collectionID}.|schemata`; + const schemataData = JSON.parse(kvStore.get(schemaKey)); + t.deepEqual(schemataData.slots, [thing1Vref]); + + // const compareEntriesByKey = ([ka], [kb]) => (ka < kb ? -1 : 1); + // t.log(Object.fromEntries([...kvStore.entries()].sort(compareEntriesByKey))); + + // Simulate upgrade by starting from the non-empty kvStore. + const clonedStore = new Map(kvStore); + + let map2; + let set2; + let newThing1; + let newThing2; + + function build2(vatPowers, _vatParameters, baggage) { + const { VatData } = vatPowers; + const handle = baggage.get('handle'); + const initData = name => ({ name }); + const how = { name: ({ state }) => state.name }; + VatData.defineDurableKind(handle, initData, how); + map2 = baggage.get('map'); + let s; + [newThing1, s] = map2.get('string'); + t.is(newThing1.name(), 'thing1'); + t.is(s, 'string'); + t.deepEqual(map2.get('string'), [newThing1, 'string']); + t.deepEqual(map2.get('number'), [newThing1, 123]); + newThing2 = map2.get('thing2')[1]; + t.is(newThing2.name(), 'thing2'); + set2 = baggage.get('set'); + return Far('root', {}); + } + const make2 = () => ({ buildRootObject: build2 }); + + const { syscall: sc2 } = buildSyscall({ kvStore: clonedStore }); + const gcTools2 = makeMockGC(); + const ls2 = makeLiveSlots(sc2, 'vatA', {}, {}, gcTools2, undefined, make2); + + // restarting should work. this exercises valueShape being durable + // and being retrievable + await ls2.dispatch(startVat); + + // [...ls2.testHooks.slotToVal.entries()].map(([slot,wr]) => console.log(slot, wr.deref())); + + t.is(ls2.testHooks.slotToVal.get(thing1Vref).deref(), newThing1); + + // the vrefs should all still be the same + t.is(ls2.testHooks.valToSlot.get(map2), mapVref); + t.is(ls2.testHooks.valToSlot.get(set2), setVref); + t.is(ls2.testHooks.valToSlot.get(newThing1), thing1Vref); + t.is(ls2.testHooks.valToSlot.get(newThing2), thing2Vref); + + // refcounts should be the same + t.is(ls2.testHooks.getReachableRefCount(mapVref), 1); + t.is(ls1.testHooks.getReachableRefCount(setVref), 1); + t.is(ls2.testHooks.getReachableRefCount(thing1Vref), 4); + t.is(ls2.testHooks.getReachableRefCount(thing2Vref), 1); + t.is(ls2.testHooks.getReachableRefCount(thing3Vref), 1); +});