comlink-extension icon indicating copy to clipboard operation
comlink-extension copied to clipboard

Issues when awaiting proxy methods simultaneously

Open arseneyr opened this issue 5 years ago • 2 comments

I'm having issues when calling two methods on the same proxied object and waiting on the results simultaneously. I have a minimal repro here (run yarn start and load the extension in the build folder) but here is the gist of it:

//background script

import * as Comlink from "comlink";
import { createBackgroundEndpoint, isMessagePort } from "comlink-extension";
import { browser } from "webextension-polyfill-ts";

class BackgroundEndpoint {
  getSubProxy() {
    return Comlink.proxy({
      getB: () => {
        return "B";
      },
    });
  }

  getA() {
    return "A";
  }
}

browser.runtime.onConnect.addListener((port) => {
  if (isMessagePort(port)) {
    return;
  }
  Comlink.expose(new BackgroundEndpoint(), createBackgroundEndpoint(port));
});

//content script

import { createEndpoint } from "comlink-extension";
import * as Comlink from "comlink";
import { browser } from "webextension-polyfill-ts";

const obj = Comlink.wrap(createEndpoint(browser.runtime.connect()));

obj
  .getSubProxy()
  .then((s) => s.getB())
  // s.getB() never resolves and "B" is not logged
  .then(console.log);

obj.getA().then(console.log);

The promise returned by s.getB() never resolves. The interesting thing is that changing the order of the promises (i.e. calling getA() before getSubProxy()) works fine. Also this is not a problem when using Comlink and web workers directly. Am I doing something obviously wrong here?

arseneyr avatar Jul 29 '20 00:07 arseneyr

Should've patched all the port creations, but maybe I left something out

If you want to debug, I'd log the messages that are sent and that should reveal more

samdenty avatar Jul 29 '20 01:07 samdenty

I believe this is a race condition in forward():

https://github.com/samdenty/comlink-extension/blob/c909591390659683f8bff105bfa9894a34964866/src/adapter.ts#L150-L152

Specifically, the port promise can resolve after a message has already been received and is lost. I'm not too familiar with Javascript internals but I assume it's an implementation detail whether the promise is resolved first or the message is processed.

I've opened #3 to make forward() sync and callers to use callbacks instead.

arseneyr avatar Jul 29 '20 06:07 arseneyr