diff --git a/packages/ses/src/error/note-log-args.js b/packages/ses/src/error/note-log-args.js index f68cc30a32..64b7fd4d16 100644 --- a/packages/ses/src/error/note-log-args.js +++ b/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,14 +8,14 @@ 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} next + * @property {DoublyLinkedCell} prev + * @property {Data} data */ /** @@ -22,21 +23,20 @@ const { isSafeInteger } = Number; * * 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} */ -const makeSelfCell = (key, value) => { - /** @type {Partial} */ +const makeSelfCell = data => { + /** @type {Partial>} */ const incompleteCell = { next: undefined, prev: undefined, - key, - value, + data, }; - incompleteCell.next = incompleteCell; - incompleteCell.prev = incompleteCell; - const selfCell = /** @type {DoublyLinkedCell} */ (incompleteCell); + const selfCell = /** @type {DoublyLinkedCell} */ (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} prev + * @param {DoublyLinkedCell} 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} 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,15 +86,19 @@ 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} keysBudget - * @returns {WeakMap} + * @returns {WeakMap} */ export const makeLRUCacheMap = keysBudget => { if (!isSafeInteger(keysBudget) || keysBudget < 0) { @@ -103,15 +106,18 @@ export const makeLRUCacheMap = keysBudget => { 'keysBudget must be a safe non-negative integer number', ); } - /** @type {Map} */ - const map = new Map(); + /** @typedef {DoublyLinkedCell | undefined>} LRUCacheCell */ + /** @type {WeakMap} */ + const keyToCell = new WeakMap(); let size = 0; // `size` must remain <= `keysBudget` - // 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); - if (cell === undefined) { + const cell = keyToCell.get(key); + if (cell === undefined || cell.data === undefined) { + // Either the key was GCed, or the cell was condemned. return undefined; } // Becomes most recently used @@ -120,49 +126,79 @@ export const makeLRUCacheMap = keysBudget => { 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 (keysBudget >= 1) { - let cell = touchCell(key); - if (cell !== undefined) { - cell.value = value; - } else { - if (size >= keysBudget) { - 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 (keysBudget < 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 > keysBudget) { + 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; }; @@ -173,6 +209,7 @@ export const makeLRUCacheMap = keysBudget => { get, set, delete: deleteIt, + [Symbol.toStringTag]: 'LRUCacheMap', }); return lruCacheMap; }; @@ -226,7 +263,7 @@ export const makeNoteLogArgsArrayKit = ( /** * @param {Error} error - * @returns {LogArgs[]} + * @returns {LogArgs[] | undefined} */ const takeLogArgsArray = error => { const result = noteLogArgsArrayMap.get(error); diff --git a/packages/ses/test/error/test-note-log-args.js b/packages/ses/test/error/test-note-log-args.js index 8de7c8caf9..683b43be39 100644 --- a/packages/ses/test/error/test-note-log-args.js +++ b/packages/ses/test/error/test-note-log-args.js @@ -1,6 +1,10 @@ +// @ts-check import test from 'ava'; -import { makeNoteLogArgsArrayKit } from '../../src/error/note-log-args.js'; +import { + makeNoteLogArgsArrayKit, + makeLRUCacheMap, +} from '../../src/error/note-log-args.js'; test('note log args array kit basic', t => { const { addLogArgs, takeLogArgsArray } = makeNoteLogArgsArrayKit(3, 2); @@ -22,3 +26,40 @@ test('note log args array kit basic', t => { t.deepEqual(takeLogArgsArray(e3), undefined); t.deepEqual(takeLogArgsArray(e4), [['d']]); }); + +test('weak LRUCacheMap', t => { + /** @type {WeakMap<{}, number>} */ + const lru = makeLRUCacheMap(3); + const o1 = {}; + const o2 = {}; + const o3 = {}; + const o4 = {}; + + // Overflow drops the oldest. + lru.set(o1, 1); + t.is(lru.get(o1), 1); + lru.set(o3, 2); + lru.set(o2, 3); + lru.set(o4, 4); // drops o1 + t.falsy(lru.has(o1)); + t.is(lru.get(o4), 4); + lru.set(o4, 5); + t.is(lru.get(o4), 5); + t.true(lru.has(o4)); + lru.set(o1, 6); // drops o3 + t.is(lru.get(o1), 6); + t.false(lru.has(o3)); + + // Explicit delete keeps all other elements. + lru.delete(o1); // explicit delete o1 + t.is(lru.get(o1), undefined); + t.false(lru.has(o1)); + t.true(lru.has(o2)); + t.true(lru.has(o4)); + t.false(lru.has(o3)); + lru.set(o3, 7); + lru.set(o1, 8); // drops o2 + t.false(lru.has(o2)); + t.is(lru.get(o1), 8); + t.is(lru.get(o3), 7); +});