From f7c3b03759aef295d6b767cef6b87f19a4f39001 Mon Sep 17 00:00:00 2001 From: jazelly Date: Sun, 8 Sep 2024 20:23:14 +0930 Subject: [PATCH] lib: propagate aborted state to dependent signals before firing events PR-URL: https://github.com/nodejs/node/pull/54826 Fixes: https://github.com/nodejs/node/issues/54466 Fixes: https://github.com/nodejs/node/issues/54601 Reviewed-By: James M Snell --- lib/internal/abort_controller.js | 38 +++++++++++-- test/fixtures/wpt/README.md | 2 +- test/fixtures/wpt/dom/abort/WEB_FEATURES.yml | 6 ++ .../wpt/dom/abort/abort-signal-any-crash.html | 23 ++++++++ .../dom/abort/crashtests/any-on-abort.html | 11 ++++ .../abort/resources/abort-signal-any-tests.js | 55 +++++++++++++++++++ test/fixtures/wpt/versions.json | 2 +- 7 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/wpt/dom/abort/WEB_FEATURES.yml create mode 100644 test/fixtures/wpt/dom/abort/abort-signal-any-crash.html create mode 100644 test/fixtures/wpt/dom/abort/crashtests/any-on-abort.html diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index ce05571c067398..d8607c499b31e0 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -4,9 +4,11 @@ // in https://github.com/mysticatea/abort-controller (MIT license) const { + ArrayPrototypePush, ObjectAssign, ObjectDefineProperties, ObjectDefineProperty, + ObjectSetPrototypeOf, PromiseResolve, SafeFinalizationRegistry, SafeSet, @@ -372,18 +374,46 @@ ObjectDefineProperty(AbortSignal.prototype, SymbolToStringTag, { defineEventHandler(AbortSignal.prototype, 'abort'); +// https://dom.spec.whatwg.org/#dom-abortsignal-abort function abortSignal(signal, reason) { + // 1. If signal is aborted, then return. if (signal[kAborted]) return; + + // 2. Set signal's abort reason to reason if it is given; + // otherwise to a new "AbortError" DOMException. signal[kAborted] = true; signal[kReason] = reason; + // 3. Let dependentSignalsToAbort be a new list. + const dependentSignalsToAbort = ObjectSetPrototypeOf([], null); + // 4. For each dependentSignal of signal's dependent signals: + signal[kDependantSignals]?.forEach((s) => { + const dependentSignal = s.deref(); + // 1. If dependentSignal is not aborted, then: + if (dependentSignal && !dependentSignal[kAborted]) { + // 1. Set dependentSignal's abort reason to signal's abort reason. + dependentSignal[kReason] = reason; + dependentSignal[kAborted] = true; + // 2. Append dependentSignal to dependentSignalsToAbort. + ArrayPrototypePush(dependentSignalsToAbort, dependentSignal); + } + }); + + // 5. Run the abort steps for signal + runAbort(signal); + // 6. For each dependentSignal of dependentSignalsToAbort, + // run the abort steps for dependentSignal. + for (let i = 0; i < dependentSignalsToAbort.length; i++) { + const dependentSignal = dependentSignalsToAbort[i]; + runAbort(dependentSignal); + } +} + +// To run the abort steps for an AbortSignal signal +function runAbort(signal) { const event = new Event('abort', { [kTrustEvent]: true, }); signal.dispatchEvent(event); - signal[kDependantSignals]?.forEach((s) => { - const signalRef = s.deref(); - if (signalRef) abortSignal(signalRef, reason); - }); } class AbortController { diff --git a/test/fixtures/wpt/README.md b/test/fixtures/wpt/README.md index 3e60ab39f00971..ad9d401380b1af 100644 --- a/test/fixtures/wpt/README.md +++ b/test/fixtures/wpt/README.md @@ -13,7 +13,7 @@ Last update: - common: https://github.com/web-platform-tests/wpt/tree/dbd648158d/common - compression: https://github.com/web-platform-tests/wpt/tree/5aa50dd415/compression - console: https://github.com/web-platform-tests/wpt/tree/767ae35464/console -- dom/abort: https://github.com/web-platform-tests/wpt/tree/d1f1ecbd52/dom/abort +- dom/abort: https://github.com/web-platform-tests/wpt/tree/0143fe244b/dom/abort - dom/events: https://github.com/web-platform-tests/wpt/tree/0a811c5161/dom/events - encoding: https://github.com/web-platform-tests/wpt/tree/5aa50dd415/encoding - fetch/data-urls/resources: https://github.com/web-platform-tests/wpt/tree/7c79d998ff/fetch/data-urls/resources diff --git a/test/fixtures/wpt/dom/abort/WEB_FEATURES.yml b/test/fixtures/wpt/dom/abort/WEB_FEATURES.yml new file mode 100644 index 00000000000000..d2fdc1555e2830 --- /dev/null +++ b/test/fixtures/wpt/dom/abort/WEB_FEATURES.yml @@ -0,0 +1,6 @@ +features: +- name: aborting + files: "**" +- name: abortsignal-any + files: + - abort-signal-any.any.js diff --git a/test/fixtures/wpt/dom/abort/abort-signal-any-crash.html b/test/fixtures/wpt/dom/abort/abort-signal-any-crash.html new file mode 100644 index 00000000000000..912c0d0ada7385 --- /dev/null +++ b/test/fixtures/wpt/dom/abort/abort-signal-any-crash.html @@ -0,0 +1,23 @@ + + + + AbortSignal::Any when source signal was garbage collected + + + + + +

Test passes if the browser does not crash.

+ + + diff --git a/test/fixtures/wpt/dom/abort/crashtests/any-on-abort.html b/test/fixtures/wpt/dom/abort/crashtests/any-on-abort.html new file mode 100644 index 00000000000000..07a0f0bd3c2ba6 --- /dev/null +++ b/test/fixtures/wpt/dom/abort/crashtests/any-on-abort.html @@ -0,0 +1,11 @@ + + + diff --git a/test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js b/test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js index 66e4141eaccb08..8f897c934e87ea 100644 --- a/test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js +++ b/test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js @@ -182,4 +182,59 @@ function abortSignalAnyTests(signalInterface, controllerInterface) { controller.abort(); assert_equals(result, "01234"); }, `Abort events for ${desc} signals fire in the right order ${suffix}`); + + test(t => { + const controller = new controllerInterface(); + const signal1 = signalInterface.any([controller.signal]); + const signal2 = signalInterface.any([signal1]); + let eventFired = false; + + controller.signal.addEventListener('abort', () => { + const signal3 = signalInterface.any([signal2]); + assert_true(controller.signal.aborted); + assert_true(signal1.aborted); + assert_true(signal2.aborted); + assert_true(signal3.aborted); + eventFired = true; + }); + + controller.abort(); + assert_true(eventFired, "event fired"); + }, `Dependent signals for ${desc} are marked aborted before abort events fire ${suffix}`); + + test(t => { + const controller1 = new controllerInterface(); + const controller2 = new controllerInterface(); + const signal = signalInterface.any([controller1.signal, controller2.signal]); + let count = 0; + + controller1.signal.addEventListener('abort', () => { + controller2.abort("reason 2"); + }); + + signal.addEventListener('abort', () => { + count++; + }); + + controller1.abort("reason 1"); + assert_equals(count, 1); + assert_true(signal.aborted); + assert_equals(signal.reason, "reason 1"); + }, `Dependent signals for ${desc} are aborted correctly for reentrant aborts ${suffix}`); + + test(t => { + const source = signalInterface.abort(); + const dependent = signalInterface.any([source]); + assert_true(source.reason instanceof DOMException); + assert_equals(source.reason, dependent.reason); + }, `Dependent signals for ${desc} should use the same DOMException instance from the already aborted source signal ${suffix}`); + + test(t => { + const controller = new controllerInterface(); + const source = controller.signal; + const dependent = signalInterface.any([source]); + controller.abort(); + assert_true(source.reason instanceof DOMException); + assert_equals(source.reason, dependent.reason); + }, `Dependent signals for ${desc} should use the same DOMException instance from the source signal being aborted later ${suffix}`); } diff --git a/test/fixtures/wpt/versions.json b/test/fixtures/wpt/versions.json index 3edb1453e7488e..225a8a83d747ee 100644 --- a/test/fixtures/wpt/versions.json +++ b/test/fixtures/wpt/versions.json @@ -12,7 +12,7 @@ "path": "console" }, "dom/abort": { - "commit": "d1f1ecbd52f2eab3b7fe5dc1b20b41174f1341ce", + "commit": "0143fe244b3d622441717ce630e0114eb204f9a7", "path": "dom/abort" }, "dom/events": {