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

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,

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
+});