fix(ses): avoid holding deep stacks strongly

packages/ses/src/error/note-log-args.js

@@ -1,3 +1,4 @@
+// @ts-check
 /* eslint-disable @endo/no-polymorphic-call */
 /* eslint-disable no-restricted-globals */
 
@@ -7,36 +8,35 @@ const { freeze } = Object;
 const { isSafeInteger } = Number;
 
 /**
+ * @template Data
  * @typedef {object} DoublyLinkedCell
  * A cell of a doubly-linked ring, i.e., a doubly-linked circular list.
  * DoublyLinkedCells are not frozen, and so should be closely encapsulated by
  * any abstraction that uses them.
- * @property {DoublyLinkedCell} next
- * @property {DoublyLinkedCell} prev
- * @property {object} key
- * @property {any} value;
+ * @property {DoublyLinkedCell<Data>} next
+ * @property {DoublyLinkedCell<Data>} prev
+ * @property {Data} data
  */
 
 /**
  * Makes a new self-linked cell. There are two reasons to do so:
  *    * To make the head sigil of a new initially-empty doubly-linked ring.
  *    * To make a non-sigil cell to be `spliceAfter`ed.
  *
- * @param {object} key
- * @param {any} value
- * @returns {DoublyLinkedCell}
+ * @template Data
+ * @param {Data} data
+ * @returns {DoublyLinkedCell<Data>}
  */
-const makeSelfCell = (key, value) => {
-  /** @type {Partial<DoublyLinkedCell>} */
+const makeSelfCell = data => {
+  /** @type {Partial<DoublyLinkedCell<Data>>} */
   const incompleteCell = {
     next: undefined,
     prev: undefined,
-    key,
-    value,
+    data,
   };
-  incompleteCell.next = incompleteCell;
-  incompleteCell.prev = incompleteCell;
-  const selfCell = /** @type {DoublyLinkedCell} */ (incompleteCell);
+  const selfCell = /** @type {DoublyLinkedCell<Data>} */ (incompleteCell);
+  selfCell.next = selfCell;
+  selfCell.prev = selfCell;
   // Not frozen!
   return selfCell;
 };
@@ -46,8 +46,9 @@ const makeSelfCell = (key, value) => {
  * `prev` could be the head sigil, or it could be some other non-sigil
  * cell within a ring.
  *
- * @param {DoublyLinkedCell} prev
- * @param {DoublyLinkedCell} selfCell
+ * @template Data
+ * @param {DoublyLinkedCell<Data>} prev
+ * @param {DoublyLinkedCell<Data>} selfCell
  */
 const spliceAfter = (prev, selfCell) => {
   if (prev === selfCell) {
@@ -69,14 +70,12 @@ const spliceAfter = (prev, selfCell) => {
 };
 
 /**
- * @param {DoublyLinkedCell} cell
- * Must be a non-sigil part of a ring, and therefore non-self-linked
+ * @template Data
+ * @param {DoublyLinkedCell<Data>} cell
+ * No-op if the cell is self-linked.
  */
 const spliceOut = cell => {
   const { prev, next } = cell;
-  if (prev === cell || next === cell) {
-    throw TypeError('Expected non-self-linked cell');
-  }
   prev.next = next;
   next.prev = prev;
   cell.prev = cell;
@@ -87,28 +86,34 @@ const spliceOut = cell => {
  * The LRUCacheMap is used within the implementation of `assert` and so
  * at a layer below SES or harden. Thus, we give it a `WeakMap`-like interface
  * rather than a `WeakMapStore`-like interface. To work before `lockdown`,
- * the implementation must use `freeze` manually, but still exhausively.
- *
- * It does not hold onto anything weakly. The only sense in which it is
- * WeakMap-like is that it does not support enumeration.
+ * the implementation must use `freeze` manually, but still exhaustively.
  *
- * TODO: Make parameterized `Key` and `Value` template types
+ * It implements the WeakMap interface, and holds its keys weakly.  Cached
+ * values are only held while the key is held by the user and the key/value
+ * bookkeeping cell has not been pushed off the end of the cache by `budget`
+ * number of more recently referenced cells.  If the key is dropped by the user,
+ * the value will no longer be held by the cache, but the bookkeeping cell
+ * itself will stay in memory.
  *
+ * @template {{}} K
+ * @template {unknown} V
  * @param {number} budget
- * @returns {WeakMap<object,any>}
+ * @returns {WeakMap<K,V>}
  */
 export const makeLRUCacheMap = budget => {
   if (!isSafeInteger(budget) || budget < 0) {
     throw new TypeError('budget must be a safe non-negative integer number');
   }
-  /** @type {Map<object, DoublyLinkedCell>} */
-  const map = new Map();
+  /** @typedef {DoublyLinkedCell<WeakMap<K, V> | undefined>} LRUCacheCell */
+  /** @type {WeakMap<K, LRUCacheCell>} */
+  const keyToCell = new WeakMap();
   let size = 0; // `size` must remain <= `budget`
-  // As a sigil, `head` uniquely is not in the `map`.
-  const head = makeSelfCell(undefined, undefined);
+  // As a sigil, `head` uniquely is not in the `keyToCell` map.
+  /** @type {LRUCacheCell} */
+  const head = makeSelfCell(undefined);
 
   const touchCell = key => {
-    const cell = map.get(key);
+    const cell = keyToCell.get(key);
     if (cell === undefined) {
       return undefined;
     }
@@ -118,49 +123,79 @@ export const makeLRUCacheMap = budget => {
     return cell;
   };
 
+  /**
+   * @param {K} key
+   */
   const has = key => touchCell(key) !== undefined;
   freeze(has);
 
+  /**
+   * @param {K} key
+   */
   // TODO Change to the following line, once our tools don't choke on `?.`.
   // See https://github.com/endojs/endo/issues/1514
-  // const get = key => touchCell(key)?.value;
+  // const get = key => touchCell(key)?.data?.get(key);
   const get = key => {
     const cell = touchCell(key);
-    return cell && cell.value;
+    return cell && cell.data && cell.data.get(key);
   };
   freeze(get);
 
+  /**
+   * @param {K} key
+   * @param {V} value
+   */
   const set = (key, value) => {
-    if (budget >= 1) {
-      let cell = touchCell(key);
-      if (cell !== undefined) {
-        cell.value = value;
-      } else {
-        if (size >= budget) {
-          const condemned = head.prev;
-          spliceOut(condemned); // Drop least recently used
-          map.delete(condemned.key);
-          size -= 1;
-        }
-        size += 1;
-        cell = makeSelfCell(key, value);
-        map.set(key, cell);
-        spliceAfter(head, cell); // start most recently used
+    if (budget < 1) {
+      // eslint-disable-next-line no-use-before-define
+      return lruCacheMap; // Implements WeakMap.set
+    }
+
+    let cell = touchCell(key);
+    if (cell === undefined) {
+      cell = makeSelfCell(undefined);
+      spliceAfter(head, cell); // start most recently used
+    }
+    if (!cell.data) {
+      // Either a fresh cell or a reused condemned cell.
+      size += 1;
+      // Add its data.
+      cell.data = new WeakMap();
+      // Advertise the cell for this key.
+      keyToCell.set(key, cell);
+      while (size > budget) {
+        const condemned = head.prev;
+        spliceOut(condemned); // Drop least recently used
+        condemned.data = undefined;
+        size -= 1;
       }
     }
+
+    // Update the data.
+    cell.data.set(key, value);
+
     // eslint-disable-next-line no-use-before-define
-    return lruCacheMap; // Needed to be WeakMap-like
+    return lruCacheMap; // Implements WeakMap.set
   };
   freeze(set);
 
   // "delete" is a keyword.
+  /**
+   * @param {K} key
+   */
   const deleteIt = key => {
-    const cell = map.get(key);
+    const cell = keyToCell.get(key);
     if (cell === undefined) {
       return false;
     }
     spliceOut(cell);
-    map.delete(key);
+    keyToCell.delete(key);
+    if (cell.data === undefined) {
+      // Already condemned.
+      return false;
+    }
+
+    cell.data = undefined;
     size -= 1;
     return true;
   };
@@ -171,6 +206,7 @@ export const makeLRUCacheMap = budget => {
     get,
     set,
     delete: deleteIt,
+    [Symbol.toStringTag]: 'LRUCacheMap',
   });
   return lruCacheMap;
 };
@@ -210,7 +246,7 @@ export const makeNoteLogArgsArrayKit = (budget = defaultLogArgsBudget) => {
 
   /**
    * @param {Error} error
-   * @returns {LogArgs[]}
+   * @returns {LogArgs[] | undefined}
    */
   const takeLogArgsArray = error => {
     const result = noteLogArgsArrayMap.get(error);