From d2b00bb24c48356d2fa6d51492cb1f0ebac04d98 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 6 Apr 2023 22:43:48 -0700 Subject: [PATCH] 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 | 17 ++- .../test/test-collection-schema-refcount.js | 106 ++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) 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 7f1dfeff7010..2612b149db7a 100644 --- a/packages/swingset-liveslots/src/collectionManager.js +++ b/packages/swingset-liveslots/src/collectionManager.js @@ -64,7 +64,7 @@ function prefixc(collectionID, dbEntryKey) { /* * Build a cache that holds the schema for each collection. * - * The cache maps collectionID to { keyShape, valueShape, label, + * The cache maps colectionID to { keyShape, valueShape, label, * schemataCapData }. These are initialized when the collection is * first constructed, and never modified afterwards. The values live * in the vatstore, inside two keys, one for the [keyShape, @@ -687,7 +687,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, '|'), @@ -730,6 +734,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 000000000000..03db7d350a47 --- /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');