Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit f7c3b03

Browse filesBrowse files
jazellytargos
authored andcommitted
lib: propagate aborted state to dependent signals before firing events
PR-URL: #54826 Fixes: #54466 Fixes: #54601 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 341b6d9 commit f7c3b03
Copy full SHA for f7c3b03

File tree

Expand file treeCollapse file tree

7 files changed

+131
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+131
-6
lines changed
Open diff view settings
Collapse file

‎lib/internal/abort_controller.js‎

Copy file name to clipboardExpand all lines: lib/internal/abort_controller.js
+34-4Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
// in https://github.com/mysticatea/abort-controller (MIT license)
55

66
const {
7+
ArrayPrototypePush,
78
ObjectAssign,
89
ObjectDefineProperties,
910
ObjectDefineProperty,
11+
ObjectSetPrototypeOf,
1012
PromiseResolve,
1113
SafeFinalizationRegistry,
1214
SafeSet,
@@ -372,18 +374,46 @@ ObjectDefineProperty(AbortSignal.prototype, SymbolToStringTag, {
372374

373375
defineEventHandler(AbortSignal.prototype, 'abort');
374376

377+
// https://dom.spec.whatwg.org/#dom-abortsignal-abort
375378
function abortSignal(signal, reason) {
379+
// 1. If signal is aborted, then return.
376380
if (signal[kAborted]) return;
381+
382+
// 2. Set signal's abort reason to reason if it is given;
383+
// otherwise to a new "AbortError" DOMException.
377384
signal[kAborted] = true;
378385
signal[kReason] = reason;
386+
// 3. Let dependentSignalsToAbort be a new list.
387+
const dependentSignalsToAbort = ObjectSetPrototypeOf([], null);
388+
// 4. For each dependentSignal of signal's dependent signals:
389+
signal[kDependantSignals]?.forEach((s) => {
390+
const dependentSignal = s.deref();
391+
// 1. If dependentSignal is not aborted, then:
392+
if (dependentSignal && !dependentSignal[kAborted]) {
393+
// 1. Set dependentSignal's abort reason to signal's abort reason.
394+
dependentSignal[kReason] = reason;
395+
dependentSignal[kAborted] = true;
396+
// 2. Append dependentSignal to dependentSignalsToAbort.
397+
ArrayPrototypePush(dependentSignalsToAbort, dependentSignal);
398+
}
399+
});
400+
401+
// 5. Run the abort steps for signal
402+
runAbort(signal);
403+
// 6. For each dependentSignal of dependentSignalsToAbort,
404+
// run the abort steps for dependentSignal.
405+
for (let i = 0; i < dependentSignalsToAbort.length; i++) {
406+
const dependentSignal = dependentSignalsToAbort[i];
407+
runAbort(dependentSignal);
408+
}
409+
}
410+
411+
// To run the abort steps for an AbortSignal signal
412+
function runAbort(signal) {
379413
const event = new Event('abort', {
380414
[kTrustEvent]: true,
381415
});
382416
signal.dispatchEvent(event);
383-
signal[kDependantSignals]?.forEach((s) => {
384-
const signalRef = s.deref();
385-
if (signalRef) abortSignal(signalRef, reason);
386-
});
387417
}
388418

389419
class AbortController {
Collapse file

‎test/fixtures/wpt/README.md‎

Copy file name to clipboardExpand all lines: test/fixtures/wpt/README.md
+1-1Lines changed: 1 addition & 1 deletion
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ Last update:
1313
- common: https://github.com/web-platform-tests/wpt/tree/dbd648158d/common
1414
- compression: https://github.com/web-platform-tests/wpt/tree/5aa50dd415/compression
1515
- console: https://github.com/web-platform-tests/wpt/tree/767ae35464/console
16-
- dom/abort: https://github.com/web-platform-tests/wpt/tree/d1f1ecbd52/dom/abort
16+
- dom/abort: https://github.com/web-platform-tests/wpt/tree/0143fe244b/dom/abort
1717
- dom/events: https://github.com/web-platform-tests/wpt/tree/0a811c5161/dom/events
1818
- encoding: https://github.com/web-platform-tests/wpt/tree/5aa50dd415/encoding
1919
- fetch/data-urls/resources: https://github.com/web-platform-tests/wpt/tree/7c79d998ff/fetch/data-urls/resources
Collapse file
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
features:
2+
- name: aborting
3+
files: "**"
4+
- name: abortsignal-any
5+
files:
6+
- abort-signal-any.any.js
Collapse file
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE html>
2+
<html class=test-wait>
3+
<head>
4+
<title>AbortSignal::Any when source signal was garbage collected</title>
5+
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1908466">
6+
<link rel="author" title="Vincent Hilla" href="mailto:vhilla@mozilla.com">
7+
<script src="/common/gc.js"></script>
8+
</head>
9+
<body>
10+
<p>Test passes if the browser does not crash.</p>
11+
<script>
12+
async function test() {
13+
let controller = new AbortController();
14+
let signal = AbortSignal.any([controller.signal]);
15+
controller = undefined;
16+
await garbageCollect();
17+
AbortSignal.any([signal]);
18+
document.documentElement.classList.remove('test-wait');
19+
}
20+
test();
21+
</script>
22+
</body>
23+
</html>
Collapse file
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8">
3+
<script>
4+
let b;
5+
window.addEventListener("DOMContentLoaded", () => {
6+
let a = new AbortController()
7+
b = AbortSignal.any([a.signal])
8+
a.signal.addEventListener("abort", () => { AbortSignal.any([b]) }, { })
9+
a.abort(undefined)
10+
})
11+
</script>
Collapse file

‎test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js‎

Copy file name to clipboardExpand all lines: test/fixtures/wpt/dom/abort/resources/abort-signal-any-tests.js
+55Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,59 @@ function abortSignalAnyTests(signalInterface, controllerInterface) {
182182
controller.abort();
183183
assert_equals(result, "01234");
184184
}, `Abort events for ${desc} signals fire in the right order ${suffix}`);
185+
186+
test(t => {
187+
const controller = new controllerInterface();
188+
const signal1 = signalInterface.any([controller.signal]);
189+
const signal2 = signalInterface.any([signal1]);
190+
let eventFired = false;
191+
192+
controller.signal.addEventListener('abort', () => {
193+
const signal3 = signalInterface.any([signal2]);
194+
assert_true(controller.signal.aborted);
195+
assert_true(signal1.aborted);
196+
assert_true(signal2.aborted);
197+
assert_true(signal3.aborted);
198+
eventFired = true;
199+
});
200+
201+
controller.abort();
202+
assert_true(eventFired, "event fired");
203+
}, `Dependent signals for ${desc} are marked aborted before abort events fire ${suffix}`);
204+
205+
test(t => {
206+
const controller1 = new controllerInterface();
207+
const controller2 = new controllerInterface();
208+
const signal = signalInterface.any([controller1.signal, controller2.signal]);
209+
let count = 0;
210+
211+
controller1.signal.addEventListener('abort', () => {
212+
controller2.abort("reason 2");
213+
});
214+
215+
signal.addEventListener('abort', () => {
216+
count++;
217+
});
218+
219+
controller1.abort("reason 1");
220+
assert_equals(count, 1);
221+
assert_true(signal.aborted);
222+
assert_equals(signal.reason, "reason 1");
223+
}, `Dependent signals for ${desc} are aborted correctly for reentrant aborts ${suffix}`);
224+
225+
test(t => {
226+
const source = signalInterface.abort();
227+
const dependent = signalInterface.any([source]);
228+
assert_true(source.reason instanceof DOMException);
229+
assert_equals(source.reason, dependent.reason);
230+
}, `Dependent signals for ${desc} should use the same DOMException instance from the already aborted source signal ${suffix}`);
231+
232+
test(t => {
233+
const controller = new controllerInterface();
234+
const source = controller.signal;
235+
const dependent = signalInterface.any([source]);
236+
controller.abort();
237+
assert_true(source.reason instanceof DOMException);
238+
assert_equals(source.reason, dependent.reason);
239+
}, `Dependent signals for ${desc} should use the same DOMException instance from the source signal being aborted later ${suffix}`);
185240
}
Collapse file

‎test/fixtures/wpt/versions.json‎

Copy file name to clipboardExpand all lines: test/fixtures/wpt/versions.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"path": "console"
1313
},
1414
"dom/abort": {
15-
"commit": "d1f1ecbd52f2eab3b7fe5dc1b20b41174f1341ce",
15+
"commit": "0143fe244b3d622441717ce630e0114eb204f9a7",
1616
"path": "dom/abort"
1717
},
1818
"dom/events": {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.