diff --git a/packages/swingset-liveslots/src/collectionManager.js b/packages/swingset-liveslots/src/collectionManager.js index 8aa7ddd6e573..9041d06039d1 100644 --- a/packages/swingset-liveslots/src/collectionManager.js +++ b/packages/swingset-liveslots/src/collectionManager.js @@ -293,6 +293,17 @@ export function makeCollectionManager( return `${dbKeyPrefix}${dbEntryKey}`; } + // a "vref" is a string like "o-4" + // an "EncodedKey" is the output of encode-passable: + // * strings become `s${string}`, like "foo" -> "sfoo" + // * positive BigInts become `p${len}:${digits}`, 47n -> "p2:47" + // * refs are assigned an "ordinal" and encode as "r0000000001:o-4" + // a "DBKey" is used to index the vatstore + // * DBKeys for collection entries join a collection prefix to an EncodedKey + // * "foo" -> "vc.5.sfoo" + // * 47n -> "vc.5.p2:47" + // * vref(o-4) -> "vc.5.r0000000001:o-4" + const encodeRemotable = remotable => { // eslint-disable-next-line no-use-before-define const ordinal = getOrdinal(remotable); @@ -308,10 +319,11 @@ export function makeCollectionManager( // the resulting function will encode only `Key` arguments. const encodeKey = makeEncodePassable({ encodeRemotable }); - const vrefFromDBKey = dbKey => dbKey.substring(BIGINT_TAG_LEN + 2); + const vrefFromEncodedKey = encodedKey => + encodedKey.substring(1 + BIGINT_TAG_LEN + 1); const decodeRemotable = encodedKey => - convertSlotToVal(vrefFromDBKey(encodedKey)); + convertSlotToVal(vrefFromEncodedKey(encodedKey)); // `makeDecodePassable` has three named options: // `decodeRemotable`, `decodeError`, and `decodePromise`. @@ -347,10 +359,16 @@ export function makeCollectionManager( } function dbKeyToKey(dbKey) { + // convert e.g. vc.5.r0000000001:o+v10/1 to r0000000001:o+v10/1 const dbEntryKey = dbKey.substring(dbKeyPrefix.length); return decodeKey(dbEntryKey); } + function dbKeyToEncodedKey(dbKey) { + assert(dbKey.startsWith(dbKeyPrefix), dbKey); + return dbKey.substring(dbKeyPrefix.length); + } + function has(key) { const { keyShape } = getSchema(); if (!matches(key, keyShape)) { @@ -563,8 +581,9 @@ export function makeCollectionManager( doMoreGC = value.slots.map(vrm.removeReachableVref).some(b => b) || doMoreGC; syscall.vatstoreDelete(dbKey); - if (isEncodedRemotable(dbKey)) { - const keyVref = vrefFromDBKey(dbKey); + const encodedKey = dbKeyToEncodedKey(dbKey); + if (isEncodedRemotable(encodedKey)) { + const keyVref = vrefFromEncodedKey(encodedKey); if (hasWeakKeys) { vrm.removeRecognizableVref(keyVref, `${collectionID}`, true); } else { diff --git a/packages/swingset-liveslots/src/virtualReferences.js b/packages/swingset-liveslots/src/virtualReferences.js index 09f78ff2ac9d..c1a21187cfc0 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -500,26 +500,52 @@ export function makeVirtualReferenceManager( } /** - * A vref is "recognizable" when it is used as the key of a weak Map - * or Set: that Map/Set can be used to query whether a future - * specimen matches the original or not, without holding onto the - * original. + * A vref is "recognizable" when it is used as the key of a weak + * collection, like a virtual/durable WeakMapStore or WeakSetStore, + * or the ephemeral voAwareWeakMap/Set that we impose upon userspace + * as "WeakMap/WeakSet". The collection can be used to query whether + * a future specimen matches the original or not, without holding + * onto the original. * - * This 'vrefRecognizers' is a Map from those vrefs to the set of - * recognizing weak collections, for virtual keys and non-virtual - * collections. Specifically, the vrefs correspond to imported - * Presences or virtual-object Representatives (Remotables do not - * participate: they are keyed by the actual Remotable object, not - * its vref). The collections are either a VirtualObjectAwareWeakMap - * or a VirtualObjectAwareWeakSet. We remove the entry when the key - * is removed from the collection, and when the entire collection is - * deleted. + * We need "recognition records" to map from the vref to the + * collection that can recognize it. When the vref is retired, we + * use the record to find all the collections from which we need to + * delete entries, so we can release the matching values. This might + * happen because the vref was for a Presence and the kernel just + * told us the upstream vat has deleted it (dispatch.retireImports), + * or because it was for a locally-managed object (an ephemeral + * Remotable or a virtual/durable Representative) and we decided to + * delete it. * - * It is critical that each collection have exactly one recognizer that is - * unique to that collection, because the recognizers themselves will be - * tracked by their object identities, but the recognizer cannot be the - * collection itself else it would prevent the collection from being garbage - * collected. + * The virtual/durable collections track their "recognition records" + * in the vatstore, in keys like "vom.ir.${vref}|${collectionID}". + * These records do not contribute to our RAM usage. + * + * voAwareWeakMap and voAwareWeakSet store their recognition records + * in RAM, using this Map named 'vrefRecognizers'. Each key is a + * vref, and the value is a Set of recognizers. Each recognizer is + * the internal 'virtualObjectMap' in which the collection maps from + * vref to value. These in-RAM collections only use virtualObjectMap + * to track Presence-style (imports) and Representative-style + * (virtual/durable) vrefs: any Remotable-style keys are stored in + * the collection's internal (real) WeakMap under the Remotable + * object itself (because the engine handles the bookkeeping, and + * there is no virtual data in the value that we need to clean up at + * deletion time). + * + * It is critical that each collection have exactly one recognizer + * that is unique to that collection, because the recognizers + * themselves will be tracked by their object identities, but the + * recognizer cannot be the collection itself else it would prevent + * the collection from being garbage collected. + * + * When an individual entry is deleted from the weak collection, we + * must also delete the recognition record. When the collection + * itself is deleted (i.e. because nothing was referencing it), we + * must both delete all recognition records and also notify the + * kernel about any Presence-style vrefs that we can no longer + * recognize (syscall.retireImports). The kernel doesn't care about + * Remotable- or Representative- style vrefs, only the imports. * * TODO: all the "recognizers" in principle could be, and probably should be, * reduced to deleter functions. However, since the VirtualObjectAware @@ -535,14 +561,24 @@ export function makeVirtualReferenceManager( /** @type {Map>} */ const vrefRecognizers = new Map(); + /** + * @param {*} value The vref-bearing object used as the collection key + * @param {string|Recognizer} recognizer The collectionID or virtualObjectMap for the collection + * @param {boolean} [recognizerIsVirtual] true for virtual/durable Stores, false for voAwareWeakMap/Set + */ function addRecognizableValue(value, recognizer, recognizerIsVirtual) { const vref = getSlotForVal(value); if (vref) { const { type, allocatedByVat, virtual, durable } = parseVatSlot(vref); - if (type === 'object' && (!allocatedByVat || virtual || durable)) { + if (type === 'object') { + // recognizerSet (voAwareWeakMap/Set) doesn't track Remotables + const notRemotable = !allocatedByVat || virtual || durable; + if (recognizerIsVirtual) { + assert.typeof(recognizer, 'string'); syscall.vatstoreSet(`vom.ir.${vref}|${recognizer}`, '1'); - } else { + } else if (notRemotable) { + assert.typeof(recognizer, 'object'); let recognizerSet = vrefRecognizers.get(vref); if (!recognizerSet) { recognizerSet = new Set(); @@ -554,18 +590,33 @@ export function makeVirtualReferenceManager( } } + /** + * @param {string} vref The vref or the object used as the collection key + * @param {string|Recognizer} recognizer The collectionID or virtualObjectMap for the collection + * @param {boolean} [recognizerIsVirtual] true for virtual/durable Stores, false for voAwareWeakMap/Set + */ function removeRecognizableVref(vref, recognizer, recognizerIsVirtual) { const { type, allocatedByVat, virtual, durable } = parseVatSlot(vref); - if (type === 'object' && (!allocatedByVat || virtual || durable)) { + if (type === 'object') { + // addToPossiblyDeadSet only needs Presence-style vrefs + const isPresence = !allocatedByVat; + // recognizerSet (voAwareWeakMap/Set) doesn't track Remotables + const notRemotable = !allocatedByVat || virtual || durable; + if (recognizerIsVirtual) { + assert.typeof(recognizer, 'string'); syscall.vatstoreDelete(`vom.ir.${vref}|${recognizer}`); - } else { + if (isPresence) { + addToPossiblyRetiredSet(vref); + } + } else if (notRemotable) { + assert.typeof(recognizer, 'object'); const recognizerSet = vrefRecognizers.get(vref); assert(recognizerSet && recognizerSet.has(recognizer)); recognizerSet.delete(recognizer); if (recognizerSet.size === 0) { vrefRecognizers.delete(vref); - if (!allocatedByVat) { + if (isPresence) { addToPossiblyRetiredSet(vref); } } @@ -573,6 +624,11 @@ export function makeVirtualReferenceManager( } } + /** + * @param {*} value The vref-bearing object used as the collection key + * @param {string|Recognizer} recognizer The collectionID or virtualObjectMap for the collection + * @param {boolean} [recognizerIsVirtual] true for virtual/durable Stores, false for voAwareWeakMap/Set + */ function removeRecognizableValue(value, recognizer, recognizerIsVirtual) { const vref = getSlotForVal(value); if (vref) { diff --git a/packages/swingset-liveslots/test/clear-collection.test.js b/packages/swingset-liveslots/test/clear-collection.test.js new file mode 100644 index 000000000000..acd9d925cffc --- /dev/null +++ b/packages/swingset-liveslots/test/clear-collection.test.js @@ -0,0 +1,568 @@ +import test from 'ava'; + +import { Far } from '@endo/marshal'; +import { kser, kslot } from '@agoric/kmarshal'; +import { makeLiveSlots } from '../src/liveslots.js'; +import { parseVatSlot } from '../src/parseVatSlots.js'; +import { buildSyscall } from './liveslots-helpers.js'; +import { makeMessage, makeStartVat, makeBringOutYourDead } from './util.js'; +import { makeMockGC } from './mock-gc.js'; + +const getPrefixedKeys = (map, prefix) => { + const keys = []; + for (const key of map.keys()) { + if (key.startsWith(prefix)) { + keys.push(key.substring(prefix.length)); + } + } + return keys; +}; + +const collectionMetaKeys = new Set([ + '|nextOrdinal', + '|entryCount', + '|schemata', +]); + +const scanCollection = (kvStore, collectionID) => { + const collectionPrefix = `vc.${collectionID}.`; + const ordinalAssignments = []; + const entries = []; + const metaKeys = []; + let totalKeys = 0; + for (const key of getPrefixedKeys(kvStore, collectionPrefix)) { + totalKeys += 1; + if (key.startsWith('|')) { + if (collectionMetaKeys.has(key)) { + metaKeys.push(key); + } else { + ordinalAssignments.push(key); + } + } else { + entries.push(key); + } + } + const keyVrefs = []; + const refcounts = {}; + for (const ordinalKey of ordinalAssignments) { + const vref = ordinalKey.substring(1); + keyVrefs.push(vref); + const rcKey = `vom.rc.${vref}`; + refcounts[vref] = kvStore.get(rcKey); + } + return { + totalKeys, + metaKeys, + ordinalAssignments, + entries, + keyVrefs, + refcounts, + }; +}; + +const GC = ['dropImports', 'retireImports', 'retireExports']; + +const doTest = async (t, mode) => { + assert(['strong-clear', 'strong-delete', 'weak-delete'].includes(mode)); + const kvStore = new Map(); + const { syscall, log } = buildSyscall({ kvStore }); + const gcTools = makeMockGC(); + const COUNT = 5; + + // we'll either call holder.clear() to exercise manual clearing, or + // gcTools.kill(holder) to exercise the collection itself being + // deleted + + let holder; + + function build(vatPowers) { + const { defineKind, makeScalarBigSetStore } = vatPowers.VatData; + const make = defineKind('target', () => ({}), {}); + holder = makeScalarBigSetStore('holder'); + const root = Far('root', { + create() { + for (let i = 0; i < COUNT; i += 1) { + // vrefs are all `o+v10/${N}`, N=1..10 + const target = make(); + holder.add(target); + // we immediately delete the presence, but the finalizers + // won't run until gcTools.flushAllFRs() + gcTools.kill(target); + } + }, + clear() { + holder.clear(); + }, + }); + return root; + } + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, () => ({ + buildRootObject: build, + })); + const { dispatch, testHooks } = ls; + const { valToSlot } = testHooks; + await dispatch(makeStartVat(kser())); + log.length = 0; + + const rootA = 'o+0'; + + await dispatch(makeMessage(rootA, 'create', [])); + log.length = 0; + + const holderVref = valToSlot.get(holder); + const collectionID = Number(parseVatSlot(holderVref).subid); + const populated = scanCollection(kvStore, collectionID); + t.is(populated.ordinalAssignments.length, COUNT); + t.is(populated.entries.length, COUNT); + t.is(populated.keyVrefs.length, COUNT); + t.true(populated.keyVrefs.every(vref => populated.refcounts[vref] === '1')); + + // Collect the representatives, leaving only the virtual-data + // pillar. This BOYD finds non-zero virtual-data refcounts for all + // five VOs, so they are not deleted. + gcTools.flushAllFRs(); + const boyd1 = await dispatch(makeBringOutYourDead()); + t.is(boyd1.possiblyDeadSet, 0); + t.is(boyd1.possiblyRetiredSet, 0); + log.length = 0; + t.is(scanCollection(kvStore, collectionID).totalKeys, populated.totalKeys); + + if (mode === 'strong-clear') { + // clearing the collections should delete both the data key and the + // ordinal key for each entry, but it won't delete the values, that + // is deferred until BOYD. The metadata is retained, because the + // collection has been cleared, not deleted. + await dispatch(makeMessage(rootA, 'clear', [])); + } else if (mode === 'strong-delete') { + gcTools.kill(holder); + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + // that should clear everything, both the holder and the referenced + // targets + } + + const scan2 = scanCollection(kvStore, collectionID); + + // all entries should be gone + t.is(scan2.ordinalAssignments.length, 0); + t.is(scan2.entries.length, 0); + t.is(scan2.keyVrefs.length, 0); + + if (mode === 'strong-clear') { + // but the collection itself is still present + t.is(scan2.metaKeys.length, populated.metaKeys.length); + for (const vref of populated.keyVrefs) { + const rcKey = `vom.rc.${vref}`; + const rc = kvStore.get(rcKey); + // the target refcounts should be zero (= undefined) + t.is(rc, undefined); + // but the data should still be present + const dataKey = `vom.${vref}`; + const data = kvStore.get(dataKey); + t.is(data, '{}'); + } + // and we need one more BOYD to notice the zero refcounts and + // delete the data + await dispatch(makeBringOutYourDead()); + } else if (mode === 'strong-delete') { + // the collection should be gone + t.is(scan2.metaKeys.length, 0); + t.is(scan2.totalKeys, 0); + } + + // all the targets should be collected now + for (const vref of populated.keyVrefs) { + const rcKey = `vom.rc.${vref}`; + const rc = kvStore.get(rcKey); + // the target refcounts should be zero (= undefined) + t.is(rc, undefined); + const dataKey = `vom.${vref}`; + const data = kvStore.get(dataKey); + t.is(data, undefined); + } + + // none of the Presences were exported, so no GC syscalls + const gcCalls1 = log.filter(l => GC.includes(l.type)); + t.deepEqual(gcCalls1, []); +}; + +// When a virtual collection's keys are the only reference to a +// virtual object, collection.clear() should let them be deleted. Bug +// #8756 caused the keys to be retained by mistake. + +test('collection.clear() deletes keys', async t => { + await doTest(t, 'strong-clear'); +}); + +// Allowing GC to delete a strong collection should delete/release the +// keys too + +test('deleting a strong collection will delete the keys', async t => { + await doTest(t, 'strong-delete'); +}); + +// Allowing GC to delete a weak collection should retire the keys, and +// delete/release the contents. + +test('deleting a weak collection will retire the keys', async t => { + const kvStore = new Map(); + const { syscall, log } = buildSyscall({ kvStore }); + const gcTools = makeMockGC(); + const COUNT = 5; + const allVrefs = []; + const allKslots = []; + for (let i = 0; i < COUNT; i += 1) { + const vref = `o-${i + 1}`; + allVrefs.push(vref); + allKslots.push(kslot(vref, 'imported')); + } + + let recognizer; + + // Import a bunch of Presences and hold them in a weakset. Drop the + // imports, but retain recognition, until we drop the weakset, which + // should delete the collection and notify the kernel that we aren't + // recognizing the keys (syscall.retireImports) + function build(vatPowers) { + const { makeScalarBigWeakSetStore } = vatPowers.VatData; + recognizer = makeScalarBigWeakSetStore('recognizer'); + const root = Far('root', { + create(presences) { + for (const p of presences) { + recognizer.add(p); + // we immediately delete the presence, but the finalizers + // won't run until gcTools.flushAllFRs() + gcTools.kill(p); + } + }, + }); + return root; + } + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, () => ({ + buildRootObject: build, + })); + const { dispatch, testHooks } = ls; + const { valToSlot } = testHooks; + await dispatch(makeStartVat(kser())); + log.length = 0; + + const rootA = 'o+0'; + + await dispatch(makeMessage(rootA, 'create', [allKslots])); + log.length = 0; + + const recognizerVref = valToSlot.get(recognizer); + const collectionID = Number(parseVatSlot(recognizerVref).subid); + + // all the Presences should be recognized by the collection, but not + // referenced + const populated = scanCollection(kvStore, collectionID); + t.is(populated.ordinalAssignments.length, COUNT); + t.is(populated.entries.length, COUNT); + t.is(populated.keyVrefs.length, COUNT); + t.true( + populated.keyVrefs.every(vref => populated.refcounts[vref] === undefined), + ); + // and there should be recognizer (.ir) entries for each vref|collection pair + t.true( + populated.keyVrefs.every(vref => + kvStore.has(`vom.ir.${vref}|${collectionID}`), + ), + ); + + // collect the Presences, which was the only remaining reachability + // pillar, leaving just the recognizers + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + // no changes to the collection + t.deepEqual(scanCollection(kvStore, collectionID), populated); + // but the Presence vrefs should be dropped + const gcCalls1 = log.filter(l => GC.includes(l.type)); + t.deepEqual(gcCalls1, [{ type: 'dropImports', slots: allVrefs }]); + log.length = 0; + + // now free the whole collection + gcTools.kill(recognizer); + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + + // the collection should be gone + const scan2 = scanCollection(kvStore, collectionID); + t.is(scan2.totalKeys, 0); + + // and the .ir entries + t.true( + populated.keyVrefs.every( + vref => !kvStore.has(`vom.ir.${vref}|${collectionID}`), + ), + ); + + // and the kernel should be notified that we don't care anymore + const gcCalls2 = log.filter(l => GC.includes(l.type)); + t.deepEqual(gcCalls2, [{ type: 'retireImports', slots: allVrefs }]); +}); + +// Allowing GC to delete a voAwareWeakSet (or Map) should retire the +// keys, and delete/release the contents. + +test('deleting a voAwareWeakSet will retire the keys', async t => { + const kvStore = new Map(); + const { syscall, log } = buildSyscall({ kvStore }); + const gcTools = makeMockGC(); + const COUNT = 5; + const allVrefs = []; + const allKslots = []; + for (let i = 0; i < COUNT; i += 1) { + const vref = `o-${i + 1}`; + allVrefs.push(vref); + allKslots.push(kslot(vref, 'imported')); + } + + let recognizer; + + // Import a bunch of Presences and hold them in a weakset. Drop the + // imports, but retain recognition, until we drop the weakset, which + // should delete the collection and notify the kernel that we aren't + // recognizing the keys (syscall.retireImports) + function build(vatPowers) { + recognizer = new vatPowers.WeakSet(); + const root = Far('root', { + create(presences) { + for (const p of presences) { + recognizer.add(p); + // we immediately delete the presence, but the finalizers + // won't run until gcTools.flushAllFRs() + gcTools.kill(p); + } + }, + }); + return root; + } + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, () => ({ + buildRootObject: build, + })); + const { dispatch, testHooks } = ls; + const { vrefRecognizers } = testHooks; + await dispatch(makeStartVat(kser())); + log.length = 0; + + const rootA = 'o+0'; + + await dispatch(makeMessage(rootA, 'create', [allKslots])); + log.length = 0; + + // the WeakSet has no vref, and doesn't store anything like ".ir" + // entries in vatstore, but we can snoop on its internal + // tables. vrefRecognizers is a Map, keyed by vref, with an entry + // for every vref that is tracked by any voAwareWeakMap/Set. The + // value is a Set of virtualObjectMaps, the internal/hidden Set used + // by voAwareWeakMap/Sets. + + const vrefKeys = [...vrefRecognizers.keys()].sort(); + + // we should be tracking all the presences + t.is(vrefKeys.length, COUNT); + // each vref should have a single recognizer + t.true(vrefKeys.every(vref => vrefRecognizers.get(vref).size === 1)); + // that single recognizer should be the virtualObjectMap for our voAwareWeakSet + const virtualObjectMap = [...vrefRecognizers.get(vrefKeys[0])][0]; + // they should all point to the same one + t.true( + vrefKeys.every( + vref => [...vrefRecognizers.get(vref)][0] === virtualObjectMap, + ), + ); + + // collect the Presences, which was the only remaining reachability + // pillar, leaving just the recognizers + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + // no changes to the collection + t.is(vrefKeys.length, COUNT); + t.true(vrefKeys.every(vref => vrefRecognizers.get(vref).size === 1)); + t.true( + vrefKeys.every( + vref => [...vrefRecognizers.get(vref)][0] === virtualObjectMap, + ), + ); + + // but the Presence vrefs should be dropped + const gcCalls1 = log.filter(l => GC.includes(l.type)); + t.deepEqual(gcCalls1, [{ type: 'dropImports', slots: allVrefs }]); + log.length = 0; + + // now free the whole collection + gcTools.kill(recognizer); + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + + // the collection should be gone + t.is(vrefRecognizers.size, 0); + + // and the kernel should be notified that we don't care anymore + const gcCalls2 = log.filter(l => GC.includes(l.type)); + t.deepEqual(gcCalls2, [{ type: 'retireImports', slots: allVrefs }]); +}); + +// explore remediation/leftover problems from bugs #7355, #8756, #9956 +// where the DB has corrupted data leftover from before they were fixed + +test('missing recognition record during delete', async t => { + const kvStore = new Map(); + const { syscall, log } = buildSyscall({ kvStore }); + const gcTools = makeMockGC(); + + let recognizer; + let target; + + // liveslots didn't always add "vom.ir." recognition-records for + // Remotable-style keys, nor remove them when the key was + // deleted. So a kernel which adds a key, upgrades to the current + // (fixed) version, then attempts to delete the key, will not see + // the record it is expecting. Make sure this doesn't cause + // problems. + + function build(vatPowers) { + const { makeScalarBigWeakSetStore } = vatPowers.VatData; + recognizer = makeScalarBigWeakSetStore('recognizer'); + target = Far('target', {}); + const root = Far('root', { + store() { + recognizer.add(target); + }, + delete() { + recognizer.delete(target); + }, + }); + return root; + } + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, () => ({ + buildRootObject: build, + })); + const { dispatch, testHooks } = ls; + const { valToSlot } = testHooks; + await dispatch(makeStartVat(kser())); + log.length = 0; + + const rootA = 'o+0'; + + await dispatch(makeMessage(rootA, 'store')); + log.length = 0; + + const targetVref = valToSlot.get(target); + const recognizerVref = valToSlot.get(recognizer); + const collectionID = Number(parseVatSlot(recognizerVref).subid); + const ordinalAssignmentKey = `vc.${collectionID}.|${targetVref}`; + const ordinalNumber = kvStore.get(ordinalAssignmentKey); + t.is(ordinalNumber, '1'); + const dataKey = `vc.${collectionID}.r0000000001:${targetVref}`; + const value = kvStore.get(dataKey); + t.deepEqual(JSON.parse(value), { body: '#null', slots: [] }); + + // the correct recognition record key + const rrKey = `vom.ir.${targetVref}|${collectionID}`; + + // our fixed version creates one + t.is(kvStore.get(rrKey), '1'); + + // now simulate data from the broken version, by deleting the + // recognition record + kvStore.delete(rrKey); + + // check that deleting the same Remotable doesn't break + await dispatch(makeMessage(rootA, 'delete')); + t.false(kvStore.has(ordinalAssignmentKey)); + t.false(kvStore.has(dataKey)); +}); + +test.failing('leftover ordinal-assignment record during init', async t => { + const kvStore = new Map(); + const { syscall, log } = buildSyscall({ kvStore }); + const gcTools = makeMockGC(); + + let store; + let target; + /** @type {any} */ + let result; + + // liveslots didn't always remove the "vc.${collectionID}.|${vref}" + // ordinal-assignment records when clearing or deleting a + // collection. So a kernel which adds a key, upgrades to the current + // (fixed) version, then clears the collection, will have a leftover + // record. Make sure this doesn't cause problems when iterating keys + // or re-adding the same key later. + + function build(vatPowers) { + const { makeScalarBigMapStore } = vatPowers.VatData; + store = makeScalarBigMapStore('store'); + target = Far('target', {}); + const root = Far('root', { + store() { + try { + store.init(target, 123); + result = 'ok'; + } catch (e) { + result = e; + } + }, + clear() { + store.clear(); + }, + has() { + result = store.has(target); + }, + }); + return root; + } + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, () => ({ + buildRootObject: build, + })); + const { dispatch, testHooks } = ls; + const { valToSlot } = testHooks; + await dispatch(makeStartVat(kser())); + log.length = 0; + + const rootA = 'o+0'; + + result = undefined; + await dispatch(makeMessage(rootA, 'store')); + t.is(result, 'ok'); + + const targetVref = valToSlot.get(target); + const storeVref = valToSlot.get(store); + const collectionID = Number(parseVatSlot(storeVref).subid); + const ordinalAssignmentKey = `vc.${collectionID}.|${targetVref}`; + const ordinalNumber = kvStore.get(ordinalAssignmentKey); + t.is(ordinalNumber, '1'); + const dataKey = `vc.${collectionID}.r0000000001:${targetVref}`; + const value = kvStore.get(dataKey); + t.deepEqual(JSON.parse(value), { body: '#123', slots: [] }); + + result = undefined; + await dispatch(makeMessage(rootA, 'clear')); + + // now simulate data from the broken version, by restoring the + // ordinal-assignment record, as if the code failed to delete it + + kvStore.set(ordinalAssignmentKey, '1'); + + // problem 1: store.has() should report "false", but incorrectly + // returns "true" + + result = undefined; + await dispatch(makeMessage(rootA, 'has')); + t.is(result, false); + + // problem 2: store.init() to re-add the old key should succeed, but + // incorrectly fails (because the store thinks the key is already + // present) + + result = undefined; + await dispatch(makeMessage(rootA, 'store')); + t.is(result, 'ok'); +}); diff --git a/packages/swingset-liveslots/test/gc-before-finalizer.test.js b/packages/swingset-liveslots/test/gc-before-finalizer.test.js index 2e073b4cd82c..cc460114febe 100644 --- a/packages/swingset-liveslots/test/gc-before-finalizer.test.js +++ b/packages/swingset-liveslots/test/gc-before-finalizer.test.js @@ -7,7 +7,12 @@ import { makeStartVat, makeMessage, makeBringOutYourDead } from './util.js'; import { makeMockGC } from './mock-gc.js'; const justGC = log => - log.filter(l => l.type === 'dropImports' || l.type === 'retireImports'); + log.filter( + l => + l.type === 'dropImports' || + l.type === 'retireImports' || + l.type === 'retireExports', + ); test('presence in COLLECTED state is not dropped yet', async t => { const { syscall, log } = buildSyscall(); @@ -109,12 +114,7 @@ test('presence in COLLECTED state is not dropped yet', async t => { ]); }); -// disabled until #9956 and #9959 are fixed, which interfere with this -// test - -/* - -test.failing('presence in COLLECTED state is not retired early', async t => { +test('presence in COLLECTED state is not retired early', async t => { const { syscall, log } = buildSyscall(); const gcTools = makeMockGC(); @@ -154,7 +154,7 @@ test.failing('presence in COLLECTED state is not retired early', async t => { let myWeakStore; function buildRootObject(vatPowers) { - const { VatData, WeakSet } = vatPowers; + const { VatData } = vatPowers; const { makeScalarBigMapStore, makeScalarBigWeakSetStore } = VatData; const store = makeScalarBigMapStore(); myWeakStore = makeScalarBigWeakSetStore(); @@ -187,10 +187,9 @@ test.failing('presence in COLLECTED state is not retired early', async t => { t.is(possiblyDeadSet.size, 0); t.is(possiblyRetiredSet.size, 0); - console.log(`-- starting`); // step 2: delete vdata ref to weakstore, make myPresence COLLECTED await dispatch(makeMessage('o+0', 'dropWeakStore', [])); - gcTools.kill(myPresence) + gcTools.kill(myPresence); log.length = 0; // weakstore is possiblyDead (NARRATORS VOICE: it's dead). Presence // is not, because the finalizer hasn't run. @@ -200,29 +199,31 @@ test.failing('presence in COLLECTED state is not retired early', async t => { // the empty weakref is still there t.true(slotToVal.has('o-1')); - // step 3: BOYD. It will collect myWeakStore on the first pass, // whose deleter should clear all entries, which will add its // recognized vrefs to possiblyRetiredSet. The post-pass will check // o-1 for reachability with slotToVal.has, and because that says it // is reachable, it will not be retired (even though it has no - // recognizer by now) - console.log(`-- doing first BOYD`); + // recognizer by now). + // + // *If* scanForDeadObjects() were mistakenly using getValForSlot() + // *instead of slotToVal.has(), we would see a retireImports here, + // *which would be a vat-fatal error, because we haven't seen a + // *dropImports yet. await dispatch(makeBringOutYourDead()); + // I tested this manually, by modifying boyd-gc.js *to use + // getValForSlot, and observed that this deepEqual(justGC(log), []) + // failed: it had an unexpected retireImports t.deepEqual(justGC(log), []); log.length = 0; // eventually, the finalizer runs gcTools.flushAllFRs(); - console.log(`-- doing second BOYD`); // *now* a BOYD will drop+retire await dispatch(makeBringOutYourDead()); - console.log(log); t.deepEqual(justGC(log), [ { type: 'dropImports', slots: ['o-1'] }, { type: 'retireImports', slots: ['o-1'] }, ]); }); - -*/ diff --git a/packages/swingset-liveslots/test/liveslots-mock-gc.test.js b/packages/swingset-liveslots/test/liveslots-mock-gc.test.js index d7cff5d0967f..1ab290c621b3 100644 --- a/packages/swingset-liveslots/test/liveslots-mock-gc.test.js +++ b/packages/swingset-liveslots/test/liveslots-mock-gc.test.js @@ -11,6 +11,7 @@ import { makeStartVat, makeBringOutYourDead, makeResolve, + makeRetireImports, } from './util.js'; import { makeMockGC } from './mock-gc.js'; @@ -465,3 +466,101 @@ for (const firstType of ['object', 'collection']) { } // test('double-free', doublefreetest, { firstType: 'object', lastType: 'collection', order: 'first->last' }); + +test('retirement', async t => { + const { syscall, fakestore, log } = buildSyscall(); + const gcTools = makeMockGC(); + + // A is a weak collection, with one entry, whose key is B (a + // Presence). We drop the RAM pillar for B and do a BOYD, which + // should provoke a syscall.dropImports. Then, when we delete A (by + // dropping the RAM pillar), the next BOYD should see a + // `syscall.retireImports`. + + let weakmapA; + let presenceB; + + function buildRootObject(vatPowers) { + const { VatData } = vatPowers; + const { makeScalarBigWeakMapStore } = VatData; + + weakmapA = makeScalarBigWeakMapStore(); + + return Far('root', { + add: p => { + presenceB = p; + weakmapA.init(presenceB, 'value'); + }, + }); + } + + const makeNS = () => ({ buildRootObject }); + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, makeNS); + const { dispatch, testHooks } = ls; + const { valToSlot } = testHooks; + + await dispatch(makeStartVat(kser())); + log.length = 0; + const weakmapAvref = valToSlot.get(weakmapA); + const { subid } = parseVatSlot(weakmapAvref); + const collectionID = String(subid); + + const rootA = 'o+0'; + const presenceBvref = 'o-1'; + await dispatch(makeMessage(rootA, 'add', [kslot(presenceBvref)])); + log.length = 0; + + // the fact that weakmapA can recognize presenceA is recorded in a + // vatstore key + const recognizerKey = `vom.ir.${presenceBvref}|${collectionID}`; + t.is(fakestore.get(recognizerKey), '1'); + + // tell mockGC that userspace has dropped presenceB + gcTools.kill(presenceB); + gcTools.flushAllFRs(); + + await dispatch(makeBringOutYourDead()); + const priorKey = `vom.ir.${presenceBvref}|`; + + t.deepEqual(log.splice(0), [ + // when a Presence is dropped, scanForDeadObjects can't drop the + // underlying vref import until it knows that virtual data isn't + // holding a reference, so we expect a refcount check + { type: 'vatstoreGet', key: `vom.rc.${presenceBvref}`, result: undefined }, + + // the vref is now in importsToDrop, but since this commonly means + // it can be retired too, scanForDeadObjects goes ahead and checks + // for recognizers + { type: 'vatstoreGetNextKey', priorKey, result: recognizerKey }, + + // it found a recognizer, so the vref cannot be retired + // yet. scanForDeadObjects finishes the BOYD by emitting the + // dropImports, but should keep watching for an opportunity to + // retire it too + { type: 'dropImports', slots: [presenceBvref] }, + ]); + + // now tell mockGC that we're dropping the weakmap too + gcTools.kill(weakmapA); + gcTools.flushAllFRs(); + + // this will provoke the deletion of the collection and all its + // data. It should *also* trigger a syscall.retireImports of the + // no-longer-recognizable key + await dispatch(makeBringOutYourDead()); + const retires = log.filter(e => e.type === 'retireImports'); + + t.deepEqual(retires, [{ type: 'retireImports', slots: [presenceBvref] }]); + + // If the bug is present, the vat won't send `syscall.retireImports` + // to the kernel. In a full system, that means the kernel can + // eventually send a `dispatch.retireImports` into the vat, if/when + // the object's hosting vat decides to drop it. Make sure that won't + // cause a crash. + + if (!retires.length) { + console.log(`testing kernel's dispatch.retireImports`); + await dispatch(makeRetireImports(presenceBvref)); + console.log(`dispatch.retireImports did not crash`); + } +}); diff --git a/packages/swingset-liveslots/test/weakset-dropped-remotable.test.js b/packages/swingset-liveslots/test/weakset-dropped-remotable.test.js new file mode 100644 index 000000000000..766182973704 --- /dev/null +++ b/packages/swingset-liveslots/test/weakset-dropped-remotable.test.js @@ -0,0 +1,50 @@ +import test from 'ava'; +import { Far } from '@endo/marshal'; +import { kser, kslot } from '@agoric/kmarshal'; +import { makeLiveSlots } from '../src/liveslots.js'; +import { buildSyscall } from './liveslots-helpers.js'; +import { makeStartVat, makeMessage, makeBringOutYourDead } from './util.js'; +import { makeMockGC } from './mock-gc.js'; + +// Test for https://github.com/Agoric/agoric-sdk/issues/9956 + +test('delete remotable key from weakset', async t => { + const { syscall, log } = buildSyscall(); + const gcTools = makeMockGC(); + const rem = Far('remotable', {}); + + function buildRootObject(vatPowers) { + const { VatData } = vatPowers; + const { makeScalarBigWeakMapStore } = VatData; + const wms = makeScalarBigWeakMapStore(); + return Far('root', { + store: p => { + wms.init(rem, p); + gcTools.kill(p); + }, + }); + } + + const makeNS = () => ({ buildRootObject }); + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, makeNS); + const { dispatch } = ls; + await dispatch(makeStartVat(kser())); + + await dispatch(makeMessage('o+0', 'store', [kslot('o-1')])); + + // pretend the Remotable was dropped from RAM + log.length = 0; + gcTools.kill(rem); + gcTools.flushAllFRs(); + await dispatch(makeBringOutYourDead()); + + // that ought to emit a drop and retire for the presence + const gcCalls = log.filter( + l => l.type === 'dropImports' || l.type === 'retireImports', + ); + t.deepEqual(gcCalls, [ + { type: 'dropImports', slots: ['o-1'] }, + { type: 'retireImports', slots: ['o-1'] }, + ]); + log.length = 0; +});