Skip to content

Commit

Permalink
feat(ses): finite deep stacks, on by default (#1513)
Browse files Browse the repository at this point in the history
  • Loading branch information
erights authored Mar 19, 2023
1 parent 6525b23 commit aae0e57
Show file tree
Hide file tree
Showing 4 changed files with 248 additions and 26 deletions.
5 changes: 3 additions & 2 deletions packages/captp/test/test-gc.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ test('test loopback gc', async t => {

await isolated(t, makeFar);
await gcAndFinalize();
t.is(getFarStats().sendCount.CTP_DROP, 3);
t.is(getNearStats().recvCount.CTP_DROP, 3);
// TODO(mfig,erights): explain why #1513 changed these counts from 3 to 2
t.is(getFarStats().sendCount.CTP_DROP, 2);
t.is(getNearStats().recvCount.CTP_DROP, 2);
});
19 changes: 14 additions & 5 deletions packages/eventual-send/src/track-turns.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/* global globalThis */
// @ts-nocheck

const { freeze } = Object;

// NOTE: We can't import these because they're not in scope before lockdown.
// import { assert, details as X, Fail } from '@agoric/assert';

Expand All @@ -22,12 +24,19 @@ const env = (globalThis.process || {}).env || {};
// Turn on if you seem to be losing error logging at the top of the event loop
const VERBOSE = (env.DEBUG || '').split(':').includes('track-turns');

// Track-turns is disabled by default and can be enabled by an environment
// option. We intend to change the default after verifying that having
// the feature enabled in production does not cause memory to leak.
const ENABLED = env.TRACK_TURNS === 'enabled';
const validOptionValues = freeze([undefined, 'enabled', 'disabled']);

// Track-turns is enabled by default and can be disabled by an environment
// option.
const envOptionValue = env.TRACK_TURNS;
if (!validOptionValues.includes(envOptionValue)) {
throw new TypeError(
`unrecognized TRACK_TURNS ${JSON.stringify(envOptionValue)}`,
);
}
const ENABLED = (envOptionValue || 'enabled') !== 'disabled';

// We hoist these functions out of trackTurns() to discourage the
// We hoist the following functions out of trackTurns() to discourage the
// closures from holding onto 'args' or 'func' longer than necessary,
// which we've seen cause HandledPromise arguments to be retained for
// a surprisingly long time.
Expand Down
23 changes: 4 additions & 19 deletions packages/ses/src/error/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
import { an, bestEffortStringify } from './stringify-utils.js';
import './types.js';
import './internal-types.js';
import { makeNoteLogArgsArrayKit } from './note-log-args.js';

// For our internal debugging purposes, uncomment
// const internalDebugConsole = console;
Expand Down Expand Up @@ -251,17 +252,7 @@ freeze(makeError);

// /////////////////////////////////////////////////////////////////////////////

/**
* @type {WeakMap<Error, LogArgs[]>}
*
* Maps from an error to an array of log args, where each log args is
* remembered as an annotation on that error. This can be used, for example,
* to keep track of additional causes of the error. The elements of any
* log args may include errors which are associated with further annotations.
* An augmented console, like the causal console of `console.js`, could
* then retrieve the graph of such annotations.
*/
const hiddenNoteLogArgsArrays = new WeakMap();
const { addLogArgs, takeLogArgsArray } = makeNoteLogArgsArrayKit();

/**
* @type {WeakMap<Error, NoteCallback[]>}
Expand Down Expand Up @@ -295,12 +286,7 @@ const note = (error, detailsNote) => {
callback(error, logArgs);
}
} else {
const logArgsArray = weakmapGet(hiddenNoteLogArgsArrays, error);
if (logArgsArray !== undefined) {
arrayPush(logArgsArray, logArgs);
} else {
weakmapSet(hiddenNoteLogArgsArrays, error, [logArgs]);
}
addLogArgs(error, logArgs);
}
};
freeze(note);
Expand Down Expand Up @@ -339,8 +325,7 @@ const loggedErrorHandler = {
return result;
},
takeNoteLogArgsArray: (error, callback) => {
const result = weakmapGet(hiddenNoteLogArgsArrays, error);
weakmapDelete(hiddenNoteLogArgsArrays, error);
const result = takeLogArgsArray(error);
if (callback !== undefined) {
const callbacks = weakmapGet(hiddenNoteCallbackArrays, error);
if (callbacks) {
Expand Down
227 changes: 227 additions & 0 deletions packages/ses/src/error/note-log-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
/* eslint-disable @endo/no-polymorphic-call */
/* eslint-disable no-restricted-globals */

import './internal-types.js';

const { freeze } = Object;
const { isSafeInteger } = Number;

/**
* @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;
*/

/**
* 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}
*/
const makeSelfCell = (key, value) => {
/** @type {Partial<DoublyLinkedCell>} */
const incompleteCell = {
next: undefined,
prev: undefined,
key,
value,
};
incompleteCell.next = incompleteCell;
incompleteCell.prev = incompleteCell;
const selfCell = /** @type {DoublyLinkedCell} */ (incompleteCell);
// Not frozen!
return selfCell;
};

/**
* Splices a self-linked non-sigil cell into a ring after `prev`.
* `prev` could be the head sigil, or it could be some other non-sigil
* cell within a ring.
*
* @param {DoublyLinkedCell} prev
* @param {DoublyLinkedCell} selfCell
*/
const spliceAfter = (prev, selfCell) => {
if (prev === selfCell) {
throw TypeError('Cannot splice a cell into itself');
}
if (selfCell.next !== selfCell || selfCell.prev !== selfCell) {
throw TypeError('Expected self-linked cell');
}
const cell = selfCell;
// rename variable cause it isn't self-linked after this point.

const next = prev.next;
cell.prev = prev;
cell.next = next;
prev.next = cell;
next.prev = cell;
// Not frozen!
return cell;
};

/**
* @param {DoublyLinkedCell} cell
* Must be a non-sigil part of a ring, and therefore non-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;
cell.next = 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.
*
* TODO: Make parameterized `Key` and `Value` template types
*
* @param {number} budget
* @returns {WeakMap<object,any>}
*/
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();
let size = 0; // `size` must remain <= `budget`
// As a sigil, `head` uniquely is not in the `map`.
const head = makeSelfCell(undefined, undefined);

const touchCell = key => {
const cell = map.get(key);
if (cell === undefined) {
return undefined;
}
// Becomes most recently used
spliceOut(cell);
spliceAfter(head, cell);
return cell;
};

const has = key => touchCell(key) !== undefined;
freeze(has);

// 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 => {
const cell = touchCell(key);
return cell && cell.value;
};
freeze(get);

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
}
}
// eslint-disable-next-line no-use-before-define
return lruCacheMap; // Needed to be WeakMap-like
};
freeze(set);

// "delete" is a keyword.
const deleteIt = key => {
const cell = map.get(key);
if (cell === undefined) {
return false;
}
spliceOut(cell);
map.delete(key);
size -= 1;
return true;
};
freeze(deleteIt);

const lruCacheMap = freeze({
has,
get,
set,
delete: deleteIt,
});
return lruCacheMap;
};
freeze(makeLRUCacheMap);

const defaultLogArgsBudget = 1000;

/**
* @param {number} [budget]
*/
export const makeNoteLogArgsArrayKit = (budget = defaultLogArgsBudget) => {
/**
* @type {WeakMap<Error, LogArgs[]>}
*
* Maps from an error to an array of log args, where each log args is
* remembered as an annotation on that error. This can be used, for example,
* to keep track of additional causes of the error. The elements of any
* log args may include errors which are associated with further annotations.
* An augmented console, like the causal console of `console.js`, could
* then retrieve the graph of such annotations.
*/
const noteLogArgsArrayMap = makeLRUCacheMap(budget);

/**
* @param {Error} error
* @param {LogArgs} logArgs
*/
const addLogArgs = (error, logArgs) => {
const logArgsArray = noteLogArgsArrayMap.get(error);
if (logArgsArray !== undefined) {
logArgsArray.push(logArgs);
} else {
noteLogArgsArrayMap.set(error, [logArgs]);
}
};
freeze(addLogArgs);

/**
* @param {Error} error
* @returns {LogArgs[]}
*/
const takeLogArgsArray = error => {
const result = noteLogArgsArrayMap.get(error);
noteLogArgsArrayMap.delete(error);
return result;
};
freeze(takeLogArgsArray);

return freeze({
addLogArgs,
takeLogArgsArray,
});
};
freeze(makeNoteLogArgsArrayKit);

0 comments on commit aae0e57

Please sign in to comment.