lib: propagate aborted state to dependent signals before firing events

jazelly · jasnell · commit d47360604047 · 2024-09-21T09:07:08.000-07:00
PR-URL: #54826 Fixes: #54466 Fixes: #54601 Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html class=test-wait>
+ <head>
+ <title>AbortSignal::Any when source signal was garbage collected</title>
+ <link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1908466">
+ <link rel="author" title="Vincent Hilla" href="mailto:vhilla@mozilla.com">
+ <script src="/common/gc.js"></script>
+ </head>
+ <body>
+ <p>Test passes if the browser does not crash.</p>
+ <script>
+ async function test() {
+ let controller = new AbortController();
+ let signal = AbortSignal.any([controller.signal]);
+ controller = undefined;
+ await garbageCollect();
+ AbortSignal.any([signal]);
+ document.documentElement.classList.remove('test-wait');
+ }
+ test();
+ </script>
+ </body>
+</html>
diff --git a/test/fixtures/wpt/dom/abort/crashtests/any-on-abort.html b/test/fixtures/wpt/dom/abort/crashtests/any-on-abort.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<script>
+ let b;
+ window.addEventListener("DOMContentLoaded", () => {
+ let a = new AbortController()
+ b = AbortSignal.any([a.signal])
+ a.signal.addEventListener("abort", () => { AbortSignal.any([b]) }, { })
+ a.abort(undefined)
+ })
+</script>
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
@@ -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
@@ -12,7 +12,7 @@
 "path": "console"
 },
 "dom/abort": {
- "commit": "d1f1ecbd52f2eab3b7fe5dc1b20b41174f1341ce",
+ "commit": "0143fe244b3d622441717ce630e0114eb204f9a7",
 "path": "dom/abort"
 },
 "dom/events": {

-Original file line number
+Diff line change
@@ @@ -0,0 +1,23 @@ @@
 +<!DOCTYPE html>
 +<htmlclass=test-wait>
 +<head>
 +<title>AbortSignal::Any when source signal was garbage collected</title>
 +<linkrel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1908466">
 +<linkrel="author" title="Vincent Hilla" href="mailto:vhilla@mozilla.com">
 +<scriptsrc="/common/gc.js"></script>
 +</head>
 +<body>
 +<p>Test passes if the browser does not crash.</p>
 +<script>
 +asyncfunctiontest(){
 +letcontroller=newAbortController();
 +letsignal=AbortSignal.any([controller.signal]);
 +controller=undefined;
 +awaitgarbageCollect();
 +AbortSignal.any([signal]);
 +document.documentElement.classList.remove('test-wait');
 +}
 +test();
 +</script>
 +</body>
 +</html>
-Original file line number
+Diff line change
 controller.abort();
 assert_equals(result,"01234");
 },`Abort events for ${desc} signals fire in the right order ${suffix}`);
++
 +test(t=>{
 +constcontroller=newcontrollerInterface();
 +constsignal1=signalInterface.any([controller.signal]);
 +constsignal2=signalInterface.any([signal1]);
 +leteventFired=false;
++
 +controller.signal.addEventListener('abort',()=>{
 +constsignal3=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=>{
 +constcontroller1=newcontrollerInterface();
 +constcontroller2=newcontrollerInterface();
 +constsignal=signalInterface.any([controller1.signal,controller2.signal]);
 +letcount=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=>{
 +constsource=signalInterface.abort();
 +constdependent=signalInterface.any([source]);
 +assert_true(source.reasoninstanceofDOMException);
 +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=>{
 +constcontroller=newcontrollerInterface();
 +constsource=controller.signal;
 +constdependent=signalInterface.any([source]);
 +controller.abort();
 +assert_true(source.reasoninstanceofDOMException);
 +assert_equals(source.reason,dependent.reason);
 +},`Dependent signals for ${desc} should use the same DOMException instance from the source signal being aborted later ${suffix}`);
+}