From cd6ca3a98d50081d9b585404de43087ca203c8e4 Mon Sep 17 00:00:00 2001 From: Chip Morningstar Date: Tue, 15 Mar 2022 23:06:02 -0700 Subject: [PATCH] fix: prevent marker facets from becoming orphaned --- .../src/liveslots/virtualObjectManager.js | 64 ++++++++-------- .../virtualObjects/test-representatives.js | 30 ++++++++ .../virtualObjects/test-virtualObjectCache.js | 54 +++++++------- .../virtualObjects/test-virtualObjectGC.js | 73 +++++++++++++------ .../test/virtualObjects/vat-orphan-bob.js | 32 ++++++++ .../virtualObjects/vat-orphan-bootstrap.js | 11 +++ 6 files changed, 185 insertions(+), 79 deletions(-) create mode 100644 packages/SwingSet/test/virtualObjects/vat-orphan-bob.js create mode 100644 packages/SwingSet/test/virtualObjects/vat-orphan-bootstrap.js diff --git a/packages/SwingSet/src/liveslots/virtualObjectManager.js b/packages/SwingSet/src/liveslots/virtualObjectManager.js index 166dd5c3b9ea..8ff05cb210a4 100644 --- a/packages/SwingSet/src/liveslots/virtualObjectManager.js +++ b/packages/SwingSet/src/liveslots/virtualObjectManager.js @@ -12,7 +12,7 @@ import { Far } from '@endo/marshal'; * starting to throw them away. * @param {(baseRef: string) => Object} fetch Function to retrieve an * object's raw state from the store by its baseRef - * @param {(baseRef: string, rawData: Object) => void} store Function to + * @param {(baseRef: string, rawState: Object) => void} store Function to * store raw object state by its baseRef * * @returns {Object} An LRU cache of (up to) the given size @@ -32,11 +32,11 @@ export function makeCache(size, fetch, store) { // kdebug(`### vo LRU evict ${lruTail.baseRef} (dirty=${lruTail.dirty})`); liveTable.delete(lruTail.baseRef); if (lruTail.dirty) { - store(lruTail.baseRef, lruTail.rawData); + store(lruTail.baseRef, lruTail.rawState); lruTail.dirty = false; dirtyCount -= 1; } - lruTail.rawData = null; + lruTail.rawState = null; if (lruTail.prev) { lruTail.prev.next = undefined; } else { @@ -59,7 +59,7 @@ export function makeCache(size, fetch, store) { let entry = lruTail; while (entry) { if (entry.dirty) { - store(entry.baseRef, entry.rawData); + store(entry.baseRef, entry.rawState); entry.dirty = false; } entry = entry.prev; @@ -110,11 +110,11 @@ export function makeCache(size, fetch, store) { if (innerObj) { cache.refresh(innerObj); } else { - innerObj = { baseRef, rawData: null, repCount: 0 }; + innerObj = { baseRef, rawState: null, repCount: 0 }; cache.remember(innerObj); } - if (load && !innerObj.rawData) { - innerObj.rawData = fetch(baseRef); + if (load && !innerObj.rawState) { + innerObj.rawState = fetch(baseRef); } return innerObj; }, @@ -185,6 +185,7 @@ export function makeVirtualObjectManager( // a state object from being able to observe the comings and goings of // representatives. const stateToRepresentative = new WeakMap(); + const facetToCohort = new WeakMap(); /** * Fetch an object's state from secondary storage. @@ -194,9 +195,9 @@ export function makeVirtualObjectManager( * @returns {*} an object representing the object's stored state. */ function fetch(baseRef) { - const raw = syscall.vatstoreGet(`vom.${baseRef}`); - if (raw) { - return JSON.parse(raw); + const rawState = syscall.vatstoreGet(`vom.${baseRef}`); + if (rawState) { + return JSON.parse(rawState); } else { return undefined; } @@ -207,10 +208,10 @@ export function makeVirtualObjectManager( * * @param {string} baseRef The baseRef of the object whose state is being * stored. - * @param {*} rawData A data object representing the state to be written. + * @param {*} rawState A data object representing the state to be written. */ - function store(baseRef, rawData) { - syscall.vatstoreSet(`vom.${baseRef}`, JSON.stringify(rawData)); + function store(baseRef, rawState) { + syscall.vatstoreSet(`vom.${baseRef}`, JSON.stringify(rawState)); } /* eslint max-classes-per-file: ["error", 2] */ @@ -506,23 +507,23 @@ export function makeVirtualObjectManager( } function ensureState() { - if (innerSelf.rawData) { + if (innerSelf.rawState) { cache.refresh(innerSelf); } else { innerSelf = cache.lookup(innerSelf.baseRef, true); } } - const wrappedData = {}; + const wrappedState = {}; for (const prop of propertyNames) { - Object.defineProperty(wrappedData, prop, { + Object.defineProperty(wrappedState, prop, { get: () => { ensureState(); - return unserialize(innerSelf.rawData[prop]); + return unserialize(innerSelf.rawState[prop]); }, set: value => { ensureState(); - const before = innerSelf.rawData[prop]; + const before = innerSelf.rawState[prop]; const after = serialize(value); if (durable) { after.slots.map(vref => @@ -533,17 +534,17 @@ export function makeVirtualObjectManager( ); } vrm.updateReferenceCounts(before.slots, after.slots); - innerSelf.rawData[prop] = after; + innerSelf.rawState[prop] = after; cache.markDirty(innerSelf); }, }); } - harden(wrappedData); + harden(wrappedState); if (initializing) { cache.remember(innerSelf); } - const self = actualize(wrappedData); + const self = actualize(wrappedState); let toHold; let toExpose; const facetiousness = assessFacetiousness(self); @@ -567,6 +568,7 @@ export function makeVirtualObjectManager( const facet = Far(`${tag} ${facetName}`, self[facetName]); toExpose[facetName] = facet; toHold.push(facet); + facetToCohort.set(facet, toHold); } harden(toExpose); break; @@ -578,9 +580,9 @@ export function makeVirtualObjectManager( } if (!proForma) { innerSelf.representative = toHold; - stateToRepresentative.set(wrappedData, toHold); + stateToRepresentative.set(wrappedState, toHold); } - return [toHold, toExpose, wrappedData]; + return [toHold, toExpose, wrappedState]; } function reanimate(baseRef, proForma) { @@ -596,9 +598,9 @@ export function makeVirtualObjectManager( function deleteStoredVO(baseRef) { let doMoreGC = false; - const state = fetch(baseRef); - if (state) { - for (const propValue of Object.values(state)) { + const rawState = fetch(baseRef); + if (rawState) { + for (const propValue of Object.values(rawState)) { propValue.slots.map( vref => (doMoreGC = doMoreGC || vrm.removeReachableVref(vref)), ); @@ -616,7 +618,7 @@ export function makeVirtualObjectManager( // kdebug(`vo make ${baseRef}`); const initialData = init ? init(...args) : {}; - const rawData = {}; + const rawState = {}; for (const prop of Object.getOwnPropertyNames(initialData)) { const data = serialize(initialData[prop]); if (durable) { @@ -625,18 +627,18 @@ export function makeVirtualObjectManager( ); } data.slots.map(vrm.addReachableVref); - rawData[prop] = data; + rawState[prop] = data; propertyNames.add(prop); } - const innerSelf = { baseRef, rawData, repCount: 0 }; - const [toHold, toExpose, wrappedData] = makeRepresentative( + const innerSelf = { baseRef, rawState, repCount: 0 }; + const [toHold, toExpose, state] = makeRepresentative( innerSelf, true, false, ); registerValue(baseRef, toHold, Array.isArray(toHold)); if (finish) { - finish(wrappedData, toExpose); + finish(state, toExpose); } cache.markDirty(innerSelf); return toExpose; diff --git a/packages/SwingSet/test/virtualObjects/test-representatives.js b/packages/SwingSet/test/virtualObjects/test-representatives.js index 7c32480a1f8f..2213b7d51535 100644 --- a/packages/SwingSet/test/virtualObjects/test-representatives.js +++ b/packages/SwingSet/test/virtualObjects/test-representatives.js @@ -391,3 +391,33 @@ test('virtual object gc', async t => { 'v1.vs.vom.o+1/9': '{"label":{"body":"\\"thing #9\\"","slots":[]}}', }); }); + +test('empty facets are not orphaned', async t => { + const config = { + bootstrap: 'bootstrap', + vats: { + bob: { + sourceSpec: new URL('vat-orphan-bob.js', import.meta.url).pathname, + creationOptions: { + virtualObjectCacheSize: 0, + }, + }, + bootstrap: { + sourceSpec: new URL('vat-orphan-bootstrap.js', import.meta.url) + .pathname, + }, + }, + }; + + const hostStorage = provideHostStorage(); + + const c = await buildVatController(config, [], { hostStorage }); + c.pinVatRoot('bootstrap'); + + await c.run(); + t.deepEqual( + c.kpResolution(c.bootstrapResult), + capargs({ '@qclass': 'undefined' }), + ); + t.deepEqual(c.dump().log, ['compare originalFacet === thing : true']); +}); diff --git a/packages/SwingSet/test/virtualObjects/test-virtualObjectCache.js b/packages/SwingSet/test/virtualObjects/test-virtualObjectCache.js index dea31ca2a115..af485b8d8df3 100644 --- a/packages/SwingSet/test/virtualObjects/test-virtualObjectCache.js +++ b/packages/SwingSet/test/virtualObjects/test-virtualObjectCache.js @@ -36,7 +36,7 @@ function makeThing(n) { // object above it return { baseRef: `t${n}`, - rawData: `thing #${n}`, + rawState: `thing #${n}`, dirty: false, }; } @@ -55,12 +55,12 @@ test('cache overflow and refresh', t => { // cache: t5, t4, t3, t2 // after initialization - t.is(things[0].rawData, null); - t.is(things[1].rawData, null); - t.is(things[2].rawData, 'thing #2'); - t.is(things[3].rawData, 'thing #3'); - t.is(things[4].rawData, 'thing #4'); - t.is(things[5].rawData, 'thing #5'); + t.is(things[0].rawState, null); + t.is(things[1].rawState, null); + t.is(things[2].rawState, 'thing #2'); + t.is(things[3].rawState, 'thing #3'); + t.is(things[4].rawState, 'thing #4'); + t.is(things[5].rawState, 'thing #5'); t.deepEqual(store.getLog(), [ ['store', 't0', 'thing #0'], ['store', 't1', 'thing #1'], @@ -68,22 +68,22 @@ test('cache overflow and refresh', t => { // lookup that refreshes cache.lookup('t2'); // cache: t2, t5, t4, t3 - t.is(things[5].rawData, 'thing #5'); + t.is(things[5].rawState, 'thing #5'); t.deepEqual(store.getLog(), []); // lookup that has no effect things[0] = cache.lookup('t0'); // cache: t0, t2, t5, t4 - things[0].rawData = 'changed thing #0'; + things[0].rawState = 'changed thing #0'; cache.markDirty(things[0]); // pretend we changed it - t.is(things[0].rawData, 'changed thing #0'); - t.is(things[3].rawData, null); + t.is(things[0].rawState, 'changed thing #0'); + t.is(things[3].rawState, null); t.deepEqual(store.getLog(), [['store', 't3', 'thing #3']]); // verify refresh cache.refresh(things[4]); // cache: t4, t0, t2, t5 things[1] = cache.lookup('t1'); // cache: t1, t4, t0, t2 - t.is(things[1].rawData, null); - t.is(things[5].rawData, null); + t.is(things[1].rawState, null); + t.is(things[5].rawState, null); t.deepEqual(store.getLog(), [['store', 't5', 'thing #5']]); // verify that everything is there @@ -100,12 +100,12 @@ test('cache overflow and refresh', t => { t.falsy(things[3].dirty); t.falsy(things[4].dirty); t.falsy(things[5].dirty); - t.is(things[0].rawData, 'changed thing #0'); - t.is(things[1].rawData, null); - t.is(things[2].rawData, 'thing #2'); - t.is(things[3].rawData, null); - t.is(things[4].rawData, 'thing #4'); - t.is(things[5].rawData, null); + t.is(things[0].rawState, 'changed thing #0'); + t.is(things[1].rawState, null); + t.is(things[2].rawState, 'thing #2'); + t.is(things[3].rawState, null); + t.is(things[4].rawState, 'thing #4'); + t.is(things[5].rawState, null); t.deepEqual(store.getLog(), [ ['store', 't2', 'thing #2'], ['store', 't0', 'changed thing #0'], @@ -122,25 +122,25 @@ test('cache overflow and refresh', t => { // verify that changes get written things[0] = cache.lookup('t0'); // cache: t0 - things[0].rawData = 'new thing #0'; + things[0].rawState = 'new thing #0'; things[0].dirty = true; things[1] = cache.lookup('t1'); // cache: t1, t0 - things[1].rawData = 'new thing #1'; + things[1].rawState = 'new thing #1'; things[1].dirty = true; things[2] = cache.lookup('t2'); // cache: t2, t1, t0 - things[2].rawData = 'new thing #2'; + things[2].rawState = 'new thing #2'; things[2].dirty = true; things[3] = cache.lookup('t3'); // cache: t3, t2, t1, t0 - things[3].rawData = 'new thing #3'; + things[3].rawState = 'new thing #3'; things[3].dirty = true; things[4] = cache.lookup('t4'); // cache: t4, t3, t2, t1 - things[4].rawData = 'new thing #4'; + things[4].rawState = 'new thing #4'; things[4].dirty = true; things[5] = cache.lookup('t5'); // cache: t5, t4, t3, t2 - things[5].rawData = 'new thing #5'; + things[5].rawState = 'new thing #5'; things[5].dirty = true; - t.is(things[0].rawData, null); - t.is(things[5].rawData, 'new thing #5'); + t.is(things[0].rawState, null); + t.is(things[5].rawState, 'new thing #5'); t.deepEqual(store.getLog(), [ ['store', 't0', 'new thing #0'], ['store', 't1', 'new thing #1'], diff --git a/packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js b/packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js index 9e262b402241..6b6837296eba 100644 --- a/packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js +++ b/packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js @@ -220,6 +220,19 @@ function buildRootObject(vatPowers) { ); const virtualHolder = makeVirtualHolder(); + const makeDualMarkerThing = defineKind( + 'thing', + () => ({ unused: 'uncared for' }), + _state => ({ + facetA: { + methodA: () => 0, + }, + facetB: { + methodB: () => 0, + }, + }), + ); + let nextThingNumber = 0; let heldThing = null; aWeakMap = new WeakMap(); @@ -251,6 +264,10 @@ function buildRootObject(vatPowers) { heldThing = makeFacetedThing('thing #0'); displaceCache(); }, + makeAndHoldDualMarkers() { + heldThing = makeDualMarkerThing().facetA; + displaceCache(); + }, makeAndHoldAndKey(isf) { heldThing = makeNextThing(isf); aWeakMap.set(heldThing, 'arbitrary'); @@ -525,11 +542,11 @@ function validateSetup(v) { ); } -function validateSetupAndCreate(v, rp, what) { +function validateSetupAndCreate(v, rp, what, heldValue = testObjValue) { validateSetup(v); // create - validate(v, matchVatstoreSet(stateKey(what), testObjValue)); + validate(v, matchVatstoreSet(stateKey(what), heldValue)); validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue)); validateReturned(v, rp); @@ -645,9 +662,9 @@ function validateDropHeld(v, rp, what, esp, rc, es) { validateDone(v); } -function validateDropHeldWithGC(v, rp, what, rc) { +function validateDropHeldWithGC(v, rp, what, rc, heldValue = testObjValue) { validateReturned(v, rp); - validateStatusCheck(v, what, rc, NONE, testObjValue); + validateStatusCheck(v, what, rc, NONE, heldValue); validateDelete(v, what); validateDone(v); } @@ -1173,6 +1190,20 @@ test.serial('VO multifacet export 3abab', async t => { validateDropHeldWithGCAndRetireFacets(v, rp, thing, 'ss'); }); +test.serial('VO multifacet markers only', async t => { + const { v, dispatchMessage } = await setupLifecycleTest(t); + const thing = 'o+4/1'; + const thingCapdata = JSON.stringify({ unused: capdata('uncared for') }); + + // lerv -> Lerv Create facets + let rp = await dispatchMessage('makeAndHoldDualMarkers'); + validateSetupAndCreate(v, rp, thing, thingCapdata); + + // Lerv -> lerv Drop in-memory reference, unreferenced VO gets GC'd + rp = await dispatchMessage('dropHeld'); + validateDropHeldWithGC(v, rp, thing, NONE, thingCapdata); +}); + // prettier-ignore function validatePrepareStore3(v, rp, isf) { const thing = facetRef(isf, thingVref(isf, 2), '1'); @@ -1439,19 +1470,19 @@ test.serial('remotable refcount management 1', async t => { validateDone(v); rp = await dispatchMessage('prepareStore3'); - validate(v, matchVatstoreSet(stateKey('o+3/2'), heldThingValue('o+4'))); - validate(v, matchVatstoreSet(stateKey('o+3/3'), heldThingValue('o+4'))); - validate(v, matchVatstoreSet(stateKey('o+3/4'), heldThingValue('o+4'))); + validate(v, matchVatstoreSet(stateKey('o+3/2'), heldThingValue('o+5'))); + validate(v, matchVatstoreSet(stateKey('o+3/3'), heldThingValue('o+5'))); + validate(v, matchVatstoreSet(stateKey('o+3/4'), heldThingValue('o+5'))); validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue)); validateReturned(v, rp); validateDone(v); rp = await dispatchMessage('finishClearHolders'); - validate(v, matchVatstoreGet(stateKey('o+3/2'), heldThingValue('o+4'))); + validate(v, matchVatstoreGet(stateKey('o+3/2'), heldThingValue('o+5'))); validate(v, matchVatstoreSet(stateKey('o+3/2'), heldThingValue(null))); - validate(v, matchVatstoreGet(stateKey('o+3/3'), heldThingValue('o+4'))); + validate(v, matchVatstoreGet(stateKey('o+3/3'), heldThingValue('o+5'))); validate(v, matchVatstoreSet(stateKey('o+3/3'), heldThingValue(null))); - validate(v, matchVatstoreGet(stateKey('o+3/4'), heldThingValue('o+4'))); + validate(v, matchVatstoreGet(stateKey('o+3/4'), heldThingValue('o+5'))); validate(v, matchVatstoreSet(stateKey('o+3/4'), heldThingValue(null))); validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue)); validateReturned(v, rp); @@ -1470,20 +1501,20 @@ test.serial('remotable refcount management 2', async t => { validateDone(v); rp = await dispatchMessage('prepareStore3'); - validate(v, matchVatstoreSet(stateKey('o+3/2'), heldThingValue('o+4'))); - validate(v, matchVatstoreSet(stateKey('o+3/3'), heldThingValue('o+4'))); - validate(v, matchVatstoreSet(stateKey('o+3/4'), heldThingValue('o+4'))); + validate(v, matchVatstoreSet(stateKey('o+3/2'), heldThingValue('o+5'))); + validate(v, matchVatstoreSet(stateKey('o+3/3'), heldThingValue('o+5'))); + validate(v, matchVatstoreSet(stateKey('o+3/4'), heldThingValue('o+5'))); validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue)); validateReturned(v, rp); validateDone(v); rp = await dispatchMessage('finishDropHolders'); validateReturned(v, rp); - validateStatusCheck(v, 'o+3/2', NONE, NONE, heldThingValue('o+4')); + validateStatusCheck(v, 'o+3/2', NONE, NONE, heldThingValue('o+5')); validateDelete(v, 'o+3/2'); - validateStatusCheck(v, 'o+3/3', NONE, NONE, heldThingValue('o+4')); + validateStatusCheck(v, 'o+3/3', NONE, NONE, heldThingValue('o+5')); validateDelete(v, 'o+3/3'); - validateStatusCheck(v, 'o+3/4', NONE, NONE, heldThingValue('o+4')); + validateStatusCheck(v, 'o+3/4', NONE, NONE, heldThingValue('o+5')); validateDelete(v, 'o+3/4'); validateDone(v); }); @@ -1571,28 +1602,28 @@ test.serial('VO holding non-VO', async t => { // Lerv -> LERv Export non-VO rp = await dispatchMessage('exportHeld'); - validate(v, matchResolveOne(rp, thingSer('o+4'))); + validate(v, matchResolveOne(rp, thingSer('o+5'))); validateDone(v); // LERv -> LERV Store non-VO reference virtually rp = await dispatchMessage('storeHeld'); validate(v, matchVatstoreGet(stateKey(virtualHolderVref), heldThingValue(null))); - validate(v, matchVatstoreSet(stateKey(virtualHolderVref), heldThingValue('o+4'))); + validate(v, matchVatstoreSet(stateKey(virtualHolderVref), heldThingValue('o+5'))); validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue)); validateReturned(v, rp); validateDone(v); // LERV -> LeRV Drop the export - await dispatchDropExports('o+4'); + await dispatchDropExports('o+5'); validateDone(v); // LeRV -> LerV Retire the export - await dispatchRetireExports('o+4'); + await dispatchRetireExports('o+5'); validateDone(v); // LerV -> LerV Read non-VO reference from VO and expect it to deserialize successfully rp = await dispatchMessage('fetchAndHold'); - validate(v, matchVatstoreGet(stateKey(virtualHolderVref), heldThingValue('o+4'))); + validate(v, matchVatstoreGet(stateKey(virtualHolderVref), heldThingValue('o+5'))); validate(v, matchVatstoreGet(stateKey(cacheDisplacerVref), cacheObjValue)); validateReturned(v, rp); validateDone(v); diff --git a/packages/SwingSet/test/virtualObjects/vat-orphan-bob.js b/packages/SwingSet/test/virtualObjects/vat-orphan-bob.js new file mode 100644 index 000000000000..0825d1fa8232 --- /dev/null +++ b/packages/SwingSet/test/virtualObjects/vat-orphan-bob.js @@ -0,0 +1,32 @@ +import { Far } from '@endo/marshal'; +import { defineKind } from '../../src/storeModule.js'; + +export function buildRootObject(vatPowers) { + const { testLog } = vatPowers; + + const makeThing = defineKind( + 'thing', + () => ({ unused: 'uncared for' }), + () => ({ + facetA: { + methodA: () => 0, + }, + facetB: { + methodB: () => 0, + }, + }), + ); + + let originalFacet; + + return Far('root', { + getYourThing() { + originalFacet = makeThing().facetA; + makeThing(); + return originalFacet; + }, + isThingYourThing(thing) { + testLog(`compare originalFacet === thing : ${originalFacet === thing}`); + }, + }); +} diff --git a/packages/SwingSet/test/virtualObjects/vat-orphan-bootstrap.js b/packages/SwingSet/test/virtualObjects/vat-orphan-bootstrap.js new file mode 100644 index 000000000000..c2ec9c3d163e --- /dev/null +++ b/packages/SwingSet/test/virtualObjects/vat-orphan-bootstrap.js @@ -0,0 +1,11 @@ +import { E } from '@agoric/eventual-send'; +import { Far } from '@endo/marshal'; + +export function buildRootObject(_vatPowers) { + return Far('root', { + async bootstrap(vats) { + const thing = await E(vats.bob).getYourThing(); + await E(vats.bob).isThingYourThing(thing); + }, + }); +}