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

Remove is_winsock_handle() and instead test if wrapping the handle in a socket.socket() works #494

Merged
merged 2 commits into from
Mar 29, 2021

Conversation

ivanpauno
Copy link
Member

When investigating failures in the nightly windows debug job I was getting a lot of debug assertions dialogs about invalid file descriptions.
I don't think this was causing the CI failures, but it was pretty anoying when debugging locally.

Testing os.dup(os.close()) had a similar issue.
This seems to work fine 🤞 .

… a socket.socket()

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno added the enhancement New feature or request label Mar 26, 2021
@ivanpauno ivanpauno requested a review from hidmic March 26, 2021 19:14
@ivanpauno ivanpauno self-assigned this Mar 26, 2021
@ivanpauno ivanpauno changed the title Remove is_winsock_handle() and instead test if wrapping the handle in a socket.socket() Remove is_winsock_handle() and instead test if wrapping the handle in a socket.socket() works Mar 26, 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.

LGTM, pending green CI 🤞

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Copy link
Member Author

ivanpauno commented Mar 26, 2021

  • Windows Build Status (unrelated build warning)

@ivanpauno ivanpauno merged commit 24fa2df into master Mar 29, 2021
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/fix-debug-assertion-testing-winsock branch March 29, 2021 13:25
hidmic pushed a commit that referenced this pull request Jun 16, 2021
… a socket.socket() works (#494)

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
hidmic pushed a commit that referenced this pull request Jun 29, 2021
… a socket.socket() works (#494)

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
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 <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
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