From 6b063efc0beeb05a22d76cac9e43bb373b4c6053 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 28 Apr 2023 23:26:19 -0700 Subject: [PATCH 1/4] chore(liveslots): give VRM access to WeakRef --- packages/swingset-liveslots/src/liveslots.js | 1 + .../src/virtualReferences.js | 3 +++ .../virtual-objects/test-cease-recognition.js | 3 ++- .../tools/fakeVirtualSupport.js | 18 +++++++++++++++++- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/swingset-liveslots/src/liveslots.js b/packages/swingset-liveslots/src/liveslots.js index 10f162e91ae..972c086e88e 100644 --- a/packages/swingset-liveslots/src/liveslots.js +++ b/packages/swingset-liveslots/src/liveslots.js @@ -650,6 +650,7 @@ function build( getSlotForVal, requiredValForSlot, FinalizationRegistry, + WeakRef, addToPossiblyDeadSet, addToPossiblyRetiredSet, relaxDurabilityRules, diff --git a/packages/swingset-liveslots/src/virtualReferences.js b/packages/swingset-liveslots/src/virtualReferences.js index bbe4f323414..493fbbad6af 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -17,6 +17,8 @@ import { * converts an object ID (vref) to an object. * @param {*} FinalizationRegistry Powerful JavaScript intrinsic normally denied * by SES + * @param {*} WeakRef Powerful JavaScript intrinsic normally denied + * by SES * @param {*} addToPossiblyDeadSet Function to record objects whose deaths * should be reinvestigated * @param {*} addToPossiblyRetiredSet Function to record dead objects whose @@ -29,6 +31,7 @@ export function makeVirtualReferenceManager( getSlotForVal, requiredValForSlot, FinalizationRegistry, + WeakRef, addToPossiblyDeadSet, addToPossiblyRetiredSet, relaxDurabilityRules, diff --git a/packages/swingset-liveslots/test/virtual-objects/test-cease-recognition.js b/packages/swingset-liveslots/test/virtual-objects/test-cease-recognition.js index 9f956b6f5a4..dffa7973a3f 100644 --- a/packages/swingset-liveslots/test/virtual-objects/test-cease-recognition.js +++ b/packages/swingset-liveslots/test/virtual-objects/test-cease-recognition.js @@ -1,4 +1,4 @@ -/* global FinalizationRegistry */ +/* global FinalizationRegistry WeakRef */ import test from 'ava'; import '@endo/init/debug.js'; @@ -16,6 +16,7 @@ function makeVRM() { getSlotForVal, requiredValForSlot, FinalizationRegistry, + WeakRef, addToPossiblyDeadSet, addToPossiblyRetiredSet, false, diff --git a/packages/swingset-liveslots/tools/fakeVirtualSupport.js b/packages/swingset-liveslots/tools/fakeVirtualSupport.js index c52e682636a..6280ec6c6f5 100644 --- a/packages/swingset-liveslots/tools/fakeVirtualSupport.js +++ b/packages/swingset-liveslots/tools/fakeVirtualSupport.js @@ -1,4 +1,5 @@ /* global WeakRef */ +/* eslint-disable max-classes-per-file */ import { makeMarshal } from '@endo/marshal'; import { assert } from '@agoric/assert'; import { parseVatSlot } from '../src/parseVatSlots.js'; @@ -19,6 +20,18 @@ class FakeFinalizationRegistry { unregister(_unregisterToken) {} } +class FakeWeakRef { + constructor(target) { + this.target = target; + } + + deref() { + return this.target; // strong ref + } +} + +const RealWeakRef = WeakRef; + export function makeFakeLiveSlotsStuff(options = {}) { let vrm; function setVrm(vrmToUse) { @@ -31,6 +44,7 @@ export function makeFakeLiveSlotsStuff(options = {}) { weak = false, log, FinalizationRegistry = FakeFinalizationRegistry, + WeakRef = FakeWeakRef, // VRM uses this addToPossiblyDeadSet = () => {}, addToPossiblyRetiredSet = () => {}, } = options; @@ -174,7 +188,7 @@ export function makeFakeLiveSlotsStuff(options = {}) { } function setValForSlot(slot, val) { - slotToVal.set(slot, weak ? new WeakRef(val) : val); + slotToVal.set(slot, weak ? new RealWeakRef(val) : val); } function convertValToSlot(val) { @@ -255,6 +269,7 @@ export function makeFakeLiveSlotsStuff(options = {}) { marshal, deleteEntry, FinalizationRegistry, + WeakRef, addToPossiblyDeadSet, addToPossiblyRetiredSet, dumpStore, @@ -273,6 +288,7 @@ export function makeFakeVirtualReferenceManager( fakeStuff.getSlotForVal, fakeStuff.getValForSlot, fakeStuff.FinalizationRegistry, + fakeStuff.WeakRef, fakeStuff.addToPossiblyDeadSet, fakeStuff.addToPossiblyRetiredSet, relaxDurabilityRules, From f3576725573b3b55e1da9b9e148241ff430eaa3a Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 28 Apr 2023 23:28:52 -0700 Subject: [PATCH 2/4] chore(liveslots): add weakRefFor() to mock GC tools --- packages/swingset-liveslots/test/mock-gc.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/swingset-liveslots/test/mock-gc.js b/packages/swingset-liveslots/test/mock-gc.js index f6c6d4ae7b2..20b945e2d69 100644 --- a/packages/swingset-liveslots/test/mock-gc.js +++ b/packages/swingset-liveslots/test/mock-gc.js @@ -46,6 +46,10 @@ export function makeMockGC() { log(` kill done`); } + function weakRefFor(val) { + return valToWeakRef.get(val); + } + const mockFinalizationRegistryProto = { register(val, context) { log(`FR.register(context=${context})`); @@ -95,6 +99,7 @@ export function makeMockGC() { WeakRef: mockWeakRef, FinalizationRegistry: mockFinalizationRegistry, kill, + weakRefFor, getAllFRs, flushAllFRs, waitUntilQuiescent, From 3ea1a2c570fe2b7f1a40a60ed4459d02cc70575b Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 28 Apr 2023 23:31:01 -0700 Subject: [PATCH 3/4] chore(liveslots): expose registerDroppedCollection instead of raw FR --- packages/swingset-liveslots/src/virtualObjectManager.js | 4 ++-- packages/swingset-liveslots/src/virtualReferences.js | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/swingset-liveslots/src/virtualObjectManager.js b/packages/swingset-liveslots/src/virtualObjectManager.js index bde553b3a2e..c67a4b06b57 100644 --- a/packages/swingset-liveslots/src/virtualObjectManager.js +++ b/packages/swingset-liveslots/src/virtualObjectManager.js @@ -418,7 +418,7 @@ export const makeVirtualObjectManager = ( actualWeakMaps.set(this, new WeakMap()); const vmap = new Map(); virtualObjectMaps.set(this, vmap); - vrm.droppedCollectionRegistry.register(this, { + vrm.registerDroppedCollection(this, { collectionDeleter: voAwareWeakMapDeleter, vmap, }); @@ -496,7 +496,7 @@ export const makeVirtualObjectManager = ( const vset = new Set(); virtualObjectSets.set(this, vset); - vrm.droppedCollectionRegistry.register(this, { + vrm.registerDroppedCollection(this, { collectionDeleter: voAwareWeakSetDeleter, vset, }); diff --git a/packages/swingset-liveslots/src/virtualReferences.js b/packages/swingset-liveslots/src/virtualReferences.js index 493fbbad6af..7d07f1bd05c 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -40,6 +40,10 @@ export function makeVirtualReferenceManager( finalizeDroppedCollection, ); + function registerDroppedCollection(target, descriptor) { + droppedCollectionRegistry.register(target, descriptor); + } + /** * Check if a virtual object is unreachable via virtual-data references. * @@ -683,7 +687,7 @@ export function makeVirtualReferenceManager( } return harden({ - droppedCollectionRegistry, + registerDroppedCollection, isDurable, isDurableKind, registerKind, From 393572396781afd17691e1366abeba696228a24e Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 28 Apr 2023 23:33:04 -0700 Subject: [PATCH 4/4] 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. These WeakRefs are stored in the FR's "context"/heldValue data, which will be passed to the callback when the VO-aware collection is dropped, collected, and finalized. The WeakRef will be kept alive through that entry until the finalizer runs. 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 | 19 +++++- .../test/test-dropped-collection-weakrefs.js | 61 +++++++++++++++++++ 2 files changed, 78 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 7d07f1bd05c..b81561a11e1 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -39,9 +39,16 @@ 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. The WeakRef is + // retained by the FR "heldValue" context record, next to the + // descriptor, and is thus released when the FR fires. function registerDroppedCollection(target, descriptor) { - droppedCollectionRegistry.register(target, descriptor); + const wr = new WeakRef(target); + droppedCollectionRegistry.register(target, { descriptor, wr }); } /** @@ -643,7 +650,13 @@ export function makeVirtualReferenceManager( } } - function finalizeDroppedCollection(descriptor) { + function finalizeDroppedCollection({ descriptor }) { + // the 'wr' WeakRef will be dropped about now + // + // note: technically, the engine is allowed to inspect this + // callback, observe that 'wr' is not extracted, and then not + // retain it in the first place (back in + // registerDroppedCollection), but no engine goes that far descriptor.collectionDeleter(descriptor); } @@ -673,6 +686,8 @@ export function makeVirtualReferenceManager( getReachableRefCount, countCollectionsForWeakKey, + // 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 00000000000..a562237ca07 --- /dev/null +++ b/packages/swingset-liveslots/test/test-dropped-collection-weakrefs.js @@ -0,0 +1,61 @@ +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. Those WeakRefs are held in the + // FR context/"heldValue" until it fires. + + 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 wmWeakRef = gcTools.weakRefFor(myVOAwareWeakMap); + const wsWeakRef = gcTools.weakRefFor(myVOAwareWeakSet); + + // we snoop inside our mock FinalizationRegistry to get the context + const fr = testHooks.getDroppedCollectionRegistry(); + t.is(fr.registry.get(myVOAwareWeakMap).wr, wmWeakRef); + t.is(fr.registry.get(myVOAwareWeakSet).wr, wsWeakRef); + + gcTools.kill(myVOAwareWeakMap); + gcTools.flushAllFRs(); + t.falsy(fr.registry.has(myVOAwareWeakMap)); + t.truthy(fr.registry.has(myVOAwareWeakSet)); // not dead yet + + gcTools.kill(myVOAwareWeakSet); + gcTools.flushAllFRs(); + t.falsy(fr.registry.has(myVOAwareWeakMap)); + t.falsy(fr.registry.has(myVOAwareWeakSet)); // dead now +});