GoogleChromeLabs / comlink

Comlink makes WebWorkers enjoyable.
Apache License 2.0
11.26k stars 386 forks source link

Expose and wrap a worker at once #425

Open KwintenP opened 4 years ago

KwintenP commented 4 years ago

Hi,

I am trying to create two-way communication between two web workers, both using 'Comlink Proxy's using a single MessageChannel.

Let's say that I have a worker running. At some point, I want to spin up a new instance of the same worker and want them to be able to communicate with each other. Since it's just a new instance, they have the exact same API. My idea was to create a MessageChannel in the main thread and pass a port to the two workers. They could then both Comlink.wrap on that port to call the other worker and also expose themselves to the same port.

worker.js

// called by main thread
connectToWorker: (port, expose, id) => {
        Comlink.expose(workerAPI, port);

        // Wrap around the port so I can send a message to the other worker
        otherWorkerProxy = Comlink.wrap(port);
        otherWorkerProxy.foo(id, 'Hi');
    },

This fails however with the following message:

comlink.js:123 Uncaught (in promise) DOMException: Failed to execute 'postMessage' on 'MessagePort': (workerId, message) => {
        console.log(`Message from ${workerId}: ${message}`)
    } could not be cloned.
    at https://unpkg.com/comlink@4.2.0/dist/umd/comlink.js:123:18

I took a little time into investigating why this happens and it actually makes sense. If you call expose, you register an eventListener for message on the port. If you call a function on a proxy in the worker, it also registers a listener for message. If I do both of these actions in the same worker, this means, that there are two listeners, one because of expose and one cause we just called a function on the proxy (which uses the same MessagePort). If the result of calling that function arrives, it will be passed to the message listener from expose. Which doesn't handle this properly and eventually results in this error.

I am aware that you can register callbacks from one worker to the other to make this work. But, since both of my workers are the exact same js file, I'd prefer to not do that. Looking at the code, it should actually be fairly simple to fix. Make the expose message handler ignore those messages if needed. I'd be more than happy to create a PR for this.

Is there an easier way to do this? Am I missing something? If not, would you consider a PR for this.

I made a repro where you can see this right here: https://github.com/KwintenP/comlink-worker-repro. Comment out line 23 and in 25 from index.js to see the error.

Thanks!

surma commented 4 years ago

I need to dig deeper to be 100% sure about this, and I currently sadly don’t have the time to do so, but...

I think the problem arises from the fact that Comlink’s RPC protocol does not handle duplex communication. I implemented having the worker use-case in mind. You have a communication channel, and expose something on one side, and call it from the other side. You want to expose something from both sides and call the other from both sides. I believe this gets the protocol tripped up.

I’d recommend just transferring to MessageChannels, one for each direction of communication.

If you can find a fix for this without increasing the size of Comlink too much, I am happy to review and accept PRs!

FilipeCosta06 commented 4 years ago

Hello,

sorry for hijacking this thread but I'm trying to find out if it is possible to use postMessage from "comlinked" Worker in order to notify the main page (React app) halfway through a function (actually, it is to notify that some of the operation failed but I don't want to change my interface just to return something like {successes: [], failures: []}).

However, from reading this leads me to believe it is not actually possible.

I implemented having the worker use-case in mind. You have a communication channel, and expose something on one side, and call it from the other side.

So, is it possible to send a message from the Worker to the main page "before" a function's return ?

PS: I didn't know MessageChannels were a thing, I'll check it out right after I submit this comment.

sivayapps commented 3 years ago

I had achieved this with SahredWorker side: expose a subscribe method which can take a callback function,


class SharedWorkerMessageChannel {
  clientId;
  callback;
constructor(clientId) {
this.clientId =clientId;
}
  subscribe( callbackFunction) {
    callback = callbackFunction;
  }
}

onconnect = connectEvent => {
  clientIdGenerator++;

  const swChannel = new SharedWorkerMessageChannel(clientIdGenerator);

//  subscriptionRegistry.registerClient(swChannel); //keep the swChannel to communicate with the page, without Comlink we used to store sharedWorker.port and use postMessage, now we can use the registered callback function to call the actual function of the page side

  Comlink.expose(wsChannel, port);
}

On Page:

sharedWorker = new SharedWorker('../../assets/js/shared.worker.js', 'WS_SW');
sharedWorkerMessageChannel = Comlink.wrap(this.sharedWorker.port);

call the exposed Callback function(subscribe) in this case by passing the callback function wrapped in Comlink.proxy(function...)

sharedWorkerMessageChannel.subscribe(Comlink.proxy((msg) => {
      console.log(`Received callback with Msg: ${msg}`);
    }));

Full Angular 12 example at: https://github.com/sivayapps/ws-lib Still working on this to extract SahredWorker interaction to service and wanted to create this as a Angualr library module for STOMP WebSocket on SharedWorker to reduce number of connections from each user. Currently trying to detect SharedWorker termination and reload sharedWorker from page.

lionel- commented 2 years ago

Duplicate of #474, fixed by #475?

BrianHung commented 9 months ago

This seems like a possible solution

https://github.com/GoogleChromeLabs/comlink/issues/450#issuecomment-827503286

I’d recommend just transferring to MessageChannels, one for each direction of communication.

Does anyone know if we can use BroadcastChannels to avoid having to setup n^2 connections between all n workers?