From 6309e5fcc60503610381ea1cb4b906beb8e8e4fc Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Tue, 16 Mar 2021 17:42:28 -0700 Subject: [PATCH] feat(swingset): add WeakRef tracking to liveslots Liveslots now uses WeakRefs and a FinalizationRegistry to track the state of each import: UNKNOWN -> REACHABLE -> UNREACHABLE -> COLLECTED -> FINALIZED -> UNKNOWN. Reintroduction can move it from UNREACHABLE/COLLECTED/FINALIZED back to REACHABLE at any time. Liveslots maintains a local `deadSet` that contains all the vrefs which are in the FINALIZED state. They will remain in that state (and in `deadSet`) until a later change which uses `syscall.dropImports` to inform the kernel, and remove them from `deadSet`. We remove imported objects from the deadSet if/when they are re-introduced. Promises are retained until resolved+retired, even if userspace somehow drops all references to them. We might do better in the future, but the story is a lot more complicated than it is for Presences. Exported Remotables are still retained indefinitely. A later change (#2664) will wire `dropExports()` up to drop them. We only register finalizers for imported objects: not imported promises, and not exports of any flavor. Liveslots is not yet calling syscall.dropImports, but by mocking WeakRef and FinalizationRegistry, we can test to make sure it updates the deadSet correctly. refs #2660 --- packages/SwingSet/src/kernel/liveSlots.js | 173 +++++++++++++--- packages/SwingSet/test/test-liveslots.js | 239 ++++++++++++++++++++++ 2 files changed, 389 insertions(+), 23 deletions(-) diff --git a/packages/SwingSet/src/kernel/liveSlots.js b/packages/SwingSet/src/kernel/liveSlots.js index 7642d91beb8..6f192f40ba2 100644 --- a/packages/SwingSet/src/kernel/liveSlots.js +++ b/packages/SwingSet/src/kernel/liveSlots.js @@ -30,6 +30,7 @@ const DEFAULT_VIRTUAL_OBJECT_CACHE_SIZE = 3; // XXX ridiculously small value to * @param {boolean} enableDisavow * @param {*} vatPowers * @param {*} vatParameters + * @param {*} gcTools { WeakRef, FinalizationRegistry, vatDecref } * @param {Console} console * @returns {*} { vatGlobals, dispatch, setBuildRootObject } * @@ -46,8 +47,10 @@ function build( enableDisavow, vatPowers, vatParameters, + gcTools, console, ) { + const { WeakRef, FinalizationRegistry } = gcTools; const enableLSDebug = false; function lsdebug(...args) { if (enableLSDebug) { @@ -60,23 +63,119 @@ function build( const outstandingProxies = new WeakSet(); /** - * Map in-vat object references -> vat slot strings. + * Translation and tracking tables to map in-vat object/promise references + * to/from vat-format slot strings. * - * Uses a weak map so that vat objects can (in princple) be GC'd. Note that - * they currently can't actually be GC'd because the slotToVal table keeps - * them alive, but that will have to be addressed by a different mechanism. + * Exports: pass-by-presence objects (Remotables) in the vat are exported + * as o+NN slots, as are "virtual object" exports. Promises are exported as + * p+NN slots. We retain a strong reference to all exports via the + * `exported` Set until (TODO) the kernel tells us all external references + * have been dropped via dispatch.dropExports, or by some unilateral + * revoke-object operation executed by our user-level code. + * + * Imports: o-NN slots are represented as a Presence. p-NN slots are + * represented as an imported Promise, with the resolver held in an + * additional table (importedPromisesByPromiseID) to handle a future + * incoming resolution message. We retain a weak reference to the Presence, + * and use a FinalizationRegistry to learn when the vat has dropped it, so + * we can notify the kernel. We retain strong references to Promises, for + * now, via the `exported` Set (whose name is not entirely accurate) until + * we figure out a better GC approach. When an import is added, the + * finalizer is added to `droppedRegistry`. + * + * slotToVal is a Map whose keys are slots (strings) and the values are + * WeakRefs. If the entry is present but wr.deref()===undefined (the + * weakref is dead), treat that as if the entry was not present. The same + * slotToVal table is used for both imports and returning exports. The + * subset of those which need to be held strongly (exported objects and + * promises, imported promises) are kept alive by `exported`. + * + * valToSlot is a WeakMap whose keys are Remotable/Presence/Promise + * objects, and the keys are (string) slot identifiers. This is used + * for both exports and returned imports. + * + * We use two weak maps plus the strong `exported` set, because it seems + * simpler than using four separate maps (import-vs-export times + * strong-vs-weak). */ - const valToSlot = new WeakMap(); - /** Map vat slot strings -> in-vat object references. */ - const slotToVal = new Map(); + const valToSlot = new WeakMap(); // object -> vref + const slotToVal = new Map(); // vref -> WeakRef(object) + const exported = new Set(); // objects + const deadSet = new Set(); // vrefs that are finalized but not yet reported + + /* + Imports are in one of 5 states: UNKNOWN, REACHABLE, UNREACHABLE, + COLLECTED, FINALIZED. Note that there's no actual state machine with those + values, and we can't observe all of the transitions from JavaScript, but + we can describe what operations could cause a transition, and what our + observations allow us to deduce about the state: + + * UKNOWN moves to REACHABLE when a crank introduces a new import + * userspace holds a reference only in REACHABLE + * REACHABLE moves to UNREACHABLE only during a userspace crank + * UNREACHABLE moves to COLLECTED when GC runs, which queues the finalizer + * COLLECTED moves to FINALIZED when a new turn runs the finalizer + * liveslots moves from FINALIZED to UNKNOWN by syscalling dropImports + + convertSlotToVal either imports a vref for the first time, or + re-introduces a previously-seen vref. It transitions from: + + * UNKNOWN to REACHABLE by creating a new Presence + * UNREACHABLE to REACHABLE by re-using the old Presence that userspace + forgot about + * COLLECTED/FINALIZED to REACHABLE by creating a new Presence + + Our tracking tables hold data that depends on the current state: + + * slotToVal holds a WeakRef in [REACHABLE, UNREACHABLE, COLLECTED] + * that WeakRef .deref()s into something in [REACHABLE, UNREACHABLE] + * deadSet holds the vref only in FINALIZED + * re-introduction must ensure the vref is not in the deadSet + + Each state thus has a set of perhaps-measurable properties: + + * UNKNOWN: slotToVal[vref] is missing, vref not in deadSet + * REACHABLE: slotToVal has live weakref, userspace can reach + * UNREACHABLE: slotToVal has live weakref, userspace cannot reach + * COLLECTED: slotToVal[vref] has dead weakref + * FINALIZED: slotToVal[vref] is missing, vref is in deadSet + + Our finalizer callback is queued by the engine's transition from + UNREACHABLE to COLLECTED, but the vref might be re-introduced before the + callback has a chance to run. There might even be multiple copies of the + finalizer callback queued. So the callback must deduce the current state + and only perform cleanup (i.e. delete the slotToVal entry and add the + vref to the deadSet) in the COLLECTED state. + + */ + + function finalizeDroppedImport(vref) { + const wr = slotToVal.get(vref); + // The finalizer for a given Presence might run in any state: + // * COLLECTED: most common. Action: move to FINALIZED + // * REACHABLE/UNREACHABLE: after re-introduction. Action: ignore + // * FINALIZED: after re-introduction and subsequent finalizer invocation + // (second finalizer executed for the same vref). Action: be idempotent + // * UNKNOWN: after re-introduction, multiple finalizer invocation, + // and post-crank cleanup does dropImports and deletes vref from + // deadSet. Action: ignore + + if (wr && !wr.deref()) { + // we're in the COLLECTED state, or FINALIZED after a re-introduction + deadSet.add(vref); + slotToVal.delete(vref); + // console.log(`-- adding ${vref} to deadSet`); + } + } + const droppedRegistry = new FinalizationRegistry(finalizeDroppedImport); /** Remember disavowed Presences which will kill the vat if you try to talk * to them */ const disavowedPresences = new WeakSet(); const disavowalError = harden(Error(`this Presence has been disavowed`)); - const importedPromisesByPromiseID = new Map(); + const importedPromisesByPromiseID = new Map(); // vpid -> { resolve, reject } let nextExportID = 1; let nextPromiseID = 5; @@ -285,7 +384,9 @@ function build( } parseVatSlot(slot); // assertion valToSlot.set(val, slot); - slotToVal.set(slot, val); + slotToVal.set(slot, new WeakRef(val)); + // we do not use droppedRegistry for exports + exported.add(val); // keep it alive until kernel tells us to release it } return valToSlot.get(val); } @@ -301,7 +402,8 @@ function build( } function convertSlotToVal(slot, iface = undefined) { - let val = slotToVal.get(slot); + const wr = slotToVal.get(slot); + let val = wr && wr.deref(); if (val) { return val; } @@ -341,7 +443,12 @@ function build( } else { assert.fail(X`unrecognized slot type '${type}'`); } - slotToVal.set(slot, val); + slotToVal.set(slot, new WeakRef(val)); + if (type === 'object' || type === 'device') { + // we don't dropImports on promises, to avoid interaction with retire + droppedRegistry.register(val, slot); + deadSet.delete(slot); // might have been FINALIZED before, no longer + } valToSlot.set(val, slot); } return val; @@ -355,7 +462,8 @@ function build( for (const slot of slots) { const { type } = parseVatSlot(slot); if (type === 'promise') { - const p = slotToVal.get(slot); + const wr = slotToVal.get(slot); + const p = wr && wr.deref(); assert(p, X`should have a value for ${slot} but didn't`); const priorResolution = knownResolutions.get(p); if (priorResolution && !doneResolutions.has(slot)) { @@ -434,7 +542,9 @@ function build( // them, we want the user-level code to get back that Promise, not 'p'. valToSlot.set(returnedP, resultVPID); - slotToVal.set(resultVPID, returnedP); + slotToVal.set(resultVPID, new WeakRef(returnedP)); + // we do not use droppedRegistry for promises, even result promises + exported.add(returnedP); // TODO: revisit, can we GC these? when? return p; } @@ -542,8 +652,12 @@ function build( function retirePromiseID(promiseID) { lsdebug(`Retiring ${forVatID}:${promiseID}`); importedPromisesByPromiseID.delete(promiseID); - const p = slotToVal.get(promiseID); - valToSlot.delete(p); + const wr = slotToVal.get(promiseID); + const p = wr && wr.deref(); + if (p) { + valToSlot.delete(p); + exported.delete(p); + } slotToVal.delete(promiseID); } @@ -682,7 +796,9 @@ function build( const rootSlot = makeVatSlot('object', true, BigInt(0)); valToSlot.set(rootObject, rootSlot); - slotToVal.set(rootSlot, rootObject); + slotToVal.set(rootSlot, new WeakRef(rootObject)); + // we do not use droppedRegistry for exports + exported.add(rootObject); } function dispatch(vatDeliveryObject) { @@ -710,7 +826,8 @@ function build( } harden(dispatch); - return harden({ vatGlobals, setBuildRootObject, dispatch, m }); + // we return 'deadSet' for unit tests + return harden({ vatGlobals, setBuildRootObject, dispatch, m, deadSet }); } /** @@ -723,7 +840,7 @@ function build( * @param {*} vatParameters * @param {number} cacheSize Upper bound on virtual object cache size * @param {boolean} enableDisavow - * @param {*} _gcTools + * @param {*} gcTools { WeakRef, FinalizationRegistry } * @param {Console} [liveSlotsConsole] * @returns {*} { vatGlobals, dispatch, setBuildRootObject } * @@ -756,7 +873,7 @@ export function makeLiveSlots( vatParameters = harden({}), cacheSize = DEFAULT_VIRTUAL_OBJECT_CACHE_SIZE, enableDisavow = false, - _gcTools, + gcTools, liveSlotsConsole = console, ) { const allVatPowers = { @@ -770,14 +887,24 @@ export function makeLiveSlots( enableDisavow, allVatPowers, vatParameters, + gcTools, liveSlotsConsole, ); - const { vatGlobals, dispatch, setBuildRootObject } = r; // omit 'm' - return harden({ vatGlobals, dispatch, setBuildRootObject }); + const { vatGlobals, dispatch, setBuildRootObject, deadSet } = r; // omit 'm' + return harden({ vatGlobals, dispatch, setBuildRootObject, deadSet }); } // for tests -export function makeMarshaller(syscall) { - const { m } = build(syscall); +export function makeMarshaller(syscall, gcTools) { + const { m } = build( + syscall, + 'forVatID', + DEFAULT_VIRTUAL_OBJECT_CACHE_SIZE, + false, + {}, + {}, + gcTools, + console, + ); return { m }; } diff --git a/packages/SwingSet/test/test-liveslots.js b/packages/SwingSet/test/test-liveslots.js index 4d27fac5602..3dba1eb4172 100644 --- a/packages/SwingSet/test/test-liveslots.js +++ b/packages/SwingSet/test/test-liveslots.js @@ -5,6 +5,7 @@ import { E } from '@agoric/eventual-send'; import { Far } from '@agoric/marshal'; import { assert, details as X } from '@agoric/assert'; import { waitUntilQuiescent } from '../src/waitUntilQuiescent'; +import { makeLiveSlots } from '../src/kernel/liveSlots'; import { buildSyscall, makeDispatch } from './liveslots-helpers'; import { capargs, @@ -662,3 +663,241 @@ test('dropExports', async t => { dispatch(makeDropExports(ex1)); // for now, all that we care about is that liveslots doesn't crash }); + +// Create a WeakRef/FinalizationRegistry pair that can be manipulated for +// tests. Limitations: +// * only one WeakRef per object +// * no deregister +// * extra debugging properties like FR.countCallbacks and FR.runOneCallback +// * nothing is hardened + +function makeMockGC() { + const weakRefToVal = new Map(); + const valToWeakRef = new Map(); + const allFRs = []; + // eslint-disable-next-line no-unused-vars + function log(...args) { + // console.log(...args); + } + + const mockWeakRefProto = { + deref() { + return weakRefToVal.get(this); + }, + }; + function mockWeakRef(val) { + assert(!valToWeakRef.has(val)); + weakRefToVal.set(this, val); + valToWeakRef.set(val, this); + } + mockWeakRef.prototype = mockWeakRefProto; + + function kill(val) { + log(`kill`, val); + if (valToWeakRef.has(val)) { + log(` killing weakref`); + const wr = valToWeakRef.get(val); + valToWeakRef.delete(val); + weakRefToVal.delete(wr); + } + for (const fr of allFRs) { + if (fr.registry.has(val)) { + log(` pushed on FR queue, context=`, fr.registry.get(val)); + fr.ready.push(val); + } + } + log(` kill done`); + } + + const mockFinalizationRegistryProto = { + register(val, context) { + log(`FR.register(context=${context})`); + this.registry.set(val, context); + }, + countCallbacks() { + log(`countCallbacks:`); + log(` ready:`, this.ready); + log(` registry:`, this.registry); + return this.ready.length; + }, + runOneCallback() { + log(`runOneCallback`); + const val = this.ready.shift(); + log(` val:`, val); + assert(this.registry.has(val)); + const context = this.registry.get(val); + log(` context:`, context); + this.registry.delete(val); + this.callback(context); + }, + }; + + function mockFinalizationRegistry(callback) { + this.registry = new Map(); + this.callback = callback; + this.ready = []; + allFRs.push(this); + } + mockFinalizationRegistry.prototype = mockFinalizationRegistryProto; + + function getAllFRs() { + return allFRs; + } + + return harden({ + WeakRef: mockWeakRef, + FinalizationRegistry: mockFinalizationRegistry, + kill, + getAllFRs, + }); +} + +test('dropImports', async t => { + const { syscall } = buildSyscall(); + const imports = []; + const gcTools = makeMockGC(); + + function build(_vatPowers) { + const root = Far('root', { + hold(imp) { + imports.push(imp); + }, + free() { + gcTools.kill(imports.pop()); + }, + ignore(imp) { + gcTools.kill(imp); + }, + }); + return root; + } + + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, undefined, false, gcTools); + const { setBuildRootObject, dispatch, deadSet } = ls; + setBuildRootObject(build); + const allFRs = gcTools.getAllFRs(); + t.is(allFRs.length, 1); + const FR = allFRs[0]; + + const rootA = 'o+0'; + + // immediate drop should push import to deadSet after finalizer runs + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-1'))); + await waitUntilQuiescent(); + // the immediate gcTools.kill() means that the import should now be in the + // "COLLECTED" state + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + FR.runOneCallback(); // moves to FINALIZED + t.deepEqual(deadSet, new Set(['o-1'])); + deadSet.delete('o-1'); // pretend liveslots did syscall.dropImport + + // separate hold and free should do the same + dispatch(makeMessage(rootA, 'hold', capargsOneSlot('o-2'))); + await waitUntilQuiescent(); + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 0); + dispatch(makeMessage(rootA, 'free', capargs([]))); + await waitUntilQuiescent(); + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + FR.runOneCallback(); // moves to FINALIZED + t.deepEqual(deadSet, new Set(['o-2'])); + deadSet.delete('o-2'); // pretend liveslots did syscall.dropImport + + // re-introduction during COLLECTED should return to REACHABLE + + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-3'))); + await waitUntilQuiescent(); + // now COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + dispatch(makeMessage(rootA, 'hold', capargsOneSlot('o-3'))); + await waitUntilQuiescent(); + // back to REACHABLE + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + FR.runOneCallback(); // stays at REACHABLE + t.deepEqual(deadSet, new Set()); + + dispatch(makeMessage(rootA, 'free', capargs([]))); + await waitUntilQuiescent(); + // now COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + FR.runOneCallback(); // moves to FINALIZED + t.deepEqual(deadSet, new Set(['o-3'])); + deadSet.delete('o-3'); // pretend liveslots did syscall.dropImport + + // multiple queued finalizers are idempotent, remains REACHABLE + + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-4'))); + await waitUntilQuiescent(); + // now COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-4'))); + await waitUntilQuiescent(); + // moves to REACHABLE and then back to COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 2); + + dispatch(makeMessage(rootA, 'hold', capargsOneSlot('o-4'))); + await waitUntilQuiescent(); + // back to REACHABLE + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 2); + + FR.runOneCallback(); // stays at REACHABLE + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + FR.runOneCallback(); // stays at REACHABLE + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 0); + + // multiple queued finalizers are idempotent, remains FINALIZED + + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-5'))); + await waitUntilQuiescent(); + // now COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-5'))); + await waitUntilQuiescent(); + // moves to REACHABLE and then back to COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 2); + + FR.runOneCallback(); // moves to FINALIZED + t.deepEqual(deadSet, new Set(['o-5'])); + t.is(FR.countCallbacks(), 1); + + FR.runOneCallback(); // stays at FINALIZED + t.deepEqual(deadSet, new Set(['o-5'])); + t.is(FR.countCallbacks(), 0); + deadSet.delete('o-5'); // pretend liveslots did syscall.dropImport + + // re-introduction during FINALIZED moves back to REACHABLE + + dispatch(makeMessage(rootA, 'ignore', capargsOneSlot('o-6'))); + await waitUntilQuiescent(); + // moves to REACHABLE and then back to COLLECTED + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 1); + + FR.runOneCallback(); // moves to FINALIZED + t.deepEqual(deadSet, new Set(['o-6'])); + t.is(FR.countCallbacks(), 0); + + dispatch(makeMessage(rootA, 'hold', capargsOneSlot('o-6'))); + await waitUntilQuiescent(); + // back to REACHABLE, removed from deadSet + t.deepEqual(deadSet, new Set()); + t.is(FR.countCallbacks(), 0); +});