From 0d8f9ffc1b81a691dfdfb02017141498d5f29649 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Tue, 2 May 2023 17:06:56 +0300 Subject: [PATCH 01/10] lib: implement AbortSignal.any() --- doc/api/globals.md | 13 ++ lib/internal/abort_controller.js | 73 ++++++- lib/internal/validators.js | 21 ++ test/fixtures/wpt/README.md | 2 +- .../wpt/dom/abort/abort-signal-any.any.js | 4 + .../abort/resources/abort-signal-any-tests.js | 185 ++++++++++++++++++ test/fixtures/wpt/versions.json | 2 +- 7 files changed, 290 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/wpt/dom/abort/abort-signal-any.any.js create mode 100644 test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js diff --git a/doc/api/globals.md b/doc/api/globals.md index 28aa3526110e7e..5e5ef761a94d9a 100644 --- a/doc/api/globals.md +++ b/doc/api/globals.md @@ -121,6 +121,18 @@ added: Returns a new `AbortSignal` which will be aborted in `delay` milliseconds. +#### Static method: `AbortSignal.any(signals)` + + + +* `signals` {Array} 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'` -* `signals` {Array} The `AbortSignal`s of which to compose a new `AbortSignal`. +* `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 diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index bd9e196f8243b8..b427a928469a88 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -198,21 +198,20 @@ class AbortSignal extends EventTarget { validateAbortSignalArray(signals, 'signals'); const resultSignal = createAbortSignal({ composite: true }); 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; } - resultSignal[kSourceSignals] ??= new SafeSet(); signal[kDependantSignals] ??= new SafeSet(); if (!signal[kComposite]) { resultSignal[kSourceSignals].add(new WeakRef(signal)); signal[kDependantSignals].add(resultSignalWeakRef); + } else if (!signal[kSourceSignals]) { + continue; } else { - if (!signal[kSourceSignals]) { - continue; - } for (const sourceSignal of signal[kSourceSignals]) { const sourceSignalRef = sourceSignal.deref(); if (!sourceSignalRef) { From f86f5bb2edc4f7470de4da919da41498c85ca3b1 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Wed, 3 May 2023 07:22:18 +0300 Subject: [PATCH 03/10] experimental --- doc/api/globals.md | 2 ++ lib/internal/abort_controller.js | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/api/globals.md b/doc/api/globals.md index dc530b796f68b8..b3b88946bec89c 100644 --- a/doc/api/globals.md +++ b/doc/api/globals.md @@ -127,6 +127,8 @@ Returns a new `AbortSignal` which will be aborted in `delay` milliseconds. added: REPLACEME --> +> Stability: 1 - Experimental + * `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 diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index b427a928469a88..6e47cea4b5a445 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -28,6 +28,7 @@ const { const { createDeferredPromise, customInspectSymbol, + emitExperimentalWarning, kEmptyObject, kEnumerableProperty, } = require('internal/util'); @@ -195,6 +196,7 @@ class AbortSignal extends EventTarget { * @returns {AbortSignal} */ static any(signals) { + emitExperimentalWarning('AbortSignal.any'); validateAbortSignalArray(signals, 'signals'); const resultSignal = createAbortSignal({ composite: true }); const resultSignalWeakRef = new WeakRef(resultSignal); @@ -365,8 +367,7 @@ function abortSignal(signal, reason) { signal.dispatchEvent(event); signal[kDependantSignals]?.forEach((s) => { const signalRef = s.deref(); - if (!signalRef) return; - abortSignal(signalRef, reason); + if (signalRef) abortSignal(signalRef, reason); }); } From 2ab215a4fe25b7fa0c37f8b742b54da654a27128 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Fri, 5 May 2023 12:39:45 +0300 Subject: [PATCH 04/10] add UT --- test/parallel/test-abortsignal-any.mjs | 107 +++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 test/parallel/test-abortsignal-any.mjs diff --git a/test/parallel/test-abortsignal-any.mjs b/test/parallel/test-abortsignal-any.mjs new file mode 100644 index 00000000000000..99f1c6ddd97ca5 --- /dev/null +++ b/test/parallel/test-abortsignal-any.mjs @@ -0,0 +1,107 @@ +import * as common from '../common/index.mjs'; +import { describe, it } from 'node:test'; +import { setTimeout } from 'node:timers/promises'; +import assert from 'node:assert'; + +describe('AbortSignal.any()', { concurrency: true }, () => { + it('should throw when not receiving an array', () => { + const regex = /The "signals" argument must be an instance of Array\. Received/; + assert.throws(() => AbortSignal.any(), regex); + assert.throws(() => AbortSignal.any(null), regex); + assert.throws(() => AbortSignal.any(undefined), regex); + }); + + it('should throw when input contains non-signal values', () => { + try { + AbortSignal.any([AbortSignal.abort(), undefined]); + assert.fail('AbortSignal.any() should not accept an input with non-signals'); + } catch (err) { + assert.strictEqual( + err.message, + 'The "signals[1]" argument must be an instance of AbortSignal. Received undefined' + ); + } + }); + + it('creates a non-aborted signal for an empty input', () => { + const signal = AbortSignal.any([]); + assert.strictEqual(signal.aborted, false); + signal.addEventListener('abort', common.mustNotCall()); + }); + + it('returns a new signal', () => { + const originalSignal = new AbortController().signal; + const signalAny = AbortSignal.any([originalSignal]); + assert.notStrictEqual(originalSignal, signalAny); + }); + + it('returns an aborted signal if input has an aborted signal', () => { + const signal = AbortSignal.any([AbortSignal.abort('some reason')]); + assert.strictEqual(signal.aborted, true); + assert.strictEqual(signal.reason, 'some reason'); + signal.addEventListener('abort', common.mustNotCall()); + }); + + it('returns an aborted signal with the reason of first aborted signal input', () => { + const signal = AbortSignal.any([AbortSignal.abort('some reason'), AbortSignal.abort('another reason')]); + assert.strictEqual(signal.aborted, true); + assert.strictEqual(signal.reason, 'some reason'); + signal.addEventListener('abort', common.mustNotCall()); + }); + + it('returns the correct signal in the event target', async () => { + const signal = AbortSignal.any([AbortSignal.timeout(5)]); + signal.addEventListener('abort', common.mustCall((e) => { + assert.strictEqual(e.target, signal); + })); + await setTimeout(10); + assert.ok(signal.aborted); + assert.strictEqual(signal.reason.name, 'TimeoutError'); + assert.strictEqual(signal.reason.message, 'The operation was aborted due to timeout'); + }); + + it('aborts with reason of first aborted signal', () => { + const controllers = Array.from({ length: 3 }, () => new AbortController()); + const combinedSignal = AbortSignal.any(controllers.map((c) => c.signal)); + controllers[1].abort(1); + controllers[2].abort(2); + assert.ok(combinedSignal.aborted); + assert.strictEqual(combinedSignal.reason, 1); + }); + + it('can accept the same signal more than once', () => { + const controller = new AbortController(); + const signal = AbortSignal.any([controller.signal, controller.signal]); + assert.strictEqual(signal.aborted, false); + controller.abort('reason'); + assert.ok(signal.aborted); + assert.strictEqual(signal.reason, 'reason'); + }); + + it('handles deeply aborted signals', async () => { + const controllers = Array.from({ length: 2 }, () => new AbortController()); + const composedSignal1 = AbortSignal.any([controllers[0].signal]); + const composedSignal2 = AbortSignal.any([composedSignal1, controllers[1].signal]); + + composedSignal2.onabort = common.mustCall(); + controllers[0].abort(); + assert.ok(composedSignal2.aborted); + assert.ok(composedSignal2.reason instanceof DOMException); + assert.strictEqual(composedSignal2.reason.name, 'AbortError'); + }); + + it('executes abort handlers in correct order', () => { + const controller = new AbortController(); + const signals = []; + signals.push(controller.signal); + signals.push(AbortSignal.any([controller.signal])); + signals.push(AbortSignal.any([controller.signal])); + signals.push(AbortSignal.any([signals[0]])); + signals.push(AbortSignal.any([signals[1]])); + + let result = ''; + signals.forEach((signal, i) => signal.addEventListener('abort', () => result += i)); + controller.abort(); + assert.strictEqual(result, '01234'); + }); +}); From db2c783e941dc1c5184b25fd96fddc5db1fb8de5 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Sun, 7 May 2023 21:34:03 +0300 Subject: [PATCH 05/10] wip --- lib/internal/abort_controller.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 6e47cea4b5a445..c94296e462476f 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -195,6 +195,22 @@ class AbortSignal extends EventTarget { * @param {AbortSignal[]} signals * @returns {AbortSignal} */ + // static any(signals) { + // // emitExperimentalWarning('AbortSignal.any'); + // validateAbortSignalArray(signals, 'signals'); + // const ac = new AbortController(); + // for (let i = 0; i < signals.length; i++) { + // const signal = signals[i]; + // if (signal.aborted) { + // ac.abort(signal.reason); + // return ac.signal; + // } + // signal.addEventListener("abort", (e) => { + // ac.abort(e.target.reason); + // }, { once: true, [kWeakHandler]: ac.signal }); + // } + // return ac.signal; + // } static any(signals) { emitExperimentalWarning('AbortSignal.any'); validateAbortSignalArray(signals, 'signals'); From 6086ef6a09e80f04d665d859c10498e918741300 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Mon, 15 May 2023 11:57:19 +0300 Subject: [PATCH 06/10] CR --- lib/internal/abort_controller.js | 18 ------------------ test/parallel/test-abortsignal-any.mjs | 25 +++++++++++-------------- 2 files changed, 11 insertions(+), 32 deletions(-) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index c94296e462476f..779d2a7c39144a 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -28,7 +28,6 @@ const { const { createDeferredPromise, customInspectSymbol, - emitExperimentalWarning, kEmptyObject, kEnumerableProperty, } = require('internal/util'); @@ -195,24 +194,7 @@ class AbortSignal extends EventTarget { * @param {AbortSignal[]} signals * @returns {AbortSignal} */ - // static any(signals) { - // // emitExperimentalWarning('AbortSignal.any'); - // validateAbortSignalArray(signals, 'signals'); - // const ac = new AbortController(); - // for (let i = 0; i < signals.length; i++) { - // const signal = signals[i]; - // if (signal.aborted) { - // ac.abort(signal.reason); - // return ac.signal; - // } - // signal.addEventListener("abort", (e) => { - // ac.abort(e.target.reason); - // }, { once: true, [kWeakHandler]: ac.signal }); - // } - // return ac.signal; - // } static any(signals) { - emitExperimentalWarning('AbortSignal.any'); validateAbortSignalArray(signals, 'signals'); const resultSignal = createAbortSignal({ composite: true }); const resultSignalWeakRef = new WeakRef(resultSignal); diff --git a/test/parallel/test-abortsignal-any.mjs b/test/parallel/test-abortsignal-any.mjs index 99f1c6ddd97ca5..8fff1edbf68733 100644 --- a/test/parallel/test-abortsignal-any.mjs +++ b/test/parallel/test-abortsignal-any.mjs @@ -5,22 +5,20 @@ import assert from 'node:assert'; describe('AbortSignal.any()', { concurrency: true }, () => { it('should throw when not receiving an array', () => { - const regex = /The "signals" argument must be an instance of Array\. Received/; - assert.throws(() => AbortSignal.any(), regex); - assert.throws(() => AbortSignal.any(null), regex); - assert.throws(() => AbortSignal.any(undefined), regex); + const expectedError = { code: 'ERR_INVALID_ARG_TYPE' }; + assert.throws(() => AbortSignal.any(), expectedError); + assert.throws(() => AbortSignal.any(null), expectedError); + assert.throws(() => AbortSignal.any(undefined), expectedError); }); it('should throw when input contains non-signal values', () => { - try { - AbortSignal.any([AbortSignal.abort(), undefined]); - assert.fail('AbortSignal.any() should not accept an input with non-signals'); - } catch (err) { - assert.strictEqual( - err.message, - 'The "signals[1]" argument must be an instance of AbortSignal. Received undefined' - ); - } + assert.throws( + () => AbortSignal.any([AbortSignal.abort(), undefined]), + { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "signals[1]" argument must be an instance of AbortSignal. Received undefined' + }, + ); }); it('creates a non-aborted signal for an empty input', () => { @@ -57,7 +55,6 @@ describe('AbortSignal.any()', { concurrency: true }, () => { await setTimeout(10); assert.ok(signal.aborted); assert.strictEqual(signal.reason.name, 'TimeoutError'); - assert.strictEqual(signal.reason.message, 'The operation was aborted due to timeout'); }); it('aborts with reason of first aborted signal', () => { From e09e0eea1e31e2faad3f4be2ccf7b4b4432b51d2 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 15 May 2023 22:32:33 +0200 Subject: [PATCH 07/10] test: keep the event loop alive --- test/common/wpt.js | 8 +++++--- test/common/wpt/worker.js | 9 +++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test/common/wpt.js b/test/common/wpt.js index cc7c5320fa972e..5ff731e13051d5 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -680,7 +680,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 @@ -796,9 +796,11 @@ class WPTRunner { * @param {object} harnessStatus - The status object returned by WPT harness. */ completionCallback(spec, harnessStatus) { + const status = this.getTestStatus(harnessStatus.status); + // Treat it like a test case failure - if (harnessStatus.status === 2) { - this.resultCallback(spec, { status: 2, name: 'Unknown' }); + if (status === kTimeout) { + this.fail(spec, { name: 'WPT testharness timeout' }, kTimeout); } this.inProgress.delete(spec); // Always force termination of the worker. Some tests allocate resources diff --git a/test/common/wpt/worker.js b/test/common/wpt/worker.js index 80b32bf5b912e7..5638845131b7f1 100644 --- a/test/common/wpt/worker.js +++ b/test/common/wpt/worker.js @@ -48,8 +48,17 @@ add_result_callback((result) => { }); }); +// Keep the event loop alive +const timeout = setTimeout(() => { + parentPort.postMessage({ + type: 'completion', + status: { status: 2 }, + }); +}, Math.pow(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, From b5be2a8d230532f8318af709cb31f6e40892e057 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Tue, 16 May 2023 07:35:46 +0300 Subject: [PATCH 08/10] CR --- test/common/wpt/worker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/wpt/worker.js b/test/common/wpt/worker.js index 5638845131b7f1..855ec7e91c394b 100644 --- a/test/common/wpt/worker.js +++ b/test/common/wpt/worker.js @@ -54,7 +54,7 @@ const timeout = setTimeout(() => { type: 'completion', status: { status: 2 }, }); -}, Math.pow(2, 31) - 1); // Max timeout is 2^31-1, when overflown the timeout is set to 1 +}, 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) => { From a086465e8acc680e79f8623bc5821f9ecba16279 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Wed, 17 May 2023 18:53:45 +0300 Subject: [PATCH 09/10] test no sleep --- test/parallel/test-abortsignal-any.mjs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-abortsignal-any.mjs b/test/parallel/test-abortsignal-any.mjs index 8fff1edbf68733..ca99c98d122db4 100644 --- a/test/parallel/test-abortsignal-any.mjs +++ b/test/parallel/test-abortsignal-any.mjs @@ -1,6 +1,6 @@ import * as common from '../common/index.mjs'; import { describe, it } from 'node:test'; -import { setTimeout } from 'node:timers/promises'; +import { once } from 'node:events'; import assert from 'node:assert'; describe('AbortSignal.any()', { concurrency: true }, () => { @@ -49,10 +49,10 @@ describe('AbortSignal.any()', { concurrency: true }, () => { it('returns the correct signal in the event target', async () => { const signal = AbortSignal.any([AbortSignal.timeout(5)]); - signal.addEventListener('abort', common.mustCall((e) => { - assert.strictEqual(e.target, signal); - })); - await setTimeout(10); + const interval = setInterval(() => {}, 100000); // Keep event loop alive + const [{ target }] = await once(signal, 'abort'); + clearInterval(interval); + assert.strictEqual(target, signal); assert.ok(signal.aborted); assert.strictEqual(signal.reason.name, 'TimeoutError'); }); From 28f5b847f4b199dbc2162b5422148f92196a35e3 Mon Sep 17 00:00:00 2001 From: atlowChemi Date: Wed, 17 May 2023 19:10:36 +0300 Subject: [PATCH 10/10] fix --- doc/api/globals.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/api/globals.md b/doc/api/globals.md index b3b88946bec89c..dc530b796f68b8 100644 --- a/doc/api/globals.md +++ b/doc/api/globals.md @@ -127,8 +127,6 @@ Returns a new `AbortSignal` which will be aborted in `delay` milliseconds. added: REPLACEME --> -> Stability: 1 - Experimental - * `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