Skip to content

revert IPC server change + fix pipe communication on Windows #11561

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

Merged
merged 2 commits into from
Aug 24, 2022

Conversation

paul-marechal
Copy link
Member

What it does

Revert #11530

Use overlapped instead of pipe when opening a process pipe for the bidirectional communication to work properly on all platforms.

How to test

File watching should work on every platform: Try renaming files, etc.

Review checklist

Reminder for reviewers

@paul-marechal
Copy link
Member Author

cc @tortmayr

@tortmayr
Copy link
Contributor

@paul-marechal Thanks for following up on that. Using overlapped instead of pipe seems to fix all Windows issues. Definitely much cleaner than the named pipe setup introduced with #11530. 👍

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me 👍 I can confirm that the application behaves as expected on Windows.

Node requires 'overlapped' on Windows when creating process pipes
for bidirectional communication to work reliably.
@paul-marechal paul-marechal merged commit 73260a1 into master Aug 24, 2022
@paul-marechal paul-marechal deleted the mp/overlapped branch August 24, 2022 18:06
@github-actions github-actions bot added this to the 1.29.0 milestone Aug 24, 2022
@vince-fugnitto vince-fugnitto added the messaging issues related to messaging label Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
messaging issues related to messaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants