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

Allow postMessage-ing AbortSignal #948

Open
Jamesernator opened this issue Feb 11, 2021 · 17 comments
Open

Allow postMessage-ing AbortSignal #948

Jamesernator opened this issue Feb 11, 2021 · 17 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal

Comments

@Jamesernator
Copy link

Currently communicating to a worker that work should be aborted is somewhat annoying. This is especially true for work that may be run in a synchronous loop as one can not simply send forward a MessageChannel that signals aborting, but rather you need to send a SharedArrayBuffer that can be synchronously read.

I'd like to propose the ability to .postMessage an AbortSignal so that we can communicate aborting to workers in a consistent way to aborting on the main thread. Ideally this would support the synchronous case as well (e.g. calling controller.abort() from the original thread would synchronously update signal.aborted in the other thread so that compute loops can simply do if (signal.aborted) { /* terminate work */ }).

If there's interest, I would be willing to create a PR for this.

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal labels Feb 11, 2021
@annevk
Copy link
Member

annevk commented Feb 11, 2021

Wouldn't that mean that signal.aborted !== signal.aborted sometimes? I don't think we want that.

@Jamesernator
Copy link
Author

Jamesernator commented Feb 11, 2021

Wouldn't that mean that signal.aborted !== signal.aborted sometimes?

Yes. Although once true, always true.

I don't think we want that.

Is there any particular reason? Web APIs that accept AbortSignal could already observe such behaviour if they wanted to, this would simply allow work in other userland threads (workers) to be able to behave similarly. The spec already explictly allows this behaviour for such Web APIs (see note below remove abort signal).

And from what I've found, doing a macrotask to allow any asynchronous notifications to come through actually kills performance quite heavily in some cases (compared to Atomics.load()). Although this concern could be separately addressed by an API that allows fast checking for any pending worker messages (e.g. if (self.areMessagesPending) { await self.flushMessages(); }).

@domenic
Copy link
Member

domenic commented Feb 11, 2021

Yeah, breaking signal.aborted !== signal.aborted is not something we do on the web platform. It violates the run to completion semi-invariant, which can currently only be violated by the very special object SharedArrayBuffer.

@domenic
Copy link
Member

domenic commented Feb 11, 2021

Web APIs that accept AbortSignal could already observe such behaviour if they wanted to

That isn't true? aborted can only be observed on the main thread, and if author JavaScript is running on the main thread, then web API browser code cannot be.

@Jamesernator
Copy link
Author

Jamesernator commented Feb 11, 2021

That isn't true? aborted can only be observed on the main thread, and if author JavaScript is running on the main thread, then web API browser code cannot be.

I mean they don't necessarily go via .aborted, but there's this note that implies that they can observe it basically anytime they need:

The abort algorithms enable APIs with complex requirements to react in a reasonable way to abort(). For example, a given API’s aborted flag might need to be propagated to a cross-thread environment, such as a service worker.

It violates the run to completion semi-invariant, which can currently only be violated by the very special object SharedArrayBuffer.

Yes this is what I'm currently using for aborting in compute loops in workers, but it is a bit annoying not to be able to use the same API on one side that is used throughout the rest of the code.

Also my proposal doesn't actually violate run to completion, as it doesn't allow any additional JS code to run at the same time, in this regard is isn't significantly different to Date.now() !== Date.now() or Math.random() !== Math.random() or the recently shipped in Chrome navigator.scheduling.isInputPending() !== navigator.scheduling.isInputPending().

@Jamesernator
Copy link
Author

Jamesernator commented Feb 28, 2021

Just to clarify how I'd imagine this working, if AbortSignal were implemented in js I would imagine doing something like this.

I've added an explicit synchronizeAborted method to this example, rather than having .aborted do this implictly (previously I was thinking .aborted getter would trigger .synchronizationSteps(), but it adds work for code paths that don't need it).

// Structured serialize steps
function serializeAbortSignal(abortSignal) {
  if (abortSignal.aborted) {
    return { type: "AbortSignal", aborted: true };
  } else {
    const messageChannel = new MessageChannel();
    abortSignal.addEventListener("abort", () => {
      // Update so aborted can be observed synchronously
      Atomics.store(sharedAbort, 0, 1));
      // And send it in case it's listening asynchronously
      messageChannel.port1.postMessage("aborted");
      messageChannel.port1.close();
    });
    const sharedAborted = new Uint8Array(new SharedArrayBuffer(1));
    return { 
      type: "AbortSignal", 
      aborted: false, 
      abortPort: messageChannel.port2,
      sharedAborted,
    };
  }
}

// Structured deserialize steps
function deserializeAbortSignal(serializedAbortSignal) {
  const abortSignal = new AbortSignal();
  
  if (serializedAbortSignal.aborted) {
    abortSignal.[[aborted]] = true;
    return abortSignal;
  }
  
  const { abortPort, sharedAborted } = serializedAbortSignal;
  // When a request to synchronize abort state with the main thread
  // happens we need to potentially fire "abort", and return 
  abortSignal.[[synchronizeSteps]] = () => {
    if (Atomics.load(sharedAborted, 0)) { // If aborted is set
      // Set the aborted flag synchronously
      abortSignal.[[aborted]] = true;
      // Fire the abort event and run any handlers synchronously
      abortSignal.dispatchEvent(new Event("abort"));
      abortPort.close();
    }
  };
  
  // Abort asynchronously if synchronizeAbort is not used
  abortPort.addEventListener("message", () => {
    if (abortSignal.[[aborted]]) return;
    abortSignal.[[aborted]] = true;
    abortSignal.dispatchEvent(new Event("abort"));
    abortPort.close();
  });
  
  return abortSignal;
}

class AbortSignal extends EventTarget {
  [[aborted]] = false;
  [[synchronizeSteps]] = () => void;

  get aborted() {
    return this.[[aborted]];
  }
  
  // This asks the abort signal to synchronize with the other thread
  // by default the synchronize steps are to do nothing, this is the
  // case today and would remain the case when the abort signal does
  // not come from another thread
  synchronizeAborted() {
    if (this.[[aborted]]) {
      return true;
    }
    this.[[synchronizeSteps]]();
    return this.[[aborted]];
  }
}
// inside a worker

self.addEventListener("message", (evt) => {
  const { abortSignal } = evt.data;
  while (true) {
    // Synchronize abortSignal with the main thread and return true if aborted
    if (abortSignal.synchronizeAborted()) { 
      break;
    }
    // ...heavy compute...
  }
});

@jasnell
Copy link
Contributor

jasnell commented Mar 9, 2021

Just to offer a different perspective on this... in Node.js (which is, of course, a different environment than browsers) we have more of a notion of objects that can be shared across workers -- much like SharedArrayBuffer but for other host objects we provide. In that environment, we actually could actually have two different AbortSignal objects (one in each thread) sharing the same underlying backend such that signal.aborted === signal.aborted. (It's not implemented this way today... but we could).

@Jamesernator
Copy link
Author

With the new abortSignal.reason synchronous observation doesn't really make sense anymore as one also needs to send the reason across the thread which is considerably less simple than the setting of a boolean cross-thread, however the asynchronous part of the API still would make sense.

For this we could have a design more like:

AbortSignal serialization steps given _abortSignal_ and _serialized_:

1. If _abortSignal_ is **aborted**.
  a. Set _serialized_.[[AbortReason]] to _abortSignal_'s **abort reason**.
  b. Set _serialized_.[[MessagePort]] to **undefined**.
2. Else.
  a. Let _port1_ be a new MessagePort in the current realm.
  b. Let _port2_ be a new MessagePort in the current realm.
  c. Entangle _port1_ and _port2_.
  d. Set _serialized_.[[AbortReason]] to **undefined**.
  e. Set _serialized_.[[MessagePort]] to the **sub-serialization** of **port2**.
  f. Add the following **abort algorithm** to _abortSignal_:
    a. Post a message consisting of _abortSignal_'s **abort reason** to _port1_.
    b. Disentangle _port1_.
    
AbortSignal deserialization steps given _dataHolder_ and _value_:

1. If _dataHolder_.[[AbortReason]] is not **undefined**:
  a. Abort _value_ with _dataHolder_.[[AbortReason]].
2. Else.
  a. Assert _serialized_.[[MessagePort]] is a MessagePort.
  b. Add a handler for _serialized_.[[MessagePort]]'s "message" event with the following steps:
    i. **abort** _value_ with the *data* of the message.
    ii. Disentangle _serialized_.[[MessagePort]]

@annevk
Copy link
Member

annevk commented Nov 30, 2021

That seems reasonable to me and is compatible with what we are planning to do for Fetch (see whatwg/fetch#1343). (The current plan there is to serialize/deserialize abort reason and recreate the AbortSignal as we already have a channel with the service worker, but it's not distinguishable from the above I think.)

@jasnell
Copy link
Contributor

jasnell commented Nov 30, 2021

There is a challenge here since given that the structured clone algorithm doesn't handle custom Error objects well without losing data. Some platforms that might not be able to handle DOMException as a host object, for instance, might not be able to deserialize it as a DOMException on the receiving end. And if I set reason to an Error subclass (e.g. class FooError extends Error), it will come across as just an Error on the other side. That's not to say we should block making AbortSignal transferable, only that we need to acknowledge the limitations and lack of fidelity. (I believe we already have the same issue with communicating errors in the streams impl so it's not a new issue)

@Jamesernator
Copy link
Author

Jamesernator commented Dec 1, 2021

That seems reasonable to me and is compatible with what we are planning to do for Fetch (see whatwg/fetch#1343). (The current plan there is to serialize/deserialize abort reason and recreate the AbortSignal as we already have a channel with the service worker, but it's not distinguishable from the above I think.)

I'm happy to make a PR if you want.

There is a challenge here since given that the structured clone algorithm doesn't handle custom Error objects well without losing data. Some platforms that might not be able to handle DOMException as a host object, for instance, might not be able to deserialize it as a DOMException on the receiving end.

This is part of the wider issue of not having any userland way of having custom structured serialized things. How do existing structured clonables/transferables like MessagePort work in Node?

Nowadays it might be more viable to have userland structured cloning by just pointing to an ES module for deserialization. With module blocks the ergonomics of it could even be rather nice as you could just return a module block from some hypothetical serialize method. This is just an idea on my part though and is well beyond the scope of this issue though.

@jasnell
Copy link
Contributor

jasnell commented Dec 1, 2021

In node.js we implement a custom value serializer delegate that allows us to serialize and deserialize host objects. The key challenge is that those have to be backed in the prototype chain by an underlying native c++ object. Today I implemented a new version of DOMException for node.js that uses this mechanism and it works well. But, it would be generally better to have a way of supporting custom userland serialization.

jasnell added a commit to nodejs/node that referenced this issue Dec 11, 2021
Allows for using `AbortSignal` across worker threads and contexts.

```js
const ac = new AbortController();
const mc = new MessageChannel();
mc.port1.onmessage = ({ data }) => {
  data.addEventListener('abort', () => {
    console.log('aborted!');
  });
};
mc.port2.postMessage(ac.signal, [ac.signal]);
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #41050
Refs: whatwg/dom#948
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Dec 14, 2021
Allows for using `AbortSignal` across worker threads and contexts.

```js
const ac = new AbortController();
const mc = new MessageChannel();
mc.port1.onmessage = ({ data }) => {
  data.addEventListener('abort', () => {
    console.log('aborted!');
  });
};
mc.port2.postMessage(ac.signal, [ac.signal]);
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #41050
Refs: whatwg/dom#948
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
@jasnell
Copy link
Contributor

jasnell commented Dec 18, 2021

Just an update on this... we recently just landed early support for this in Node.js. When sending an AbortSignal over postMessage, a new AbortSignal is created on the receiving end connected to the original via a MessagePort. When the original signal is triggered, it has an abort algorithms that sends a message over the MessagePort that triggers the other. Very simple and straightforward. The key caveat is that there is some latency between when the original AbortSignal is triggered and when the newly created one is triggered due to the message passing infrastructure (in Node.js there's one event loop turn required)

@Jamesernator
Copy link
Author

Just an update on this... we recently just landed early support for this in Node.js. When sending an AbortSignal over postMessage, a new AbortSignal is created on the receiving end connected to the original via a MessagePort. When the original signal is triggered, it has an abort algorithms that sends a message over the MessagePort that triggers the other. Very simple and straightforward.

This is essentially what I'd capture a PR, although I do notice the Node implementation has AbortSignal as a [Transferable] due to some technical limitations, my intent was just to make AbortSignal serializable as the AbortSignal will still be perfectly usable on the original thread (as it is in Node anyway) which doesn't really feel like a "transferable". Do you think this would be a problem for Node to change later?

The key caveat is that there is some latency between when the original AbortSignal is triggered and when the newly created one is triggered due to the message passing infrastructure (in Node.js there's one event loop turn required)

This seems acceptable as communication with a worker (other than SharedArrayBuffer) always requires sending a message anyway.

@jasnell
Copy link
Contributor

jasnell commented Dec 19, 2021 via email

danielleadams pushed a commit to nodejs/node that referenced this issue Jan 31, 2022
Allows for using `AbortSignal` across worker threads and contexts.

```js
const ac = new AbortController();
const mc = new MessageChannel();
mc.port1.onmessage = ({ data }) => {
  data.addEventListener('abort', () => {
    console.log('aborted!');
  });
};
mc.port2.postMessage(ac.signal, [ac.signal]);
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #41050
Refs: whatwg/dom#948
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Jan 31, 2022
Allows for using `AbortSignal` across worker threads and contexts.

```js
const ac = new AbortController();
const mc = new MessageChannel();
mc.port1.onmessage = ({ data }) => {
  data.addEventListener('abort', () => {
    console.log('aborted!');
  });
};
mc.port2.postMessage(ac.signal, [ac.signal]);
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #41050
Refs: whatwg/dom#948
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this issue Jan 31, 2022
Allows for using `AbortSignal` across worker threads and contexts.

```js
const ac = new AbortController();
const mc = new MessageChannel();
mc.port1.onmessage = ({ data }) => {
  data.addEventListener('abort', () => {
    console.log('aborted!');
  });
};
mc.port2.postMessage(ac.signal, [ac.signal]);
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#41050
Refs: whatwg/dom#948
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Feb 1, 2022
Allows for using `AbortSignal` across worker threads and contexts.

```js
const ac = new AbortController();
const mc = new MessageChannel();
mc.port1.onmessage = ({ data }) => {
  data.addEventListener('abort', () => {
    console.log('aborted!');
  });
};
mc.port2.postMessage(ac.signal, [ac.signal]);
```

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #41050
Refs: whatwg/dom#948
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
@uasan
Copy link

uasan commented Jun 20, 2023

in the documentation of Node.js and HTML, the interface for transmitting a signal to the worker is not described, is this a problem with the documentation or the implementation yet?

@Jamesernator
Copy link
Author

and HTML, the interface for transmitting a signal to the worker is not described

This is still just a suggestion, no browsers have implemented this and there is no spec.

in the documentation of Node.js

Node.js implements something similar to the suggestion, which is documented.

shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 18, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 19, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
shirakaba added a commit to shirakaba/dom-events-wintercg that referenced this issue Mar 20, 2024
Seems it's a non-standard feature. Dropping because it would still have required users to do extra work like providing their own MessageChannel implementation to override the included one.

It's a feature that requires runtime integration, so I can only provide a basis and not a full implementation.

whatwg/dom#948
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal
Development

No branches or pull requests

5 participants