From 0aae798c762e865deba9a505497e7af77eb27454 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 28 Apr 2023 23:33:04 -0700 Subject: [PATCH] fix(liveslots): retain WeakRefs to voAware collections A lingering source of GC sensitivity is when a FinalizationRegistry is watching an object that userspace might drop, and that FR callback might perform syscalls, or simply consume more computrons when GC happens than when it does not. While we expect all instances of a consensus kernel to perform GC at the same time, any tolerance we can maintain against divergent GC schedules will help us with upgrade (replaying a transcript with a slightly different version of XS). To help this, in XS, WeakRefs are treated as strong references during "organic" GC, and are only actually weak during the forced GC we do inside bringOutYourDead. We'd like this improvement for objects tracked by FinalizationRegistries too. Liveslots has two FRs, `vreffedObjectRegistry` (whose entries all have WeakRefs in slotToVal, so they're covered), and `droppedCollectionRegistry` (in the VRM). The VRM's `droppedCollectionRegistry` tracks the VO-aware replacements for WeakMap/Set that we impose on userspace, and these do not have vref identities, so they will never appear in slotToVal or valToSlot, so we need to create new WeakRefs to trigger the organic-GC-retention behavior. This commit adds a Map named `droppedCollectionWeakRefs` to the VRM, which maps from a new 'holder' object to a special WeakRef for each (imposed) VO-aware WeakMap/Set that userspace creates. This entry is held until the FR callback is run, which removes it. The holder object is tracked by the context/heldValue value that the FR passes to its callback. The net result is that userspace dropping their VO-aware collection objects should not result in the FinalizationRegistry callback running until a bringOutYourDead delivery, which happens on a fixed (within-consensus) schedule. closes #7371 --- .../src/virtualReferences.js | 18 +++- .../test/test-dropped-collection-weakrefs.js | 85 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 packages/swingset-liveslots/test/test-dropped-collection-weakrefs.js diff --git a/packages/swingset-liveslots/src/virtualReferences.js b/packages/swingset-liveslots/src/virtualReferences.js index 7d07f1bd05c9..d8895e31d75f 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -39,9 +39,19 @@ export function makeVirtualReferenceManager( const droppedCollectionRegistry = new FinalizationRegistry( finalizeDroppedCollection, ); + // Our JS engine is configured to treat WeakRefs as strong during + // organic (non-forced GC), to minimize execution variation. To + // prevent FinalizationRegistry callbacks from varying this way, we + // must maintain WeakRefs to everything therein. This Map maps from + // "holder" objects to a WeakRef around the real target, and the map + // entry is deleted when the FR fires. + const droppedCollectionWeakRefs = new Map(); // holder -> WeakRef(target) function registerDroppedCollection(target, descriptor) { - droppedCollectionRegistry.register(target, descriptor); + const holder = {}; + const wr = new WeakRef(target); + droppedCollectionWeakRefs.set(holder, wr); + droppedCollectionRegistry.register(target, { descriptor, holder }); } /** @@ -643,7 +653,8 @@ export function makeVirtualReferenceManager( } } - function finalizeDroppedCollection(descriptor) { + function finalizeDroppedCollection({ descriptor, holder }) { + droppedCollectionWeakRefs.delete(holder); descriptor.collectionDeleter(descriptor); } @@ -673,6 +684,9 @@ export function makeVirtualReferenceManager( getReachableRefCount, countCollectionsForWeakKey, + droppedCollectionWeakRefs, + // don't harden() the mock FR, that will break it + getDroppedCollectionRegistry: () => droppedCollectionRegistry, remotableRefCounts, vrefRecognizers, kindInfoTable, diff --git a/packages/swingset-liveslots/test/test-dropped-collection-weakrefs.js b/packages/swingset-liveslots/test/test-dropped-collection-weakrefs.js new file mode 100644 index 000000000000..4825941e4de2 --- /dev/null +++ b/packages/swingset-liveslots/test/test-dropped-collection-weakrefs.js @@ -0,0 +1,85 @@ +import test from 'ava'; +import '@endo/init/debug.js'; +import { Far } from '@endo/marshal'; +import { makeLiveSlots } from '../src/liveslots.js'; +import { kser } from './kmarshal.js'; +import { buildSyscall } from './liveslots-helpers.js'; +import { makeStartVat } from './util.js'; +import { makeMockGC } from './mock-gc.js'; + +test('droppedCollectionWeakRefs', async t => { + const { syscall } = buildSyscall(); + const gcTools = makeMockGC(); + let myVOAwareWeakMap; + let myVOAwareWeakSet; + + // In XS, WeakRefs are treated as strong references except for + // forced GC, which reduces our sensitivity to GC timing (which is + // more likely to change under small upgrades of the engine). We'd + // like this improvement for objects tracked by + // FinalizationRegistries too. Liveslots has two FRs, + // `vreffedObjectRegistry` (whose entries all have WeakRefs in + // slotToVal), and `droppedCollectionRegistry` (in the VRM). + // + // `droppedCollectionRegistry` tracks the VO-aware replacements for + // WeakMap/Set that we impose on userspace, and these do not have + // vref identities, so they will never appear in slotToVal or + // valToSlot, so we need to create new WeakRefs to trigger the + // retain-under-organic-GC behavior. The VRM has a Map named + // `droppedCollectionWeakRefs`, which maps from a new 'holder' + // object to a special WeakRef for each (imposed) VO-aware + // WeakMap/Set that userspace creates. This entry is held until the + // FR callback is run, which removes it. + // + // Our test inspects `droppedCollectionWeakRefs` to find a *value* + // that matches the target VO-aware WeakMap/Set instance. + + function buildRootObject(vatPowers) { + const { WeakMap, WeakSet } = vatPowers; + // creating a WeakMap/Set should put it in droppedCollectionWeakRefs + myVOAwareWeakMap = new WeakMap(); + myVOAwareWeakSet = new WeakSet(); + return Far('root', {}); + } + + const makeNS = () => ({ buildRootObject }); + const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, makeNS); + const { dispatch, testHooks } = ls; + await dispatch(makeStartVat(kser())); + + const { droppedCollectionWeakRefs } = testHooks; + const keyForValue = (map, target) => { + const entries = [...map.entries()]; + const matching = entries.filter(([_k, v]) => v === target)[0]; + return matching?.[0]; + }; + + // make sure my keyForValue logic works + const m = new Map(); + m.set(1, 2); + m.set(3, 4); + t.is(keyForValue(m, 2), 1); + t.is(keyForValue(m, 4), 3); + t.is(keyForValue(m, 1), undefined); + t.is(keyForValue(m, 5), undefined); + + const wmWeakRef = gcTools.weakRefFor(myVOAwareWeakMap); + const wmHolder = keyForValue(droppedCollectionWeakRefs, wmWeakRef); + const wsWeakRef = gcTools.weakRefFor(myVOAwareWeakSet); + const wsHolder = keyForValue(droppedCollectionWeakRefs, wsWeakRef); + + t.truthy(wmHolder); + t.truthy(droppedCollectionWeakRefs.has(wmHolder)); // tautology + t.truthy(wsHolder); + t.truthy(droppedCollectionWeakRefs.has(wsHolder)); // tautology + + gcTools.kill(myVOAwareWeakMap); + gcTools.flushAllFRs(); + t.falsy(droppedCollectionWeakRefs.has(wmHolder)); + t.truthy(droppedCollectionWeakRefs.has(wsHolder)); // not dead yet + + gcTools.kill(myVOAwareWeakSet); + gcTools.flushAllFRs(); + t.falsy(droppedCollectionWeakRefs.has(wmHolder)); + t.falsy(droppedCollectionWeakRefs.has(wsHolder)); // dead now +});