Skip to content

Commit

Permalink
fix(eventual-send): hoist closures to discourage argument retention
Browse files Browse the repository at this point in the history
We've seen HandledPromise invocation arguments be retained against GC
for surprisingly long periods of time. It seems that the closures
defined in trackTurns() were holding on to more of their lexical scope
than strictly necessary (a lot of JS engines do this). This hoists
those closures up to top-level functions to discourage that retention.

closes #1245
  • Loading branch information
warner authored and kriskowal committed Aug 25, 2022
1 parent 8e8d41c commit 7786d4c
Showing 1 changed file with 44 additions and 39 deletions.
83 changes: 44 additions & 39 deletions packages/eventual-send/src/track-turns.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,49 @@ let hiddenCurrentEvent = 0;
// Turn on if you seem to be losing error logging at the top of the event loop
const VERBOSE = false;

// We hoist these 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.

const addRejectionNote = detailsNote => reason => {
if (reason instanceof Error) {
assert.note(reason, detailsNote);
}
if (VERBOSE) {
console.log('REJECTED at top of event loop', reason);
}
};

const wrapFunction = (func, sendingError, X) => (...args) => {
hiddenPriorError = sendingError;
hiddenCurrentTurn += 1;
hiddenCurrentEvent = 0;
try {
let result;
try {
result = func(...args);
} catch (err) {
if (err instanceof Error) {
assert.note(
err,
X`Thrown from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
}
if (VERBOSE) {
console.log('THROWN to top of event loop', err);
}
throw err;
}
// Must capture this now, not when the catch triggers.
const detailsNote = X`Rejection from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`;
Promise.resolve(result).catch(addRejectionNote(detailsNote));
return harden(result);
} finally {
hiddenPriorError = undefined;
}
};

/**
* @typedef {((...args: any[]) => any) | undefined} TurnStarterFn
* An optional function that is not this-sensitive, expected to be called at
Expand Down Expand Up @@ -53,43 +96,5 @@ export const trackTurns = funcs => {
assert.note(sendingError, X`Caused by: ${hiddenPriorError}`);
}

return funcs.map(
func =>
func &&
((...args) => {
hiddenPriorError = sendingError;
hiddenCurrentTurn += 1;
hiddenCurrentEvent = 0;
try {
let result;
try {
result = func(...args);
} catch (err) {
if (err instanceof Error) {
assert.note(
err,
X`Thrown from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`,
);
}
if (VERBOSE) {
console.log('THROWN to top of event loop', err);
}
throw err;
}
// Must capture this now, not when the catch triggers.
const detailsNote = X`Rejection from: ${hiddenPriorError}:${hiddenCurrentTurn}.${hiddenCurrentEvent}`;
Promise.resolve(result).catch(reason => {
if (reason instanceof Error) {
assert.note(reason, detailsNote);
}
if (VERBOSE) {
console.log('REJECTED at top of event loop', reason);
}
});
return harden(result);
} finally {
hiddenPriorError = undefined;
}
}),
);
return funcs.map(func => func && wrapFunction(func, sendingError, X));
};

0 comments on commit 7786d4c

Please sign in to comment.