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

[v18.x backport] lib: implement AbortSignal.any() #48800

Closed
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,18 @@ added:

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

#### Static method: `AbortSignal.any(signals)`

<!-- YAML
added: REPLACEME
-->

* `signals` {AbortSignal\[]} The `AbortSignal`s of which to compose a new `AbortSignal`.

Returns a new `AbortSignal` which will be aborted if any of the provided
signals are aborted. Its [`abortSignal.reason`][] will be set to whichever
one of the `signals` caused it to be aborted.

#### Event: `'abort'`

<!-- YAML
Expand Down Expand Up @@ -906,6 +918,7 @@ A browser-compatible implementation of [`WritableStreamDefaultWriter`][].
[`WritableStream`]: webstreams.md#class-writablestream
[`__dirname`]: modules.md#__dirname
[`__filename`]: modules.md#__filename
[`abortSignal.reason`]: #abortsignalreason
[`buffer.atob()`]: buffer.md#bufferatobdata
[`buffer.btoa()`]: buffer.md#bufferbtoadata
[`clearImmediate`]: timers.md#clearimmediateimmediate
Expand Down
82 changes: 70 additions & 12 deletions lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const {

const {
validateAbortSignal,
validateAbortSignalArray,
validateObject,
validateUint32,
} = require('internal/validators');
Expand All @@ -54,6 +55,7 @@ const {
clearTimeout,
setTimeout,
} = require('timers');
const assert = require('internal/assert');

const {
messaging_deserialize_symbol: kDeserialize,
Expand All @@ -80,13 +82,16 @@ function lazyMakeTransferable(obj) {
}

const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);
const timeOutSignals = new SafeSet();
const gcPersistentSignals = new SafeSet();

const kAborted = Symbol('kAborted');
const kReason = Symbol('kReason');
const kCloneData = Symbol('kCloneData');
const kTimeout = Symbol('kTimeout');
const kMakeTransferable = Symbol('kMakeTransferable');
const kComposite = Symbol('kComposite');
const kSourceSignals = Symbol('kSourceSignals');
const kDependantSignals = Symbol('kDependantSignals');

function customInspect(self, obj, depth, options) {
if (depth < 0)
Expand Down Expand Up @@ -116,7 +121,7 @@ function setWeakAbortSignalTimeout(weakRef, delay) {
const timeout = setTimeout(() => {
const signal = weakRef.deref();
if (signal !== undefined) {
timeOutSignals.delete(signal);
gcPersistentSignals.delete(signal);
abortSignal(
signal,
new DOMException(
Expand Down Expand Up @@ -185,25 +190,71 @@ class AbortSignal extends EventTarget {
return signal;
}

/**
* @param {AbortSignal[]} signals
* @returns {AbortSignal}
*/
static any(signals) {
validateAbortSignalArray(signals, 'signals');
const resultSignal = createAbortSignal({ composite: true });
if (!signals.length) {
return resultSignal;
}
const resultSignalWeakRef = new WeakRef(resultSignal);
resultSignal[kSourceSignals] = new SafeSet();
for (let i = 0; i < signals.length; i++) {
const signal = signals[i];
if (signal.aborted) {
abortSignal(resultSignal, signal.reason);
return resultSignal;
}
signal[kDependantSignals] ??= new SafeSet();
if (!signal[kComposite]) {
resultSignal[kSourceSignals].add(new WeakRef(signal));
signal[kDependantSignals].add(resultSignalWeakRef);
} else if (!signal[kSourceSignals]) {
continue;
} else {
for (const sourceSignal of signal[kSourceSignals]) {
const sourceSignalRef = sourceSignal.deref();
if (!sourceSignalRef) {
continue;
}
assert(!sourceSignalRef.aborted);
assert(!sourceSignalRef[kComposite]);

if (resultSignal[kSourceSignals].has(sourceSignal)) {
continue;
}
resultSignal[kSourceSignals].add(sourceSignal);
sourceSignalRef[kDependantSignals].add(resultSignalWeakRef);
}
}
}
return resultSignal;
}

[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
const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size);
if (isTimeoutOrNonEmptyCompositeSignal &&
type === 'abort' &&
!this.aborted &&
!weak &&
size === 1) {
// If this is a timeout signal, or a non-empty composite 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);
gcPersistentSignals.add(this);
}
}

[kRemoveListener](size, type, listener, capture) {
super[kRemoveListener](size, type, listener, capture);
if (this[kTimeout] && type === 'abort' && size === 0) {
timeOutSignals.delete(this);
const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size);
if (isTimeoutOrNonEmptyCompositeSignal && type === 'abort' && size === 0) {
gcPersistentSignals.delete(this);
}
}

Expand Down Expand Up @@ -287,7 +338,8 @@ defineEventHandler(AbortSignal.prototype, 'abort');
* @param {{
* aborted? : boolean,
* reason? : any,
* transferable? : boolean
* transferable? : boolean,
* composite? : boolean,
* }} [init]
* @returns {AbortSignal}
*/
Expand All @@ -296,11 +348,13 @@ function createAbortSignal(init = kEmptyObject) {
aborted = false,
reason = undefined,
transferable = false,
composite = false,
} = init;
const signal = new EventTarget();
ObjectSetPrototypeOf(signal, AbortSignal.prototype);
signal[kAborted] = aborted;
signal[kReason] = reason;
signal[kComposite] = composite;
return transferable ? lazyMakeTransferable(signal) : signal;
}

Expand All @@ -312,6 +366,10 @@ function abortSignal(signal, reason) {
[kTrustEvent]: true,
});
signal.dispatchEvent(event);
signal[kDependantSignals]?.forEach((s) => {
const signalRef = s.deref();
if (signalRef) abortSignal(signalRef, reason);
});
}

// TODO(joyeecheung): use private fields and we'll get invalid access
Expand Down
21 changes: 21 additions & 0 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,26 @@ function validateBooleanArray(value, name) {
}
}

/**
* @callback validateAbortSignalArray
* @param {*} value
* @param {string} name
* @returns {asserts value is AbortSignal[]}
*/

/** @type {validateAbortSignalArray} */
function validateAbortSignalArray(value, name) {
validateArray(value, name);
for (let i = 0; i < value.length; i++) {
const signal = value[i];
const indexedName = `${name}[${i}]`;
if (signal == null) {
throw new ERR_INVALID_ARG_TYPE(indexedName, 'AbortSignal', signal);
}
validateAbortSignal(signal, indexedName);
}
}

/**
* @param {*} signal
* @param {string} [name='signal']
Expand Down Expand Up @@ -528,6 +548,7 @@ module.exports = {
validateArray,
validateStringArray,
validateBooleanArray,
validateAbortSignalArray,
validateBoolean,
validateBuffer,
validateDictionary,
Expand Down
10 changes: 5 additions & 5 deletions test/common/wpt.js
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this change is really related / wanted here

Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ class WPTRunner {

process.on('exit', () => {
for (const spec of this.inProgress) {
this.fail(spec, { name: 'Unknown' }, kIncomplete);
this.fail(spec, { name: 'Incomplete' }, kIncomplete);
}
inspect.defaultOptions.depth = Infinity;
// Sorts the rules to have consistent output
Expand Down Expand Up @@ -738,11 +738,11 @@ class WPTRunner {
* @param {object} harnessStatus - The status object returned by WPT harness.
*/
completionCallback(filename, harnessStatus) {
const status = this.getTestStatus(harnessStatus.status);

// Treat it like a test case failure
if (harnessStatus.status === 2) {
const title = this.getTestTitle(filename);
console.log(`---- ${title} ----`);
this.resultCallback(filename, { status: 2, name: 'Unknown' });
if (status === kTimeout) {
this.fail(filename, { name: 'WPT testharness timeout' }, kTimeout);
}
this.inProgress.delete(filename);
// Always force termination of the worker. Some tests allocate resources
Expand Down
9 changes: 9 additions & 0 deletions test/common/wpt/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,17 @@ add_result_callback((result) => {
});
});

// Keep the event loop alive
const timeout = setTimeout(() => {
parentPort.postMessage({
type: 'completion',
status: { status: 2 },
});
}, 2 ** 31 - 1); // Max timeout is 2^31-1, when overflown the timeout is set to 1.

// eslint-disable-next-line no-undef
add_completion_callback((_, status) => {
clearTimeout(timeout);
parentPort.postMessage({
type: 'completion',
status,
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/wpt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Last update:

- common: https://github.com/web-platform-tests/wpt/tree/03c5072aff/common
- console: https://github.com/web-platform-tests/wpt/tree/767ae35464/console
- dom/abort: https://github.com/web-platform-tests/wpt/tree/8fadb38120/dom/abort
- dom/abort: https://github.com/web-platform-tests/wpt/tree/d1f1ecbd52/dom/abort
- dom/events: https://github.com/web-platform-tests/wpt/tree/ab8999891c/dom/events
- encoding: https://github.com/web-platform-tests/wpt/tree/0c1b9d1622/encoding
- fetch/data-urls/resources: https://github.com/web-platform-tests/wpt/tree/7c79d998ff/fetch/data-urls/resources
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/wpt/dom/abort/abort-signal-any.any.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// META: script=./resources/abort-signal-any-tests.js

abortSignalAnySignalOnlyTests(AbortSignal);
abortSignalAnyTests(AbortSignal, AbortController);
Loading