diff --git a/packages/SwingSet/test/abandon-export.test.js b/packages/SwingSet/test/abandon-export.test.js index 55d3762e22e7..2ee7d12c3860 100644 --- a/packages/SwingSet/test/abandon-export.test.js +++ b/packages/SwingSet/test/abandon-export.test.js @@ -5,15 +5,19 @@ import { test } from '../tools/prepare-test-env-ava.js'; import buildKernel from '../src/kernel/index.js'; import { initializeKernel } from '../src/controller/initializeKernel.js'; import { extractMethod } from '../src/lib/kdebug.js'; +import makeKernelKeeper from '../src/kernel/state/kernelKeeper.js'; import { makeKernelEndowments, buildDispatch } from './util.js'; import { kser, kunser, kslot } from '@agoric/kmarshal'; const makeKernel = async () => { const endowments = makeKernelEndowments(); - const { kvStore } = endowments.kernelStorage; - await initializeKernel({}, endowments.kernelStorage); + const { kernelStorage } = endowments; + const { kvStore } = kernelStorage; + await initializeKernel({}, kernelStorage); const kernel = buildKernel(endowments, {}, {}); - return { kernel, kvStore }; + const kernelKeeper = makeKernelKeeper(kernelStorage, null); + kernelKeeper.loadStats(); + return { kernel, kvStore, kernelKeeper }; }; /** @@ -43,7 +47,7 @@ const assertSingleEntryLog = (t, log, type, details = {}) => { return entry; }; -const makeTestVat = async (t, kernel, name) => { +const makeTestVat = async (t, kernel, name, kernelKeeper) => { const { log, dispatch } = buildDispatch(); let syscall; const setup = providedSyscall => { @@ -56,19 +60,29 @@ const makeTestVat = async (t, kernel, name) => { const kref = kernel.getRootObject(vatID); kernel.pinObject(kref); const flushDeliveries = async () => { - // Make a dummy delivery so the kernel will call processRefcounts(). - // This isn't normally needed, but we're directly making syscalls - // outside of a delivery. - // If that ever stops working, we can update `makeTestVat` to return - // functions supporting more true-to-production vat behavior like - // ```js - // enqueueSyscall('send', target, kser([...args]), resultVPID); - // enqueueSyscall('subscribe', resultVPID); - // flushSyscalls(); // issues queued syscalls inside a dummy delivery - // ``` - kernel.queueToKref(kref, 'flush', []); + // This test uses a really lazy+sketchy approach to triggering + // syscalls: it just grabs the vat's `syscall` object and invokes + // it, from outside a crank. We're exercising refcount-changing + // syscalls, but the kernel only runs processRefcounts() after a + // real crank (i.e. inside processDeliveryMessage and + // processAcceptanceMessage). We need to trigger this, but we want + // to avoid making a real delivery to one of our vats, so their + // delivery logs won't be cluttered with our dummy + // processRefcounts-triggering activity + // + // The previous hack was to do a queueToKref(), and then try to + // remove the resulting delivery from the stream. + // + // The new hack is to inject a `{ type: 'negated-gc-action' }` + // onto the run-queue, which is handled by processDeliveryMessage + // but doesn't actually delivery anything. + // + // A safer approach would be to define the vat's dispatch() to + // listen for some messages that trigger each of the syscalls we + // want to invoke + // + kernelKeeper.addToRunQueue({ type: 'negated-gc-action' }); await kernel.run(); - assertFirstLogEntry(t, log, 'deliver', { method: 'flush' }); }; return { vatID, kref, dispatch, log, syscall, flushDeliveries }; }; @@ -78,7 +92,7 @@ async function doAbandon(t, reachable) { // vatB abandons it // vatA should retain the object // sending to the abandoned object should get an error - const { kernel, kvStore } = await makeKernel(); + const { kernel, kvStore, kernelKeeper } = await makeKernel(); await kernel.start(); const { @@ -87,13 +101,14 @@ async function doAbandon(t, reachable) { log: logA, syscall: syscallA, flushDeliveries: flushDeliveriesA, - } = await makeTestVat(t, kernel, 'vatA'); + } = await makeTestVat(t, kernel, 'vatA', kernelKeeper); const { vatID: vatB, kref: bobKref, log: logB, syscall: syscallB, - } = await makeTestVat(t, kernel, 'vatB'); + flushDeliveries: flushDeliveriesB, + } = await makeTestVat(t, kernel, 'vatB', kernelKeeper); await kernel.run(); // introduce B to A, so it can send 'holdThis' later @@ -142,16 +157,24 @@ async function doAbandon(t, reachable) { // now have vatB abandon the export syscallB.abandonExports([targetForBob]); - await flushDeliveriesA(); - - // no GC messages for either vat - t.deepEqual(logA, []); - t.deepEqual(logB, []); - + await flushDeliveriesB(); targetOwner = kvStore.get(`${targetKref}.owner`); targetRefCount = kvStore.get(`${targetKref}.refCount`); - t.is(targetOwner, undefined); - t.is(targetRefCount, expectedRefCount); // unchanged + + if (reachable) { + // no GC messages for either vat + t.deepEqual(logA, []); + t.deepEqual(logB, []); + t.is(targetOwner, undefined); + t.is(targetRefCount, expectedRefCount); // unchanged + } else { + // 'target' was orphaned and unreachable, kernel will delete it, + // so A will get a retireImports now + assertSingleEntryLog(t, logA, 'retireImports', { vrefs: [targetForAlice] }); + t.deepEqual(logB, []); + t.is(targetOwner, undefined); + t.is(targetRefCount, undefined); + } if (reachable) { // vatA can send a message, but it will reject @@ -180,17 +203,11 @@ async function doAbandon(t, reachable) { await flushDeliveriesA(); // vatB should not get a dispatch.dropImports t.deepEqual(logB, []); - // the object still exists, now only recognizable - targetRefCount = kvStore.get(`${targetKref}.refCount`); - expectedRefCount = '0,1'; - t.is(targetRefCount, expectedRefCount); // merely recognizable + // the kernel automatically retires the orphaned + // now-merely-recognizable object + t.deepEqual(logA, [{ type: 'retireImports', vrefs: [targetForAlice] }]); + // which deletes it } - - // now vatA retires the object too - syscallA.retireImports([targetForAlice]); - await flushDeliveriesA(); - // vatB should not get a dispatch.retireImports - t.deepEqual(logB, []); // the object no longer exists targetRefCount = kvStore.get(`${targetKref}.refCount`); expectedRefCount = undefined; diff --git a/packages/SwingSet/test/gc-dead-vat/bootstrap.js b/packages/SwingSet/test/gc-dead-vat/bootstrap.js index 50668b2902b0..7bf0c47039b8 100644 --- a/packages/SwingSet/test/gc-dead-vat/bootstrap.js +++ b/packages/SwingSet/test/gc-dead-vat/bootstrap.js @@ -1,5 +1,6 @@ import { Far, E } from '@endo/far'; import { makePromiseKit } from '@endo/promise-kit'; +import { makeScalarBigWeakSetStore } from '@agoric/vat-data'; async function sendExport(doomedRoot) { const exportToDoomed = Far('exportToDoomed', {}); @@ -11,6 +12,7 @@ export function buildRootObject() { let doomedRoot; const pin = []; const pk1 = makePromiseKit(); + const wh = makeScalarBigWeakSetStore('weak-holder'); return Far('root', { async bootstrap(vats, devices) { const vatMaker = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin); @@ -19,6 +21,8 @@ export function buildRootObject() { await sendExport(doomedRoot); const doomedExport1Presence = await E(doomedRoot).getDoomedExport1(); pin.push(doomedExport1Presence); + const doomedExport3Presence = await E(doomedRoot).getDoomedExport3(); + wh.add(doomedExport3Presence); }, async stash() { // Give vat-doomed a target that doesn't resolve one() right away. diff --git a/packages/SwingSet/test/gc-dead-vat/vat-doomed.js b/packages/SwingSet/test/gc-dead-vat/vat-doomed.js index f3ad636b7c0d..19acca8758b8 100644 --- a/packages/SwingSet/test/gc-dead-vat/vat-doomed.js +++ b/packages/SwingSet/test/gc-dead-vat/vat-doomed.js @@ -4,6 +4,7 @@ export function buildRootObject(vatPowers) { const pin = []; const doomedExport1 = Far('doomedExport1', {}); const doomedExport2 = Far('doomedExport2', {}); + const doomedExport3 = Far('doomedExport3', {}); return Far('root', { accept(exportToDoomedPresence) { pin.push(exportToDoomedPresence); @@ -14,6 +15,9 @@ export function buildRootObject(vatPowers) { stashDoomedExport2(target) { E(E(target).one()).neverCalled(doomedExport2); }, + getDoomedExport3() { + return doomedExport3; + }, terminate() { vatPowers.exitVat('completion'); }, diff --git a/packages/SwingSet/test/gc-kernel-orphan.test.js b/packages/SwingSet/test/gc-kernel-orphan.test.js new file mode 100644 index 000000000000..b49deacd3d89 --- /dev/null +++ b/packages/SwingSet/test/gc-kernel-orphan.test.js @@ -0,0 +1,342 @@ +// @ts-nocheck +/* global WeakRef, FinalizationRegistry */ + +import anylogger from 'anylogger'; +// eslint-disable-next-line import/order +import { test } from '../tools/prepare-test-env-ava.js'; + +import { assert } from '@agoric/assert'; +import { kser, kunser, kslot } from '@agoric/kmarshal'; +import { initSwingStore } from '@agoric/swing-store'; +import { waitUntilQuiescent } from '@agoric/internal/src/lib-nodejs/waitUntilQuiescent.js'; +import buildKernel from '../src/kernel/index.js'; +import { initializeKernel } from '../src/controller/initializeKernel.js'; + +function makeConsole(tag) { + const log = anylogger(tag); + const cons = {}; + for (const level of ['debug', 'log', 'info', 'warn', 'error']) { + cons[level] = log[level]; + } + return harden(cons); +} + +function writeSlogObject(o) { + function bigintReplacer(_, arg) { + if (typeof arg === 'bigint') { + return Number(arg); + } + return arg; + } + 0 && console.log(JSON.stringify(o, bigintReplacer)); +} + +function makeEndowments() { + return { + waitUntilQuiescent, + kernelStorage: initSwingStore().kernelStorage, + runEndOfCrank: () => {}, + makeConsole, + writeSlogObject, + WeakRef, + FinalizationRegistry, + }; +} + +async function makeKernel() { + const endowments = makeEndowments(); + const { kvStore } = endowments.kernelStorage; + await initializeKernel({}, endowments.kernelStorage); + const kernel = buildKernel(endowments, {}, {}); + return { kernel, kvStore }; +} + +const orphanTest = test.macro(async (t, cause, when, amyState) => { + assert(['reachable', 'recognizable', 'none'].includes(amyState)); + assert(['abandon', 'terminate'].includes(cause)); + // There is a third case (vatA gets upgraded, and the object wasn't + // durable), but we can't upgrade vats created by createTestVat(). + assert(['before', 'after'].includes(when)); + const retireImmediately = false; + const { kernel, kvStore } = await makeKernel(); + await kernel.start(); + + const vrefs = {}; // track vrefs within vats + + // Our two root objects (alice and bob) are pinned so they don't disappear + // while the test is talking to them. So we make alice introduce "amy" as a + // new object that's doomed to be collected. Bob first drops amy, then + // retires her, provoking first a dropExports then a retireImports on + // alice. + + vrefs.aliceForA = 'o+100'; + vrefs.amyForA = 'o+101'; + function setupA(syscall, _state, _helpers, _vatPowers) { + let exportingAmy = false; + function dispatch(vd) { + if (vd[0] === 'startVat') { + return; // skip startVat + } + let method; + if (vd[0] === 'message') { + const methargs = kunser(vd[2].methargs); + [method] = methargs; + if (method === 'init') { + // initial conditions: bob holds a reference to amy + vrefs.bobForA = vd[2].methargs.slots[0]; + syscall.send( + vrefs.bobForA, + kser(['accept-amy', [kslot(vrefs.amyForA)]]), + ); + exportingAmy = true; + } + if (method === 'abandon') { + if (exportingAmy) { + syscall.abandonExports([vrefs.amyForA]); + } + } + if (method === 'terminate') { + syscall.exit(false, kser('terminated')); + } + } + if (vd[0] === 'dropExports') { + if (retireImmediately) { + // pretend there are no local strongrefs, and as soon as liveslots + // drops it's claim, the object goes away completely + syscall.retireExports(vd[1]); + } + } + if (vd[0] === 'retireExports') { + exportingAmy = false; + } + } + return dispatch; + } + await kernel.createTestVat('vatA', setupA); + const vatA = kernel.vatNameToID('vatA'); + const alice = kernel.addExport(vatA, vrefs.aliceForA); + + let amyRetiredToB = false; + function setupB(syscall, _state, _helpers, _vatPowers) { + function dispatch(vd) { + if (vd[0] === 'startVat') { + return; // skip startVat + } + let method; + if (vd[0] === 'message') { + [method] = kunser(vd[2].methargs); + if (method === 'accept-amy') { + vrefs.amyForB = vd[2].methargs.slots[0]; + } + if (method === 'drop') { + syscall.dropImports([vrefs.amyForB]); + } + if (method === 'drop and retire') { + syscall.dropImports([vrefs.amyForB]); + syscall.retireImports([vrefs.amyForB]); + } + if (method === 'retire') { + // it would be an error for bob to do this before or without + // dropImports + syscall.retireImports([vrefs.amyForB]); + } + } + if (vd[0] === 'retireImports') { + t.deepEqual(vd[1], [vrefs.amyForB]); + amyRetiredToB = true; + } + } + return dispatch; + } + await kernel.createTestVat('vatB', setupB); + const vatB = kernel.vatNameToID('vatB'); + vrefs.bobForB = 'o+200'; + const bob = kernel.addExport(vatB, vrefs.bobForB); + + // we always start with bob importing an object from alice + kernel.queueToKref(alice, 'init', [kslot(bob)], 'none'); + await kernel.run(); + + const amy = kvStore.get(`${vatA}.c.${vrefs.amyForA}`); + t.is(amy, 'ko22'); // probably + + if (when === 'before') { + // the object is abandoned before vatB drops anything + if (cause === 'abandon') { + kernel.queueToKref(alice, 'abandon', [], 'none'); + } else if (cause === 'terminate') { + kernel.queueToKref(alice, 'terminate', [], 'none'); + } + await kernel.run(); + t.is(kvStore.get(`${amy}.owner`), undefined); + } + + t.is(kvStore.get(`${amy}.refCount`), '1,1'); + t.is(kvStore.get(`${vatB}.c.${amy}`), `R ${vrefs.amyForB}`); + + // vatB now drops/retires/neither the "amy" object + + if (amyState === 'reachable') { + // no change + } else if (amyState === 'recognizable') { + kernel.queueToKref(bob, 'drop', [], 'none'); + await kernel.run(); + if (when === 'before') { + // dropping an abandoned object should also retire it + t.true(amyRetiredToB); // fixed by #7212 + t.is(kvStore.get(`${vatB}.c.${amy}`), undefined); + t.is(kvStore.get(`${amy}.refCount`), undefined); + } else { + // dropping a living object should merely drop it + t.is(kvStore.get(`${vatB}.c.${amy}`), `_ ${vrefs.amyForB}`); + t.is(kvStore.get(`${amy}.refCount`), '0,1'); + } + } else if (amyState === 'none') { + kernel.queueToKref(bob, 'drop and retire', [], 'none'); + await kernel.run(); + t.is(kvStore.get(`${vatB}.c.${amy}`), undefined); + t.is(kvStore.get(`${amy}.refCount`), undefined); + } + + if (when === 'after') { + // the object is abandoned *after* vatB drops anything + if (cause === 'abandon') { + kernel.queueToKref(alice, 'abandon', [], 'none'); + } else if (cause === 'terminate') { + kernel.queueToKref(alice, 'terminate', [], 'none'); + } + await kernel.run(); + } + + t.is(kvStore.get(`${amy}.owner`), undefined); + + if (amyState === 'reachable') { + // amy should remain defined and reachable, just lacking an owner + t.is(kvStore.get(`${vatB}.c.${amy}`), `R ${vrefs.amyForB}`); + t.is(kvStore.get(`${amy}.refCount`), '1,1'); + } else if (amyState === 'recognizable') { + // an unreachable koid should be retired upon abandonment + t.true(amyRetiredToB); // fixed by #7212 + t.is(kvStore.get(`${vatB}.c.${amy}`), undefined); + t.is(kvStore.get(`${amy}.refCount`), undefined); + } else if (amyState === 'none') { + // no change + } +}); + +for (const cause of ['abandon', 'terminate']) { + for (const when of ['before', 'after']) { + for (const amyState of ['reachable', 'recognizable', 'none']) { + test( + `orphan test ${cause}-${when}-${amyState}`, + orphanTest, + cause, + when, + amyState, + ); + } + } +} + +// exercise a failure case I saw in the zoe tests (zoe - +// secondPriceAuction - valid inputs): +// * vatB is exporting an object ko120 to vatA +// * vatA does E(ko120).wake(), then drops the reference +// (vatA can still recognize ko120) +// * vatA gets a BOYD, does syscall.dropImport(ko120) +// * ko120.refCount = 1,2 (0,1 from vatA, 1,1 from the run-queue wake()) +// * wake() is delivered to vatB +// * removing it from the run-queue drops refCount to 0,1 +// * and adds ko120 to maybeFreeKrefs +// * wake() provokes vatB to syscall.exit +// * post-crank terminateVat() orphans ko120, then retires it +// * deleting both .owner and .refCount +// * post-er-crank processRefcounts sees ko120 in maybeFreeKrefs +// * tries to look up .refCount, fails, panics + +// the fix was to change kernelKeeper.getRefCounts to handle a missing +// koNN.refCounts by just returning 0,0 + +test('termination plus maybeFreeKrefs', async t => { + const { kernel, kvStore } = await makeKernel(); + await kernel.start(); + + const vrefs = {}; // track vrefs within vats + + // vatB exports an object to vatA, vatA sends it a message and drops + // it (but doesn't retire it), vatB will self-terminate upon + // receiving that message, creating two places that try to retire it + + // Our two root objects (alice and bob) are pinned so they don't + // disappear while the test is talking to them, so vatB exports + // "billy". + + vrefs.aliceForA = 'o+100'; + vrefs.bobForB = 'o+200'; + vrefs.billyForB = 'o+201'; + let billyKref; + + let vatA; + function setupA(syscall, _state, _helpers, _vatPowers) { + function dispatch(vd) { + if (vd[0] === 'startVat') { + return; // skip startVat + } + // console.log(`deliverA:`, JSON.stringify(vd)); + if (vd[0] === 'message') { + const methargs = kunser(vd[2].methargs); + const [method] = methargs; + if (method === 'call-billy') { + t.is(vd[2].methargs.slots.length, 1); + vrefs.billyForA = vd[2].methargs.slots[0]; + t.is(vrefs.billyForA, 'o-50'); // probably + billyKref = kvStore.get(`${vatA}.c.${vrefs.billyForA}`); + syscall.send( + vrefs.billyForA, + kser(['terminate', [kslot(vrefs.billyForA, 'billy-A')]]), + ); + syscall.dropImports([vrefs.billyForA]); + } + } + } + return dispatch; + } + await kernel.createTestVat('vatA', setupA); + vatA = kernel.vatNameToID('vatA'); + const alice = kernel.addExport(vatA, vrefs.aliceForA); + + function setupB(syscall, _state, _helpers, _vatPowers) { + function dispatch(vd) { + if (vd[0] === 'startVat') { + return; // skip startVat + } + // console.log(`deliverB:`, JSON.stringify(vd)); + if (vd[0] === 'message') { + const [method] = kunser(vd[2].methargs); + if (method === 'init') { + vrefs.aliceForB = vd[2].methargs.slots[0]; + syscall.send( + vrefs.aliceForB, + kser(['call-billy', [kslot(vrefs.billyForB, 'billy-B')]]), + ); + } + if (method === 'terminate') { + t.is(vd[2].methargs.slots.length, 1); + assert.equal(vd[2].methargs.slots[0], vrefs.billyForB); + syscall.exit(false, kser('reason')); + } + } + } + return dispatch; + } + await kernel.createTestVat('vatB', setupB); + const vatB = kernel.vatNameToID('vatB'); + const bob = kernel.addExport(vatB, vrefs.bobForB); + + // this triggers everything, the bug was a kernel crash + kernel.queueToKref(bob, 'init', [kslot(alice, 'alice')], 'none'); + await kernel.run(); + + t.is(kvStore.get(`${billyKref}.owner`), undefined); + t.is(kvStore.get(`${billyKref}.refCounts`), undefined); +}); diff --git a/packages/SwingSet/test/gc-kernel.test.js b/packages/SwingSet/test/gc-kernel.test.js index 742e586bddae..91b3a0bc26ab 100644 --- a/packages/SwingSet/test/gc-kernel.test.js +++ b/packages/SwingSet/test/gc-kernel.test.js @@ -1091,12 +1091,15 @@ test('terminated vat', async t => { // we'll watch for this to be deleted when the vat is terminated // console.log(`pinKref`, pinKref); - // find the highest export: doomedExport1 / doomedExport1Presence + // find the two highest exports: doomedExport[13] / doomedExport[13]Presence let exports = Object.keys(vrefs).filter(vref => vref.startsWith('o+')); sortVrefs(exports); - const doomedExport1Vref = exports[exports.length - 1]; + const doomedExport1Vref = exports[exports.length - 2]; + const doomedExport3Vref = exports[exports.length - 1]; t.is(doomedExport1Vref, 'o+10'); // arbitrary + t.is(doomedExport3Vref, 'o+11'); // arbitrary const doomedExport1Kref = vrefs[doomedExport1Vref]; + const doomedExport3Kref = vrefs[doomedExport3Vref]; // this should also be deleted // console.log(`doomedExport1Kref`, doomedExport1Kref); @@ -1105,6 +1108,8 @@ test('terminated vat', async t => { t.is(owners[pinKref], bootstrapVat); t.deepEqual(refcounts[doomedExport1Kref], [1, 1]); t.is(owners[doomedExport1Kref], doomedVat); + t.deepEqual(refcounts[doomedExport3Kref], [0, 1]); + t.is(owners[doomedExport3Kref], doomedVat); // Tell bootstrap to give a promise to the doomed vat. The doomed vat will // send a second export in a message to this promise, so the only @@ -1117,7 +1122,7 @@ test('terminated vat', async t => { exports = Object.keys(vrefs).filter(vref => vref.startsWith('o+')); sortVrefs(exports); const doomedExport2Vref = exports[exports.length - 1]; - t.is(doomedExport2Vref, 'o+11'); // arbitrary + t.is(doomedExport2Vref, 'o+12'); // arbitrary const doomedExport2Kref = vrefs[doomedExport2Vref]; [refcounts, owners] = getRefCountsAndOwners(); t.deepEqual(refcounts[doomedExport2Kref], [1, 1]); // from promise queue @@ -1137,6 +1142,9 @@ test('terminated vat', async t => { t.deepEqual(refcounts[doomedExport2Kref], [1, 1]); t.falsy(owners[doomedExport1Kref]); t.falsy(owners[doomedExport2Kref]); + // but the merely-recognizable export should be deleted + t.falsy(owners[doomedExport3Kref]); + t.deepEqual(refcounts[doomedExport3Kref], undefined); // send a message to the orphan, to wiggle refcounts some more const r = c.queueToVatRoot('bootstrap', 'callOrphan', [], 'panic'); diff --git a/packages/SwingSet/test/upgrade/upgrade.test.js b/packages/SwingSet/test/upgrade/upgrade.test.js index 77b7c781b877..aca1efbbd892 100644 --- a/packages/SwingSet/test/upgrade/upgrade.test.js +++ b/packages/SwingSet/test/upgrade/upgrade.test.js @@ -707,20 +707,11 @@ test('non-durable exports are abandoned by upgrade of non-liveslots vat', async '1,1', 'strong observation of abandoned object retains ref counts', ); - // TODO: Expect and verify observer receipt of dispatch.retireExports - // and corresponding removal of weakKref ref counts. - // https://github.com/Agoric/agoric-sdk/issues/7212 t.is( kvStore.get(`${weakKref}.refCount`), - '0,1', - 'weak observation of abandoned object retains ref counts before #7212', + undefined, + 'unreachable abandoned object is forgotten', ); - // t.is( - // kvStore.get(`${weakKref}.refCount`), - // undefined, - // 'unreachable abandoned object is forgotten', - // ); - // const observerLog = await messageToVat('observer', 'getDispatchLog'); }); // No longer valid as of removing stopVat per #6650