Skip to content
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

Only try to wrap the fd in a socket on Windows #498

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

ivanpauno
Copy link
Member

After rereading #494 I noticed that we shouldn't be trying to wrap the fd in a socket on non-Windows platforms.

I'm not sure if that could cause a bug or not, but this is more clean.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added the enhancement New feature or request label Mar 30, 2021
@ivanpauno ivanpauno requested a review from hidmic March 30, 2021 18:08
@ivanpauno ivanpauno self-assigned this Mar 30, 2021
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

Before #494, socket wrapping could only happen on Windows. Perhaps that's why it was overlooked.

Anyhow, LGTM (pending green CI)!

@ivanpauno
Copy link
Member Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ivanpauno ivanpauno merged commit b59bb6b into master Mar 30, 2021
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/dont-try-to-wrap-fd-in-socket-on-linux branch March 30, 2021 20:22
hidmic pushed a commit that referenced this pull request Jun 16, 2021
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
hidmic pushed a commit that referenced this pull request Jun 29, 2021
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
hidmic pushed a commit that referenced this pull request Jul 7, 2021
* Handle signals within the asyncio loop. (#476)
* Workaround asyncio signal handling on Unix (#479)
* Remove is_winsock_handle() and instead test if wrapping the handle in a socket.socket() works (#494)
* Close the socket pair used for signal management (#497)
* Only try to wrap the fd in a socket on Windows (#498)
* Bring back deprecated launch.utilities.signal_management APIs

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants