fix(liveslots): collection deletion didn't free key objects

Fix collection deletion. Previously, objects (passables/vrefs) used as
collection keys were not de-referenced when the collection was
dropped/deleted or unconditionally cleared (`collection.clear()`
without keyPatt or valuePatt). Strong collections would leak a
reachable-refcount to the key object, and weak collections would leak
a recognizable-refcount, leading to the object being kept alive (or
kept unretired) forever. In addition, each entry would leak a vatstore
key until the collection was finally deleted.

The code in `clearInternalFull()` confused dbKeys, encodedKeys, and
vrefs, and gave the wrong kind of key to `isEncodedRemotable()`, so
the answer was always "no".

fixes #8756

move test, new naming convention

more test, needs refactor

refactor test

finish weak-collection test

fix: retire objects from weakstore recognizers

add test of voAwareWeakSet deletion and retirement

simplify add/removeRecognizableVref

These functions have two purposes:

* track "recognition records" for keys of weak collections
* tell BOYD about Presence vrefs (addToPossiblyRetiredSet) when a
  record is removed

Virtual/durable weak stores keep their recognition records in vatstore
`vom.ir.${vref}|${collectionID}` keys. voAwareWeakMap/Set keeps them
in RAM, in a Map named vrefRecognizers, where the key is the vref
being recognized, and the value is a Set of virtualObjectMaps (one per
recognizing WeakMap/Set).

Adding a record means adding a `vom.ir.` key, or adding to the
Set (which might require adding a Map entry). Removing a record means
removing a `vom.ir.` key, or removing from the Set (and maybe the Map
entry).

We need to add the vref to possiblyRetiredSet when both 1: it is a
Presence-style vref (o-NN), and 2: it might be the last
recognizer. For `vom.ir.` keys, we do this for every Presence-style
vref, whether or not we're the last one, because searching for other
records would be expensive. For voAwareWeakMap/Set, we have to delete
the Map entry if the Set became empty anyways, so we only
addToPossiblyRetiredSet if it was the last one.

The previous version only updated records for Presence-style and
Representative-style (virtual/durable object) vrefs, and avoided
updating records for Remotable-style vrefs. This changes the logic to
update records for all vrefs, so that retiring a key can remove the
weakstore entry (and its associated data, for WeakMapStores).

This causes two behavioral changes:
* add vom.ir. keys for Remotable vrefs
* only addToPossiblyRetiredSet for presence-style vrefs in weakstores

make virtualReferences right

more

more

more

packages/swingset-liveslots/src/collectionManager.js

@@ -293,6 +293,17 @@ export function makeCollectionManager(
       return `${dbKeyPrefix}${dbEntryKey}`;
     }
 
+    // a "vref" is a string like "o-4"
+    // an "EncodedKey" is the output of encode-passable:
+    // * strings become `s${string}`, like "foo" -> "sfoo"
+    // * positive BigInts become `p${len}:${digits}`, 47n -> "p2:47"
+    // * refs are assigned an "ordinal" and encode as "r0000000001:o-4"
+    // a "DBKey" is used to index the vatstore
+    // * DBKeys for collection entries join a collection prefix to an EncodedKey
+    // * "foo" -> "vc.5.sfoo"
+    // * 47n -> "vc.5.p2:47"
+    // * vref(o-4) -> "vc.5.r0000000001:o-4"
+
     const encodeRemotable = remotable => {
       // eslint-disable-next-line no-use-before-define
       const ordinal = getOrdinal(remotable);
@@ -308,10 +319,11 @@ export function makeCollectionManager(
     // the resulting function will encode only `Key` arguments.
     const encodeKey = makeEncodePassable({ encodeRemotable });
 
-    const vrefFromDBKey = dbKey => dbKey.substring(BIGINT_TAG_LEN + 2);
+    const vrefFromEncodedKey = encodedKey =>
+      encodedKey.substring(1 + BIGINT_TAG_LEN + 1);
 
     const decodeRemotable = encodedKey =>
-      convertSlotToVal(vrefFromDBKey(encodedKey));
+      convertSlotToVal(vrefFromEncodedKey(encodedKey));
 
     // `makeDecodePassable` has three named options:
     // `decodeRemotable`, `decodeError`, and `decodePromise`.
@@ -347,10 +359,15 @@ export function makeCollectionManager(
     }
 
     function dbKeyToKey(dbKey) {
+      // convert e.g. vc.5.r0000000001:o+v10/1 to r0000000001:o+v10/1
       const dbEntryKey = dbKey.substring(dbKeyPrefix.length);
       return decodeKey(dbEntryKey);
     }
 
+    function dbKeyToEncodedKey(dbKey) {
+      return dbKey.substring(dbKeyPrefix.length);
+    }
+
     function has(key) {
       const { keyShape } = getSchema();
       if (!matches(key, keyShape)) {
@@ -563,8 +580,9 @@ export function makeCollectionManager(
         doMoreGC =
           value.slots.map(vrm.removeReachableVref).some(b => b) || doMoreGC;
         syscall.vatstoreDelete(dbKey);
-        if (isEncodedRemotable(dbKey)) {
-          const keyVref = vrefFromDBKey(dbKey);
+        const encodedKey = dbKeyToEncodedKey(dbKey);
+        if (isEncodedRemotable(encodedKey)) {
+          const keyVref = vrefFromEncodedKey(encodedKey);
           if (hasWeakKeys) {
             vrm.removeRecognizableVref(keyVref, `${collectionID}`, true);
           } else {

packages/swingset-liveslots/src/virtualReferences.js

@@ -539,10 +539,13 @@ export function makeVirtualReferenceManager(
     const vref = getSlotForVal(value);
     if (vref) {
       const { type, allocatedByVat, virtual, durable } = parseVatSlot(vref);
-      if (type === 'object' && (!allocatedByVat || virtual || durable)) {
+      if (type === 'object') {
+        // recognizerSet (voAwareWeakMap/Set) doesn't track Remotables
+        const notRemotable = !allocatedByVat || virtual || durable;
+
         if (recognizerIsVirtual) {
           syscall.vatstoreSet(`vom.ir.${vref}|${recognizer}`, '1');
-        } else {
+        } else if (notRemotable) {
           let recognizerSet = vrefRecognizers.get(vref);
           if (!recognizerSet) {
             recognizerSet = new Set();
@@ -556,16 +559,24 @@ export function makeVirtualReferenceManager(
 
   function removeRecognizableVref(vref, recognizer, recognizerIsVirtual) {
     const { type, allocatedByVat, virtual, durable } = parseVatSlot(vref);
-    if (type === 'object' && (!allocatedByVat || virtual || durable)) {
+    if (type === 'object') {
+      // addToPossiblyDeadSet only needs Presence-style vrefs
+      const isPresence = !allocatedByVat;
+      // recognizerSet (voAwareWeakMap/Set) doesn't track Remotables
+      const notRemotable = !allocatedByVat || virtual || durable;
+
       if (recognizerIsVirtual) {
         syscall.vatstoreDelete(`vom.ir.${vref}|${recognizer}`);
-      } else {
+        if (isPresence) {
+          addToPossiblyRetiredSet(vref);
+        }
+      } else if (notRemotable) {
         const recognizerSet = vrefRecognizers.get(vref);
         assert(recognizerSet && recognizerSet.has(recognizer));
         recognizerSet.delete(recognizer);
         if (recognizerSet.size === 0) {
           vrefRecognizers.delete(vref);
-          if (!allocatedByVat) {
+          if (isPresence) {
             addToPossiblyRetiredSet(vref);
           }
         }