Add abortSignal.addAbortCallback
This PR is a discussion starter. The goals:
- Make progress on allowing TC39 able to adopt/use
AbortControllerandAbortSignalwithout requiringEventandEventTarget. - Provide a system where abort 'listeners' can be GC'd as soon as abort has happened.
The behaviour in this PR means that abort reactions in userland and the web platform interleave. @bakkot is keen on this. I'm a little unsure, so I'm interested in more opinions.
I'm a little worried about:
const controller = new AbortController();
const signal = controller.signal;
let thing;
signal.addAbortCallback(() => {
console.log(thing.running);
// The above is unexpectedly true,
// because the platform's steps to set it to false haven't run yet.
});
thing = doThing(signal);
controller.abort();
But maybe this already happens with promises?
- [ ] At least two implementers are interested (and none opposed):
- …
- …
- [ ] Tests are written and can be reviewed and commented upon at:
- …
- [ ] Implementation bugs are filed:
- Chromium: …
- Gecko: …
- WebKit: …
- Deno (only for aborting and events): …
- Node.js (only for aborting and events): …
- [ ] MDN issue is filed: …
- [ ] The top of this comment includes a clear commit message to use.
(See WHATWG Working Mode: Changes for more details.)
With regards to the example, the way a user is actually likely to run into this is more like
import { operation } from 'some-framework';
async function whatever(signal) {
await Promise.all([
builtin1({ signal }),
operation({ signal }),
builtin2({ signal }),
]);
}
From the user's perspective, it is very strange if these are aborted in anything other than LIFO or FIFO order. Users are not (and should not be) reasoning about whether the operations they're calling are defined in their framework or in the platform, and there is no way to understand the builtin1, builtin2, operation order without such reasoning.
That example is pretty compelling.
I'd like the design to allow #1389 in the future 👀 (doesn't seem to block it right now, which is cool)
From a language-level perspective, I think it's better if we defined the mechanism as a cancelation/abort protocol in the language... of which AbortSignal could be update to be / considered an implementation.
Specifically, I've been imagining something along the lines of...
const genericAbortSignal = {
[Symbol.onabort]() {
// Register and return the abort handle
return {
cancel() { /* Unregister the abort handler */ },
[Symbol.dispose]() { this.cancel(); }
};
},
};
doSomething(genericAbortSignal);
async function doSomething(signal) {
using abortHandle = signal[Symbol.onabort]();
abortHandle.onabort = (reason) => { /* ... */ };
// Do async stuff that can be aborted...
}
Then, the actual AbortSignal class here could just add [Symbol.addAbortHandler] such that it defers to addEventListener.
This type of approach would allow any object to become an AbortSignal. It would allow TC39 to define abort semantic without relying on AbortSignal, AbortController, Event, or EventTarget.
The requirement of the protocol would be that a call to Symbol.onabort must return an object that minimally must expose a cancel() method that would unregister the handler from the signal.
AbortSignal could easily be adapted to support this:
class AbortSignal extends EventTarget {
// ...
[Symbol.onabort]() {
let callback;
const self = this;
const handler = {
cancel() {
self.removeEventListener('abort', callback);
};
};
callback = (event) => handler.onabort?.(event.reason);
this.addEventListener('abort', callback, { once: true });
return handler;
}
}
@jasnell There's no need to define a symbol-based protocol. The protocol for consumers can literally be "call .addAbortCallback(yourCleanupFunction)". That requires no other changes from WHATWG.
(Though I agree that a removeAbortCallback(yourCleanupFunction), or similar mechanism, would also be nice.)
Edit: also, your implementation ends up still using the addEventListener; the primary purpose of this PR is to give us a path to avoid that machinery, because of the various problems with it.
... also, your implementation ends up still using the addEventListener
Only within a hypothetical impl of the abort protocol in the existing AbortSignal machinery. That would not be an actual requirement tho. I showed that only for illustration.
I think the nice thing about protocol approach is that it would not require use of the AbortSignal class at all. From the languages point of view, an "abort signal" can be any object that happens to implement @@Symbol.onabort (or whatever we want to call it). Exactly how that is implemented is left entirely open.
Calling .addAbortCallback(yourCleanupFunction) works just as well as using a symbol; arbitrary objects can have a method named "addAbortCallback". There's no need to introduce a new symbol here.
The main reasons to use a symbol-based rather than string-based protocol are when this is a protocol that arbitrary objects might implement (like Iterator), or when you want to switch on the presence of the protocol (like Promise). Neither holds here.
Symbol vs Not-a-symbol isn't that important to me. I'm good with either. More focused on the overall pattern than the particular color of the bikeshed ;-)
So the only difference between what you're proposing and what's in this PR already is just that instead of the new method returning undefined it would return an object with a cancel method which removes the listener?
That and we'd be sure to define it (in the language) as a protocol that is not dependent specifically on using AbortSignal/AbortController. I want to make certain that it is allowable that !(signal instanceof AbortSignal but it would still work...
that is, I want us to say, "An abort signal is any object that implements [...] function" ... as opposed to "An abort signal is always an AbortSignal as defined by WHATWG"... hopefully that make sense.
Yes, the idea is that ECMAScript would literally use the new method added in this PR (i.e., it will just attempt to call addAbortCallback on whatever object is passed).
If ECMAScript was going to reach into the internals of AbortSignal there would be no need for the new method.
addAbortCallback returning a disposable (Symbol.dispose) sounds useful to me - or are there concerns about always allocating an object that might only sometimes be used?
or are there concerns about always allocating an object that might only sometimes be used?
It's pretty easy for an engine to know when this expression is in a position where the return value is not used.