Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: add AbortSignal.timeout #40899

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions doc/api/globals.md
Expand Up @@ -104,6 +104,17 @@ changes:

Returns a new already aborted `AbortSignal`.

#### Static method: `AbortSignal.timeout(delay)`

<!-- YAML
added: REPLACEME
-->

* `delay` {number} The number of milliseconds to wait before triggering
the AbortSignal.

Returns a new `AbortSignal` which will be aborted in `delay` milliseconds.

#### Event: `'abort'`

<!-- YAML
Expand Down
85 changes: 84 additions & 1 deletion lib/internal/abort_controller.js
Expand Up @@ -8,15 +8,20 @@ const {
ObjectDefineProperties,
ObjectSetPrototypeOf,
ObjectDefineProperty,
SafeFinalizationRegistry,
SafeSet,
Symbol,
SymbolToStringTag,
WeakRef,
} = primordials;

const {
defineEventHandler,
EventTarget,
Event,
kTrustEvent
kTrustEvent,
kNewListener,
kRemoveListener,
} = require('internal/event_target');
const {
customInspectSymbol,
Expand All @@ -29,8 +34,26 @@ const {
}
} = require('internal/errors');

const {
validateUint32,
} = require('internal/validators');

const {
DOMException,
} = internalBinding('messaging');

jasnell marked this conversation as resolved.
Show resolved Hide resolved
const {
clearTimeout,
setTimeout,
} = require('timers');

const kAborted = Symbol('kAborted');
const kReason = Symbol('kReason');
const kTimeout = Symbol('kTimeout');

const timeOutSignals = new SafeSet();

const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);

function customInspect(self, obj, depth, options) {
if (depth < 0)
Expand All @@ -48,6 +71,30 @@ function validateAbortSignal(obj) {
throw new ERR_INVALID_THIS('AbortSignal');
}

// Because the AbortSignal timeout cannot be canceled, we don't want the
// presence of the timer alone to keep the AbortSignal from being garbage
// collected if it otherwise no longer accessible. We also don't want the
// timer to keep the Node.js process open on it's own. Therefore, we wrap
// the AbortSignal in a WeakRef and have the setTimeout callback close
// over the WeakRef rather than directly over the AbortSignal, and we unref
// the created timer object. Separately, we add the signal to a
// FinalizerRegistry that will clear the timeout when the signal is gc'd.
function setWeakAbortSignalTimeout(weakRef, delay) {
const timeout = setTimeout(() => {
const signal = weakRef.deref();
if (signal !== undefined) {
timeOutSignals.delete(signal);
abortSignal(
signal,
new DOMException(
'The operation was aborted due to timeout',
'TimeoutError'));
}
}, delay);
timeout.unref();
return timeout;
}

class AbortSignal extends EventTarget {
constructor() {
throw new ERR_ILLEGAL_CONSTRUCTOR();
Expand Down Expand Up @@ -82,6 +129,42 @@ class AbortSignal extends EventTarget {
static abort(reason) {
return createAbortSignal(true, reason);
}

/**
* @param {number} delay
* @returns {AbortSignal}
*/
static timeout(delay) {
jasnell marked this conversation as resolved.
Show resolved Hide resolved
validateUint32(delay, 'delay', true);
const signal = createAbortSignal();
signal[kTimeout] = true;
clearTimeoutRegistry.register(
signal,
setWeakAbortSignalTimeout(new WeakRef(signal), delay));
return signal;
}

[kNewListener](size, type, listener, once, capture, passive, weak) {
super[kNewListener](size, type, listener, once, capture, passive, weak);
if (this[kTimeout] &&
type === 'abort' &&
!this.aborted &&
!weak &&
size === 1) {
// If this is a timeout signal, and we're adding a non-weak abort
// listener, then we don't want it to be gc'd while the listener
// is attached and the timer still hasn't fired. So, we retain a
// strong ref that is held for as long as the listener is registered.
timeOutSignals.add(this);
}
}

[kRemoveListener](size, type, listener, capture) {
super[kRemoveListener](size, type, listener, capture);
if (this[kTimeout] && type === 'abort' && size === 0) {
timeOutSignals.delete(this);
}
}
}

ObjectDefineProperties(AbortSignal.prototype, {
Expand Down
15 changes: 11 additions & 4 deletions lib/internal/event_target.js
Expand Up @@ -371,7 +371,7 @@ class EventTarget {
initEventTarget(this);
}

[kNewListener](size, type, listener, once, capture, passive) {
[kNewListener](size, type, listener, once, capture, passive, weak) {
if (this[kMaxEventTargetListeners] > 0 &&
size > this[kMaxEventTargetListeners] &&
!this[kMaxEventTargetListenersWarned]) {
Expand Down Expand Up @@ -440,7 +440,14 @@ class EventTarget {
// This is the first handler in our linked list.
new Listener(root, listener, once, capture, passive,
isNodeStyleListener, weak);
this[kNewListener](root.size, type, listener, once, capture, passive);
this[kNewListener](
root.size,
type,
listener,
once,
capture,
passive,
weak);
this[kEvents].set(type, root);
return;
}
Expand All @@ -461,7 +468,7 @@ class EventTarget {
new Listener(previous, listener, once, capture, passive,
isNodeStyleListener, weak);
root.size++;
this[kNewListener](root.size, type, listener, once, capture, passive);
this[kNewListener](root.size, type, listener, once, capture, passive, weak);
}

removeEventListener(type, listener, options = {}) {
Expand Down Expand Up @@ -811,7 +818,7 @@ function defineEventHandler(emitter, name) {
if (typeof wrappedHandler.handler === 'function') {
this[kEvents].get(name).size++;
const size = this[kEvents].get(name).size;
this[kNewListener](size, name, value, false, false, false);
this[kNewListener](size, name, value, false, false, false, false);
}
} else {
wrappedHandler = makeEventHandler(value);
Expand Down
81 changes: 79 additions & 2 deletions test/parallel/test-abortcontroller.js
@@ -1,10 +1,21 @@
// Flags: --no-warnings
// Flags: --no-warnings --expose-gc --expose-internals
'use strict';

const common = require('../common');
const { inspect } = require('util');

const { ok, strictEqual, throws } = require('assert');
const {
ok,
notStrictEqual,
strictEqual,
throws,
} = require('assert');

const {
kWeakHandler,
} = require('internal/event_target');

const { setTimeout: sleep } = require('timers/promises');

{
// Tests that abort is fired with the correct event type on AbortControllers
Expand Down Expand Up @@ -153,3 +164,69 @@ const { ok, strictEqual, throws } = require('assert');
const signal = AbortSignal.abort('reason');
strictEqual(signal.reason, 'reason');
}

{
// Test AbortSignal timeout
const signal = AbortSignal.timeout(10);
ok(!signal.aborted);
setTimeout(common.mustCall(() => {
ok(signal.aborted);
strictEqual(signal.reason.name, 'TimeoutError');
strictEqual(signal.reason.code, 23);
}), 20);
}

{
(async () => {
// Test AbortSignal timeout doesn't prevent the signal
// from being garbage collected.
let ref;
{
ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
}

await sleep(10);
globalThis.gc();
strictEqual(ref.deref(), undefined);
})().then(common.mustCall());

(async () => {
// Test that an AbortSignal with a timeout is not gc'd while
// there is an active listener on it.
let ref;
function handler() {}
{
ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
ref.deref().addEventListener('abort', handler);
}

await sleep(10);
globalThis.gc();
notStrictEqual(ref.deref(), undefined);
ok(ref.deref() instanceof AbortSignal);

ref.deref().removeEventListener('abort', handler);

await sleep(10);
globalThis.gc();
strictEqual(ref.deref(), undefined);
})().then(common.mustCall());

(async () => {
// If the event listener is weak, however, it should not prevent gc
let ref;
function handler() {}
{
ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
ref.deref().addEventListener('abort', handler, { [kWeakHandler]: {} });
}

await sleep(10);
globalThis.gc();
strictEqual(ref.deref(), undefined);
})().then(common.mustCall());

// Setting a long timeout (20 minutes here) should not
// keep the Node.js process open (the timer is unref'd)
AbortSignal.timeout(1_200_000);
}