sindresorhus / electron-better-ipc

Simplified IPC communication for Electron apps
MIT License
715 stars 60 forks source link

Overlapping calls seem to return the first available result #3

Closed DavidAnson closed 5 years ago

DavidAnson commented 6 years ago

By inspection, it seems like the following scenario will not work:

  1. main calls answerRenderer for channel "makeUpper"
  2. renderer calls callMain for channel "makeUpper" and data "aaa"
  3. renderer calls callMain for channel "makeUpper" and data "bbb"
  4. Callback for "makeUpper" of "aaa" returns "AAA"
  5. Both renderer calls to callMain receive this data (they are using the same channel)
  6. Code thinks makeUpper of "bbb" returned "AAA"

When I implemented something similar (before this module was available), I needed a unique identifier to correlate request/response and avoid this problem: https://github.com/DavidAnson/Lenz/blob/master/ipc.js

Might something like that be necessary here?

DavidAnson commented 6 years ago

I believe I've confirmed the issue above is present. Please have a look at the commits in the issue-3 branch of my fork: https://github.com/DavidAnson/electron-better-ipc/commits/issue-3

Output from my machine below. Note that the second call to callMain does not receive the same data it passed.

  Difference:

    [
      '',
  -   '"test:renderer:answer-from-main2: test:main:answer: optional-data"',
  +   '"test:renderer:answer-from-main2: test:main:answer: optional-data2"',
      '"test:renderer:answer-from-main: test:main:answer: optional-data"',
      '"test:renderer:data-from-main: optional-data"',
      'test:main:answer-from-renderer: test:renderer:answer-data: optional-data',
      'test:main:data-from-renderer: optional-data',
      'test:main:data-from-renderer: optional-data2',
    ]
k7sleeper commented 6 years ago

I can confirm this problem, too. It's not avoidable to include a synchronous answer by event.returnValue which contains a unique id of the call.

sindresorhus commented 6 years ago

I never considered multiple concurrent calls when I made this...

We need to add a unique ID, probably a UUID, to each event call.

DavidAnson commented 5 years ago

Thank you! I’ll try this soon. BTW, I don’t see any tests with the commit above - please feel free to use the code I wrote above to reproduce the problem!

sindresorhus commented 5 years ago

@DavidAnson Yup. Will do. Just didn't have time right now and thought it was more important to get the fix out. https://github.com/sindresorhus/electron-better-ipc/issues/12