Merge pull request #2996 from Agoric/virtual-object-single-representa…

…tive

Use WeakRefs to ensure virtual objects have at most one representative.
Closes #1968.

packages/SwingSet/docs/virtual-objects.md

@@ -94,9 +94,7 @@ Additional important details:
 
 - A virtual object's state may include references to other virtual objects. The latter objects will be persisted separately and only deserialized as needed, so "swapping in" a virtual object that references other virtual objects does not entail swapping in the entire associated object graph.
 
-- The in-memory manifestations of a virtual object (which we call its "representatives") are not necessarily (or even usually) `===` to each other, even when they represent the same object, since a new representative is created for each act of deserialization.  (Note: future implementations may remove this limitation by having at most one representative in memory for any given virtual object, at which point object identity, i.e., `===`, _will_ be usable for comparison.)  All representatives for a given object do, however, operate on the same underlying state, so changes to the state made by a method call to one representative will be visible to the other representatives.
-
-- The `self` object returned by the `instanceKitMaker` function _is_ the representative, so you can capture it inside any of the body methods (as well as the `init` function) in order to pass the virtual object to somebody else.  So you can write things like the following:
+- The `self` object returned by the `instanceKitMaker` function _is_ the in-memory representation of the virtual object, so you can capture it inside any of the body methods (as well as the `init` function) in order to pass the virtual object to somebody else.  So you can write things like the following:
 
 ```javascript
   function thingKitMaker(state) {
@@ -124,10 +122,10 @@ to obtain a new weak store instance, which you can then access using the `has`,
 
 However, the vat secondary storage system's `makeWeakStore` augments the one provided by the `@agoric/store` package in two important ways:
 
-- Weak store operations may use virtual object representatives as keys, and if you do this the corresponding underlying virtual object instance identities will be used for indexing rather than the object identities of the representative objects (i.e., two different representives for the same underlying virtual object will operate on the same weak store entry).
+- Weak store operations may use virtual objects as keys, and if you do this the corresponding underlying virtual object instance identities will be used for indexing rather than the current in-memory object identities of the objects.
 
 - The values set for weak store entries will be saved persistently to secondary storage.  This means that such weak stores can grow to be very large without impacting the vat's memory footprint.
 
 Since weak store values are saved persistently, they must be serializable and will be hardened as soon as they are set.
 
-An important caveat is that the latter entries are currently not actually weak, in the sense that if a virtual object becomes unreferenced, any corresponding weak store entries will not go away.  This is not semantically visible to vat code, but does impact the disk storage footprint of the vat.  Since virtual object representatives may come and go depending on the working state of the vat, there is no graceful way to garbage collect the underlying objects or corresponding weak store entries; a future system which can do robust distributed garbage collection should eventually rectify this, but for now you should not rely on any such garbage collection happening.
+An important caveat is that the latter entries are currently not actually weak, in the sense that if a virtual object becomes unreferenced, any corresponding weak store entries will not go away.  This is not semantically visible to vat code, but does impact the disk storage footprint of the vat.  Since the in-memory representations of virtual objects may come and go depending on the working state of the vat, there is no graceful way to garbage collect the underlying objects or corresponding weak store entries; a future system which can do robust distributed garbage collection should eventually rectify this, but for now you should not rely on any such garbage collection happening.

packages/SwingSet/src/kernel/liveSlots.js

@@ -27,7 +27,7 @@ const DEFAULT_VIRTUAL_OBJECT_CACHE_SIZE = 3; // XXX ridiculously small value to
  * @param {*} vatParameters
  * @param {*} gcTools { WeakRef, FinalizationRegistry, waitUntilQuiescent }
  * @param {Console} console
- * @returns {*} { vatGlobals, dispatch, setBuildRootObject }
+ * @returns {*} { vatGlobals, inescapableGlobalProperties, dispatch, setBuildRootObject }
  *
  * setBuildRootObject should be called, once, with a function that will
  * create a root object for the new vat The caller provided buildRootObject
@@ -342,10 +342,14 @@ function build(
     makeVirtualObjectRepresentative,
     makeWeakStore,
     makeKind,
+    RepairedWeakMap,
+    RepairedWeakSet,
   } = makeVirtualObjectManager(
     syscall,
     allocateExportID,
     valToSlot,
+    // eslint-disable-next-line no-use-before-define
+    registerValue,
     m,
     cacheSize,
   );
@@ -397,7 +401,7 @@ function build(
   }
 
   function registerValue(slot, val) {
-    const { type } = parseVatSlot(slot);
+    const { type, virtual } = parseVatSlot(slot);
     slotToVal.set(slot, new WeakRef(val));
     valToSlot.set(val, slot);
     if (type === 'object' || type === 'device') {
@@ -414,16 +418,30 @@ function build(
     // that test-liveslots.js test('dropImports') passes despite this,
     // because it uses a fake WeakRef that doesn't care about the strong
     // reference.
-    safetyPins.add(val);
+    if (!virtual) {
+      safetyPins.add(val);
+    }
   }
 
   function convertSlotToVal(slot, iface = undefined) {
+    const { type, allocatedByVat, virtual } = parseVatSlot(slot);
     const wr = slotToVal.get(slot);
     let val = wr && wr.deref();
     if (val) {
+      if (virtual) {
+        // If it's a virtual object for which we already have a representative,
+        // we are going to use that existing representative to preserve ===
+        // equality and WeakMap key usability, BUT we are going to ask the user
+        // code to make a new representative anyway (which we'll discard) so
+        // that as far as the user code is concerned we are making a new
+        // representative with each act of deserialization.  This way they can't
+        // detect reanimation by playing games inside their instanceKitMaker to
+        // try to observe when new representatives are created (e.g., by
+        // counting calls or squirreling things away in hidden WeakMaps).
+        makeVirtualObjectRepresentative(slot, true); // N.b.: throwing away the result
+      }
       return val;
     }
-    const { type, allocatedByVat, virtual } = parseVatSlot(slot);
     if (virtual) {
       // Virtual objects should never be put in the slotToVal table, as their
       // entire raison d'etre is to be absent from memory when they're not being
@@ -433,7 +451,7 @@ function build(
       // this anyway in the cases of creating virtual objects in the first place
       // and swapping them in from disk.
       assert.equal(type, 'object');
-      val = makeVirtualObjectRepresentative(slot);
+      val = makeVirtualObjectRepresentative(slot, false);
     } else {
       assert(!allocatedByVat, X`I don't remember allocating ${slot}`);
       if (type === 'object') {
@@ -459,8 +477,8 @@ function build(
       } else {
         assert.fail(X`unrecognized slot type '${type}'`);
       }
-      registerValue(slot, val);
     }
+    registerValue(slot, val);
     return val;
   }
 
@@ -797,6 +815,11 @@ function build(
     makeKind,
   });
 
+  const inescapableGlobalProperties = harden({
+    WeakMap: RepairedWeakMap,
+    WeakSet: RepairedWeakSet,
+  });
+
   function setBuildRootObject(buildRootObject) {
     assert(!didRoot);
     didRoot = true;
@@ -888,7 +911,14 @@ function build(
   harden(dispatch);
 
   // we return 'deadSet' for unit tests
-  return harden({ vatGlobals, setBuildRootObject, dispatch, m, deadSet });
+  return harden({
+    vatGlobals,
+    inescapableGlobalProperties,
+    setBuildRootObject,
+    dispatch,
+    m,
+    deadSet,
+  });
 }
 
 /**
@@ -903,7 +933,7 @@ function build(
  * @param {boolean} enableDisavow
  * @param {*} gcTools { WeakRef, FinalizationRegistry, waitUntilQuiescent }
  * @param {Console} [liveSlotsConsole]
- * @returns {*} { vatGlobals, dispatch, setBuildRootObject }
+ * @returns {*} { vatGlobals, inescapableGlobalProperties, dispatch, setBuildRootObject }
  *
  * setBuildRootObject should be called, once, with a function that will
  * create a root object for the new vat The caller provided buildRootObject
@@ -951,8 +981,20 @@ export function makeLiveSlots(
     gcTools,
     liveSlotsConsole,
   );
-  const { vatGlobals, dispatch, setBuildRootObject, deadSet } = r; // omit 'm'
-  return harden({ vatGlobals, dispatch, setBuildRootObject, deadSet });
+  const {
+    vatGlobals,
+    inescapableGlobalProperties,
+    dispatch,
+    setBuildRootObject,
+    deadSet,
+  } = r; // omit 'm'
+  return harden({
+    vatGlobals,
+    inescapableGlobalProperties,
+    dispatch,
+    setBuildRootObject,
+    deadSet,
+  });
 }
 
 // for tests

packages/SwingSet/src/kernel/vatManager/manager-local.js

@@ -129,6 +129,7 @@ export function makeLocalVatManagerFactory(tools) {
       assert,
     });
     const inescapableTransforms = [];
+    const inescapableGlobalProperties = { ...ls.inescapableGlobalProperties };
     const inescapableGlobalLexicals = {};
     if (metered) {
       const getMeter = meterRecord.getMeter;
@@ -141,6 +142,7 @@ export function makeLocalVatManagerFactory(tools) {
       endowments,
       inescapableTransforms,
       inescapableGlobalLexicals,
+      inescapableGlobalProperties,
     });
 
     let dispatch;

packages/SwingSet/src/kernel/vatManager/supervisor-nodeworker.js

@@ -95,14 +95,18 @@ parentPort.on('message', ([type, ...margs]) => {
       assert,
     };
 
-    importBundle(bundle, { endowments }).then(vatNS => {
-      workerLog(`got vatNS:`, Object.keys(vatNS).join(','));
-      sendUplink(['gotBundle']);
-      ls.setBuildRootObject(vatNS.buildRootObject);
-      dispatch = makeSupervisorDispatch(ls.dispatch, waitUntilQuiescent);
-      workerLog(`got dispatch:`, Object.keys(dispatch).join(','));
-      sendUplink(['dispatchReady']);
-    });
+    const inescapableGlobalProperties = { ...ls.inescapableGlobalProperties };
+
+    importBundle(bundle, { endowments, inescapableGlobalProperties }).then(
+      vatNS => {
+        workerLog(`got vatNS:`, Object.keys(vatNS).join(','));
+        sendUplink(['gotBundle']);
+        ls.setBuildRootObject(vatNS.buildRootObject);
+        dispatch = makeSupervisorDispatch(ls.dispatch, waitUntilQuiescent);
+        workerLog(`got dispatch:`, Object.keys(dispatch).join(','));
+        sendUplink(['dispatchReady']);
+      },
+    );
   } else if (type === 'deliver') {
     if (!dispatch) {
       workerLog(`error: deliver before dispatchReady`);

packages/SwingSet/src/kernel/vatManager/supervisor-subprocess-node.js

@@ -108,14 +108,18 @@ fromParent.on('data', ([type, ...margs]) => {
       assert,
     };
 
-    importBundle(bundle, { endowments }).then(vatNS => {
-      workerLog(`got vatNS:`, Object.keys(vatNS).join(','));
-      sendUplink(['gotBundle']);
-      ls.setBuildRootObject(vatNS.buildRootObject);
-      dispatch = makeSupervisorDispatch(ls.dispatch, waitUntilQuiescent);
-      workerLog(`got dispatch:`, Object.keys(dispatch).join(','));
-      sendUplink(['dispatchReady']);
-    });
+    const inescapableGlobalProperties = { ...ls.inescapableGlobalProperties };
+
+    importBundle(bundle, { endowments, inescapableGlobalProperties }).then(
+      vatNS => {
+        workerLog(`got vatNS:`, Object.keys(vatNS).join(','));
+        sendUplink(['gotBundle']);
+        ls.setBuildRootObject(vatNS.buildRootObject);
+        dispatch = makeSupervisorDispatch(ls.dispatch, waitUntilQuiescent);
+        workerLog(`got dispatch:`, Object.keys(dispatch).join(','));
+        sendUplink(['dispatchReady']);
+      },
+    );
   } else if (type === 'deliver') {
     if (!dispatch) {
       workerLog(`error: deliver before dispatchReady`);

packages/SwingSet/src/kernel/vatManager/supervisor-subprocess-xsnap.js

@@ -203,7 +203,13 @@ function makeWorker(port) {
       // bootstrap provides HandledPromise
       HandledPromise: globalThis.HandledPromise,
     };
-    const vatNS = await importBundle(bundle, { endowments });
+
+    const inescapableGlobalProperties = { ...ls.inescapableGlobalProperties };
+
+    const vatNS = await importBundle(bundle, {
+      endowments,
+      inescapableGlobalProperties,
+    });
     workerLog(`got vatNS:`, Object.keys(vatNS).join(','));
     ls.setBuildRootObject(vatNS.buildRootObject);
     assert(ls.dispatch);