Skip to content

Commit 8602dbb

Browse files
committed
lib: capture no stack for default abort reason
The default AbortError created by AbortController.prototype.abort() and AbortSignal.abort() captures a stack that V8 keeps unformatted. Until it is formatted, V8 holds on to the call sites, which retain the receivers and functions of the calling frames. A signal that outlives the work it cancelled can then keep that work alive, including large buffers. Create the default reason with Error.stackTraceLimit set to 0, so no call sites are captured. This also makes abort() without a reason faster, as the stack is no longer collected. Callers that want a stack can pass their own reason. Reasons passed by the caller are left untouched. Signed-off-by: Kuldeep Yadav <kuldeeep.yadav1@gmail.com>
1 parent e7d8ab5 commit 8602dbb

2 files changed

Lines changed: 67 additions & 3 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const {
4141
ERR_INVALID_ARG_TYPE,
4242
ERR_INVALID_THIS,
4343
},
44+
isErrorStackTraceLimitWritable,
4445
} = require('internal/errors');
4546
const {
4647
converters,
@@ -216,6 +217,23 @@ function setWeakAbortSignalTimeout(weakRef, delay) {
216217
return timeout;
217218
}
218219

220+
// The default reason captures no stack. A captured stack keeps the receivers
221+
// and functions of the aborting caller's frames alive until it is formatted,
222+
// so a retained signal would keep the aborted work alive, and formatting it
223+
// eagerly costs several microseconds per abort. Callers that want the stack
224+
// can pass their own reason.
225+
// Refs: https://github.com/nodejs/node/issues/66192
226+
function createDefaultAbortReason() {
227+
if (!isErrorStackTraceLimitWritable()) {
228+
return new DOMException('This operation was aborted', 'AbortError');
229+
}
230+
const limit = Error.stackTraceLimit;
231+
Error.stackTraceLimit = 0;
232+
const reason = new DOMException('This operation was aborted', 'AbortError');
233+
Error.stackTraceLimit = limit;
234+
return reason;
235+
}
236+
219237
class AbortSignal extends EventTarget {
220238
#brand;
221239

@@ -293,8 +311,7 @@ class AbortSignal extends EventTarget {
293311
* @param {any} [reason]
294312
* @returns {AbortSignal}
295313
*/
296-
static abort(
297-
reason = new DOMException('This operation was aborted', 'AbortError')) {
314+
static abort(reason = createDefaultAbortReason()) {
298315
return new AbortSignal(kDontThrowSymbol, { aborted: true, reason });
299316
}
300317

@@ -576,7 +593,7 @@ class AbortController {
576593
/**
577594
* @param {any} [reason]
578595
*/
579-
abort(reason = new DOMException('This operation was aborted', 'AbortError')) {
596+
abort(reason = createDefaultAbortReason()) {
580597
abortSignal(this.#signal ??= new AbortSignal(kDontThrowSymbol), reason);
581598
}
582599

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Flags: --expose-gc
2+
3+
import '../common/index.mjs';
4+
import { gcUntil } from '../common/gc.js';
5+
import assert from 'node:assert/strict';
6+
import { it } from 'node:test';
7+
8+
// The default abort reason must not keep the objects on the caller's stack
9+
// alive. Refs: https://github.com/nodejs/node/issues/66192
10+
11+
class Job {
12+
controller = new AbortController();
13+
14+
cancel() {
15+
this.controller.abort();
16+
return this.controller.signal;
17+
}
18+
19+
cancelStatic() {
20+
return AbortSignal.abort();
21+
}
22+
}
23+
24+
for (const method of ['cancel', 'cancelStatic']) {
25+
it(`does not retain the caller through the default reason (${method})`, async () => {
26+
let job = new Job();
27+
const jobRef = new WeakRef(job);
28+
const signal = job[method]();
29+
job = null;
30+
31+
await gcUntil('job is collected', () => jobRef.deref() === undefined);
32+
assert.strictEqual(signal.aborted, true);
33+
assert.strictEqual(signal.reason.name, 'AbortError');
34+
assert.doesNotMatch(signal.reason.stack, /\n\s+at /);
35+
});
36+
}
37+
38+
it('does not throw when Error.prepareStackTrace throws', () => {
39+
const { prepareStackTrace } = Error;
40+
Error.prepareStackTrace = () => { throw new Error('boom'); };
41+
try {
42+
assert.strictEqual(new Job().cancel().reason.name, 'AbortError');
43+
assert.strictEqual(new Job().cancelStatic().reason.name, 'AbortError');
44+
} finally {
45+
Error.prepareStackTrace = prepareStackTrace;
46+
}
47+
});

0 commit comments

Comments
 (0)