AbortSignal.any() assertion failure

[vinhill]

What is the issue with the DOM Standard?

In Gecko Bug 1903676, a test case was found where the assertion in create a dependent abort signal step 4.2.1. does not hold. I believe this is a spec issue.

Here is the example

let a = new AbortController();
let b = AbortSignal.any([a.signal]);
a.signal.addEventListener("abort", () => {
  AbortSignal.any([b]);
  console.log(b.aborted, a.signal.aborted);  // false, true
})
a.abort();

During the abort event of a, we create a dependent signal from b, which is not yet aborted. If b is not aborted and dependent on a, the spec expects a to be not aborted too and asserts this. The problem is that signal abort fires the event before aborting the dependent signals, steps 5 and 6 of signal abort could be switched to resolve this issue.

@shaseley

[vinhill]

I can get a PR up for this, the wpt "Abort events for AbortSignal.any() signals fire in the right order (using AbortController)" probably would require tweaking. The abort algorithm was introduced in https://github.com/whatwg/dom/pull/1152

[shaseley]

Thanks, yes that's indeed a problem. I confirmed we hit the same assert in Chromium.

Re: moving the steps around, I think step 6 in "signal abort" might need to go above step 3 though, since running the abort algorithms can invoke JS, which could also call AbortSignal.any() and hit that assert (i.e the problem is running any JS between setting the source's abort reason and dependents')?

If we wanted to maintain the same order, we'd need to change "create a dependent abort signal" to check for the aborted state of the dependent signals' source signals, but then you could get cases where AbortSignal.any([b]) returns an aborted signal even though b isn't aborted yet. I guess you could also propagate the aborted state separately first, but that's a lot more complexity than it's probably worth.

[shaseley]

On second thought, I don't think reordering would work. You could still have situations where the assert fails, e.g. by modifying the test case as follows:

<script>

let a = new AbortController();
let b = AbortSignal.any([a.signal]);
let c = AbortSignal.any([b]);
let d;
b.addEventListener("abort", () => {
  d = AbortSignal.any([c]);
  console.log(b.aborted, a.signal.aborted, c.aborted, d.aborted);
})
a.abort();
console.log(b.aborted, a.signal.aborted, c.aborted, d.aborted);

</script>

Maybe it wouldn't be too bad to split out propagating the abort reason and running abort algorithms/firing events so that all dependents have their aborted state atomically updated.

[shaseley]

Potential fix: https://github.com/whatwg/dom/pull/1295