diff --git a/packages/swingset-liveslots/src/liveslots.js b/packages/swingset-liveslots/src/liveslots.js index 18aaeea5bc4..55cc0a7b649 100644 --- a/packages/swingset-liveslots/src/liveslots.js +++ b/packages/swingset-liveslots/src/liveslots.js @@ -257,18 +257,37 @@ function build( // eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await await gcTools.gcAndFinalize(); - // `deadSet` is the subset of those vrefs which lack an in-memory - // manifestation *right now* (i.e. the non-resurrected ones), for which - // we must check the remaining pillars. + // possiblyDeadSet contains a baseref for everything (Presences, + // Remotables, Representatives) that might have lost a + // pillar. The object might still be supported by other pillars, + // and the lost pillar might have been reinstanted by the time + // we get here. The first step is to filter this down to a list + // of definitely dead baserefs. + const deadSet = new Set(); + for (const baseRef of possiblyDeadSet) { // eslint-disable-next-line no-use-before-define - if (!slotToVal.has(baseRef)) { - deadSet.add(baseRef); + if (slotToVal.has(baseRef)) { + continue; // RAM pillar remains } + const { virtual, durable, type } = parseVatSlot(baseRef); + assert(type === 'object', `unprepared to track ${type}`); + if (virtual || durable) { + // eslint-disable-next-line no-use-before-define + if (vrm.isVirtualObjectReachable(baseRef)) { + continue; // vdata or export pillar remains + } + } + deadSet.add(baseRef); } possiblyDeadSet.clear(); + // deadBaserefs now contains objects which are certainly dead + + // possiblyRetiredSet holds (a subset of??) baserefs which have + // lost a recognizer recently. TODO recheck this + for (const vref of possiblyRetiredSet) { // eslint-disable-next-line no-use-before-define if (!getValForSlot(vref) && !deadSet.has(vref)) { @@ -291,7 +310,7 @@ function build( if (virtual || durable) { // Representative: send nothing, but perform refcount checking // eslint-disable-next-line no-use-before-define - const [gcAgain, retirees] = vrm.possibleVirtualObjectDeath(baseRef); + const [gcAgain, retirees] = vrm.deleteVirtualObject(baseRef); if (retirees) { retirees.map(retiree => exportsToRetire.add(retiree)); } diff --git a/packages/swingset-liveslots/src/virtualReferences.js b/packages/swingset-liveslots/src/virtualReferences.js index 33b921cbb30..3a7be8eb16b 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -38,8 +38,7 @@ export function makeVirtualReferenceManager( ); /** - * Check if a virtual object is truly dead - i.e., unreachable - and truly - * delete it if so. + * Check if a virtual object is unreachable via virtual-data references. * * A virtual object is kept alive if it or any of its facets are reachable by * any of three legs: @@ -48,9 +47,28 @@ export function makeVirtualReferenceManager( * - virtual references (if so, it will have a refcount > 0) * - being exported (if so, its export flag will be set) * - * This function is called after a leg has been reported missing, and only - * if the memory (Representative) leg is currently missing, to see if the - * other two legs are now gone also. + * This function is called after a leg has been reported missing, + * and reports back on whether the virtual-reference and + * export-status legs remain. The caller (liveslots + * scanForDeadObjects) will combine this with information about the + * RAM leg to decide whether the object is completely unreachable, + * and thus should be deleted. + * + * @param {string} baseRef The virtual object cohort might be dead + * + * @returns {boolean} True if the object remains referenced, false if unreachable + */ + function isVirtualObjectReachable(baseRef) { + const refCount = getRefCount(baseRef); + const [reachable, _retirees] = getExportStatus(baseRef); + return !!(reachable || refCount); + } + + /** + * Delete a virtual object + * + * Once the caller determines that all three legs are gone, they + * call us to delete the object. * * Deletion consists of removing the vatstore entries that describe its state * and track its refcount status. In addition, when a virtual object is @@ -58,23 +76,22 @@ export function makeVirtualReferenceManager( * it had been exported, we also inform the kernel that the vref has been * retired, so other vats can delete their weak collection entries too. * - * @param {string} baseRef The virtual object cohort that's plausibly dead + * @param {string} baseRef The virtual object cohort that's certainly dead * * @returns {[boolean, string[]]} A pair of a flag that's true if this * possibly created a new GC opportunity and an array of vrefs that should * now be regarded as unrecognizable */ - function possibleVirtualObjectDeath(baseRef) { + function deleteVirtualObject(baseRef) { const refCount = getRefCount(baseRef); const [reachable, retirees] = getExportStatus(baseRef); - if (!reachable && refCount === 0) { - let doMoreGC = deleteStoredRepresentation(baseRef); - syscall.vatstoreDelete(`vom.rc.${baseRef}`); - syscall.vatstoreDelete(`vom.es.${baseRef}`); - doMoreGC = ceaseRecognition(baseRef) || doMoreGC; - return [doMoreGC, retirees]; - } - return [false, []]; + assert(!reachable); + assert(!refCount); + let doMoreGC = deleteStoredRepresentation(baseRef); + syscall.vatstoreDelete(`vom.rc.${baseRef}`); + syscall.vatstoreDelete(`vom.es.${baseRef}`); + doMoreGC = ceaseRecognition(baseRef) || doMoreGC; + return [doMoreGC, retirees]; } /** @@ -577,7 +594,7 @@ export function makeVirtualReferenceManager( // themselves. const kindInfo = kindInfoTable.get(`${p.id}`); // This function can be called either from `dispatch.retireImports` or - // from `possibleVirtualObjectDeath`. In the latter case the vref is + // from `deleteVirtualObject`. In the latter case the vref is // actually a baseRef and so needs to be expanded to cease recognition of // all the facets. if (kindInfo) { @@ -676,7 +693,8 @@ export function makeVirtualReferenceManager( isPresenceReachable, isVrefRecognizable, setExportStatus, - possibleVirtualObjectDeath, + isVirtualObjectReachable, + deleteVirtualObject, ceaseRecognition, setDeleteCollectionEntry, testHooks, diff --git a/packages/swingset-liveslots/test/storeGC/test-weak-key.js b/packages/swingset-liveslots/test/storeGC/test-weak-key.js index c53dcd22785..e8f6ddd320c 100644 --- a/packages/swingset-liveslots/test/storeGC/test-weak-key.js +++ b/packages/swingset-liveslots/test/storeGC/test-weak-key.js @@ -69,13 +69,14 @@ test.serial('verify store weak key GC', async t => { // the full sequence is: // * finalizer(held) pushes vref onto possiblyDeadSet - // * BOYD calls vrm.possibleVirtualObjectDeath + // * BOYD calls vrm.isVirtualObjectReachable // * that checks vdata refcount and export status (vstore reads) // * concludes no pillars are remaining, initiates deletion - // * pVOD uses deleteStoredRepresentation() to delete vobj data - // * 'held' is empty, so has no vobj data to delete - // * vom.(rc.es).${baseRef} keys deleted (refcount/export-status trackers) - // * ceaseRecognition() is called, then pVOD returns + // * BOYD calls vrm.deleteVirtualObject + // * dVO uses deleteStoredRepresentation() to delete vobj data + // * 'held' is empty, so has no vobj data to delete + // * vom.(rc.es).${baseRef} keys deleted (refcount/export-status trackers) + // * ceaseRecognition() is called, then dVO returns // * cR removes from all voAwareWeakMap/Sets (none) // * cR walks vom.ir.${vref}|XX to find weak-store recognizers // * this finds both our WeakSet and our WeakMap diff --git a/packages/swingset-liveslots/test/virtual-objects/test-virtualObjectManager.js b/packages/swingset-liveslots/test/virtual-objects/test-virtualObjectManager.js index 74cd48b8cdb..00e8eab7aaa 100644 --- a/packages/swingset-liveslots/test/virtual-objects/test-virtualObjectManager.js +++ b/packages/swingset-liveslots/test/virtual-objects/test-virtualObjectManager.js @@ -562,7 +562,7 @@ test('durable kind IDs can be reanimated', t => { log, }); const { makeKindHandle, defineDurableKind, flushStateCache } = vom; - const { possibleVirtualObjectDeath } = vrm; + const { isVirtualObjectReachable } = vrm; const { makeScalarBigMapStore } = cm; const { deleteEntry } = fakeStuff; @@ -591,10 +591,11 @@ test('durable kind IDs can be reanimated', t => { t.is(log.shift(), 'set vc.1.|entryCount 1'); t.deepEqual(log, []); - // Forget its existence + // Forget its Representative kindHandle = null; deleteEntry(khid); - possibleVirtualObjectDeath(khid); + t.deepEqual(log, []); + t.truthy(isVirtualObjectReachable(khid)); t.is(log.shift(), `get vom.rc.${khid} => 1`); t.is(log.shift(), `get vom.es.${khid} => undefined`); t.deepEqual(log, []); @@ -631,7 +632,8 @@ test('virtual object gc', t => { const log = []; const { vom, vrm, fakeStuff } = makeFakeVirtualStuff({ log }); const { defineKind, flushStateCache } = vom; - const { setExportStatus, possibleVirtualObjectDeath } = vrm; + const { setExportStatus, deleteVirtualObject, isVirtualObjectReachable } = + vrm; const { deleteEntry, dumpStore } = fakeStuff; const makeThing = defineKind('thing', initThing, thingBehavior); @@ -689,9 +691,12 @@ test('virtual object gc', t => { ]); // This is what the finalizer would do if the local reference was dropped and GC'd - function pretendGC(vref) { + function pretendGC(vref, shouldDelete) { deleteEntry(vref); - possibleVirtualObjectDeath(vref); + t.is(!!isVirtualObjectReachable(vref), !shouldDelete); + if (shouldDelete) { + deleteVirtualObject(vref); + } } // case 1: export, drop local ref, drop export @@ -701,7 +706,7 @@ test('virtual object gc', t => { t.is(log.shift(), `set vom.es.${tbase}/1 r`); t.deepEqual(log, []); // drop local ref -- should not delete because exported - pretendGC(`${tbase}/1`); + pretendGC(`${tbase}/1`, false); t.is(log.shift(), `get vom.rc.${tbase}/1 => undefined`); t.is(log.shift(), `get vom.es.${tbase}/1 => r`); t.deepEqual(log, []); @@ -728,7 +733,9 @@ test('virtual object gc', t => { t.is(log.shift(), `set vom.es.${tbase}/1 s`); t.is(log.shift(), `get vom.rc.${tbase}/1 => undefined`); t.deepEqual(log, []); - pretendGC(`${tbase}/1`); + pretendGC(`${tbase}/1`, true); + t.is(log.shift(), `get vom.rc.${tbase}/1 => undefined`); + t.is(log.shift(), `get vom.es.${tbase}/1 => s`); t.is(log.shift(), `get vom.rc.${tbase}/1 => undefined`); t.is(log.shift(), `get vom.es.${tbase}/1 => s`); t.is(log.shift(), `get vom.${tbase}/1 => ${thingVal(0, 'thing #1', 0)}`); @@ -788,7 +795,9 @@ test('virtual object gc', t => { ]); // drop local ref -- should delete - pretendGC(`${tbase}/2`); + pretendGC(`${tbase}/2`, true); + t.is(log.shift(), `get vom.rc.${tbase}/2 => undefined`); + t.is(log.shift(), `get vom.es.${tbase}/2 => s`); t.is(log.shift(), `get vom.rc.${tbase}/2 => undefined`); t.is(log.shift(), `get vom.es.${tbase}/2 => s`); t.is(log.shift(), `get vom.${tbase}/2 => ${thingVal(0, 'thing #2', 0)}`); @@ -816,7 +825,9 @@ test('virtual object gc', t => { // case 3: drop local ref with no prior export // drop local ref -- should delete - pretendGC(`${tbase}/3`); + pretendGC(`${tbase}/3`, true); + t.is(log.shift(), `get vom.rc.${tbase}/3 => undefined`); + t.is(log.shift(), `get vom.es.${tbase}/3 => undefined`); t.is(log.shift(), `get vom.rc.${tbase}/3 => undefined`); t.is(log.shift(), `get vom.es.${tbase}/3 => undefined`); t.is(log.shift(), `get vom.${tbase}/3 => ${thingVal(0, 'thing #3', 0)}`); @@ -867,7 +878,7 @@ test('virtual object gc', t => { t.is(log.shift(), `set vom.es.${tbase}/4 r`); t.deepEqual(log, []); // drop local ref -- should not delete because ref'd virtually AND exported - pretendGC(`${tbase}/4`); + pretendGC(`${tbase}/4`, false); t.is(log.shift(), `get vom.rc.${tbase}/4 => 1`); t.is(log.shift(), `get vom.es.${tbase}/4 => r`); t.deepEqual(log, []); @@ -907,7 +918,7 @@ test('virtual object gc', t => { ['vom.vkind.11', '{"kindID":"11","tag":"ref"}'], ]); // drop local ref -- should not delete because ref'd virtually AND exported - pretendGC(`${tbase}/5`); + pretendGC(`${tbase}/5`, false); t.is(log.shift(), `get vom.rc.${tbase}/5 => 1`); t.is(log.shift(), `get vom.es.${tbase}/5 => r`); t.deepEqual(log, []); @@ -943,7 +954,7 @@ test('virtual object gc', t => { ['vom.vkind.11', '{"kindID":"11","tag":"ref"}'], ]); // drop local ref -- should not delete because ref'd virtually - pretendGC(`${tbase}/6`); + pretendGC(`${tbase}/6`, false); t.is(log.shift(), `get vom.rc.${tbase}/6 => 1`); t.is(log.shift(), `get vom.es.${tbase}/6 => undefined`); t.deepEqual(log, []);