Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbortSignal "signal abort" ordering #493

Closed
jakearchibald opened this issue Aug 14, 2017 · 5 comments
Closed

AbortSignal "signal abort" ordering #493

jakearchibald opened this issue Aug 14, 2017 · 5 comments

Comments

@jakearchibald
Copy link
Collaborator

jakearchibald commented Aug 14, 2017

https://dom.spec.whatwg.org/#abortsignal-signal-abort

Currently the order is:

  1. If signal’s aborted flag is set, then return.
  2. Set signal’s aborted flag.
  3. For each algorithm in signal’s abort algorithms: run algorithm.
  4. Empty signal’s abort algorithms.
  5. Fire an event named abort at signal.

If you want to create a new signal that copies another signal, you do something like this:

  1. Let originalSignal be a signal we got from somewhere.
  2. Let signal be a new AbortSignal.
  3. Add the following steps to originalSignal:
    1. Signal abort on signal.
  4. Return signal.

A side effect of this, is the "abort" event will despatch on signal before originalSignal.

const controller = new AbortController();
const signal = controller.signal;
const request = new Request('.', {signal});
const requestClone = request.clone(); // this copies the signal as above.

request.signal.addEventListener('abort', () => console.log('original'));
requestClone.signal.addEventListener('abort', () => console.log('clone'));

controller.abort();

In the code above, "clone" is logged before "original". Is this a problem?

cc @domenic @annevk @wycats @mikewest @bterlson @jyasskin

@jakearchibald
Copy link
Collaborator Author

On IRC @mikewest had minor concerns about the ordering, but "WebAuthn will be fine either way".

@domenic
Copy link
Member

domenic commented Aug 14, 2017

To me the basic problem here is that we're using the abort algorithms as pseudo-event-handlers. What you would ideally like is for them to be interleaved, I think.

Unless we're interested in creating an "abstract event listeners" concept, I'm not sure this is worth solving.

@mikewest
Copy link
Member

For clarity: I'm fine with either outcome. It doesn't seem to matter for the use cases I have in mind. It does seem like it would be best to agree upon behavior, but as long as there's agreement, I'm happy.

@annevk
Copy link
Member

annevk commented Dec 20, 2017

There is agreement in the standard and in the tests. It's just unclear whether there's some scenario not considered that would be better served by different ordering. Given the length of time this has been open however, I'd encourage @jakearchibald to consider it closed (and close it) as it seems unlikely new information will come forward. If there's any fallout later on we can reconsider at that point.

@jakearchibald
Copy link
Collaborator Author

Agreed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants