Merge pull request #7552 from Agoric/7371-weakref-to-FR-target

fix(liveslots): retain WeakRefs to voAware collections

packages/swingset-liveslots/src/liveslots.js

@@ -650,6 +650,7 @@ function build(
     getSlotForVal,
     requiredValForSlot,
     FinalizationRegistry,
+    WeakRef,
     addToPossiblyDeadSet,
     addToPossiblyRetiredSet,
     relaxDurabilityRules,

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

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,13 +31,25 @@ export function makeVirtualReferenceManager(
   getSlotForVal,
   requiredValForSlot,
   FinalizationRegistry,
+  WeakRef,
   addToPossiblyDeadSet,
   addToPossiblyRetiredSet,
   relaxDurabilityRules,
 ) {
   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) {
+    const wr = new WeakRef(target);
+    droppedCollectionRegistry.register(target, { descriptor, wr });
+  }
 
   /**
    * Check if a virtual object is unreachable via virtual-data references.
@@ -636,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);
   }
 
@@ -666,6 +686,8 @@ export function makeVirtualReferenceManager(
     getReachableRefCount,
     countCollectionsForWeakKey,
 
+    // don't harden() the mock FR, that will break it
+    getDroppedCollectionRegistry: () => droppedCollectionRegistry,
     remotableRefCounts,
     vrefRecognizers,
     kindInfoTable,
@@ -680,7 +702,7 @@ export function makeVirtualReferenceManager(
   }
 
   return harden({
-    droppedCollectionRegistry,
+    registerDroppedCollection,
     isDurable,
     isDurableKind,
     registerKind,

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,

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

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,

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,