Re: [whatwg/dom] Expose an `aborted` promise on AbortSignal? (#946)

> (I removed a || || from the code above, sorry if that's part of an upcoming proposal I'm not aware of)

No no that was just a typo ^^

> I'm really struggling to understand how the above works. I'm never the smartest developer in the room, but I think others would struggle with it too.

Basically there are two problems we've encountered with `AbortSignal`s:
 - It is very easy to forget to remove event listeners or to do so incorrectly.
 - The code that checks if a signal is aborted and then adds the listener is almost always repeated.

> In the above, assuming the reference that the body of aborted has to timer is weak, timer is out of reference once the promise returned by aborted(signal, timer) rejects, fulfills and its reaction callback is called, or the promise loses the ability to fulfill or reject due to other things going out of reference. 

Basically - in `aborted(signal, timer)` it adds a listener to the signal and binds its lifetime to the lifetime of the timer. There are three scenarios:

 - If the signal was already aborted, the promise fulfils immediately - the timer gets removed and the timer promise is rejected with abort error.
 - If `abort()` is called on the corresponding AbortController while the timer is still waiting to run - the promise fulfils and the timer gets cleared.
 - If however `timer` finishes cleanly and goes out of scope - the signal does not retain the timer preventing the memory leak and providing the ergonomic API.

> From other comments in the thread, it seems like finalization of the timer object is used in some way, but it seems circular. What am I missing?

```js
const retainerMap = new WeakMap();

function aborted(signal, retainer) {
  if (signal.aborted) return Promise.resolve();
  return new Promise(resolve => {
    // only hold the listener as long as "retainer" is alive, this means if retainer is not alive no one has a reference
    // to the resolve function and the promise can be GCd (and so can its then callbacks etc)
    // In node we implement this with a FinalizationRegistry and WeakRefs
    signal.addEventListener('abort', resolve, { magicInternalWeakOption: retainer });
    // explicitly make the retainer hold a reference to the resolve function so we have a strong reference to it 
    // and the lifetime of `resolve` is bound to `retainer` since it's the only one that holds it strongly
    // note that this is a weak map so `retainer` itself is held weakly here.
    retainerMap.set(retainer, resolve);
  });
}
```
----

I'd also like to emphasise I'm not fixated on _this particular solution_. I just brought it up as an idea and I'm not convinced myself it's a good one though I am tempted to implement as experimental and see what people say.

I figured that it should be a promise because that's our async primitive for "timing independent  multicast one-off things". I figured it needs a way to tie its lifetime to a resource since that's the bit everyone is doing manually in the meantime.

It's entirely possible there are much better ways to accomplish this I haven't thought of yet. I am in no hurry :]

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/946#issuecomment-773944739

Received on Friday, 5 February 2021 10:28:31 UTC