Skip to content

Debounce message events - #1087

Merged
sindresorhus merged 1 commit into
mainfrom
ipc-debounce-send
May 23, 2024
Merged

sindresorhus merged 1 commit into
mainfrom
ipc-debounce-send

Conversation

@ehmicky

@ehmicky ehmicky commented May 23, 2024 •

Copy link
Copy Markdown
Collaborator

The following pattern has a race condition described in #1070:

await subprocess.sendMessage('ping');
const pong = await subprocess.getOneMessage();

That's because the subprocess might respond between those two calls, in which case subprocess.getOneMessage() hangs forever. In #1077, we introduced a subprocess.exchangeMessage() to fix this problem.

However, to avoid race conditions, users must remember to use that method. Also, there is a series of "Best practices" that we documented: in essence, there are many pitfalls to avoid to make sure IPC does not run into similar race conditions as above.

This PR implements a better solution to the underlying problem: it debounces IPC messages. Unlike the current solution, this does not require users to remember any pattern or best practice: it works automatically. This removes any race condition not only from the example above, but also from the following additional situations.

// Read single messages, multiple times in a row
while (/* ... */) {
  const message = await subprocess.getOneMessage();
  // ...
}
// Send multiple messages in a row, then read one message
await subprocess.sendMessage('foo');
await subprocess.sendMessage('bar');
const message = await subprocess.getOneMessage();

This also removes the need for the exchangeMessage() method, which simplifies the API. It was a little confusing whether this method sent then received a message, or received then sent a message. Since sending and receiving messages are now separate methods, this is much clearer.

This PR also adds lots of tests to cover several edge cases.

Finally, this implementation works better when mixing both Execa's IPC methods and the native Node.js IPC methods. Previously, mixing the two could lead to some issues.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants