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
multi: implement wait using winsock #5397
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still failing test 1564 on Azure Pipelines CI. Please make sure that test works, thanks.
WSACreateEvent and other related functions are only available from Windows 8.1. |
Sorry, what? The docs indicate that these functions were available at least as far back as Vista, and as far as I can tell they were also available way back in NT4. |
The functions are indeed available since Windows 95 / NT4. The documentation only mentions Windows 8.1 and Windows Vista due to the introduction of Universal Windows Platform apps and the end of support for Windows 7. Microsoft has removed the information about older Windows versions from their updated documentation, which is very unfortunate. |
Sry, I was just confused by MSDN docs then, it used to tell the minimum version exactly. |
0fa9ac1
to
3e46eda
Compare
Restructured to just reuse 1 event object, which also cuts down on allocations quite a lot. Tests are now passing. |
This avoids using a pair of TCP ports to provide wakeup functionality for every multi instance on Windows, where socketpair() is emulated using a TCP socket on loopback.
Thanks a lot! I merged this since I consider this a bugfix which should be part of this release. |
This avoids using a pair of TCP ports to provide wakeup functionality for every multi instance on Windows, where socketpair() is emulated using a TCP socket on loopback which could in turn lead to socket resource exhaustion. A previous version of this patch failed to account for how in Winsock, FD_WRITE is set only once when writing becomes possible and not again until after a send has failed due to the buffer filling. This contrasts to how FD_READ and FD_OOB continue to be set until the conditions they refer to no longer apply. This meant that if a user wrote some data to a socket, but not enough data to completely fill its send buffer, then waited on that socket to become writable, we'd erroneously stall until their configured timeout rather than returning immediately. This version of the patch addresses that issue by checking each socket we're waiting on to become writable with select() before the wait, and zeroing the timeout if it's already writable. Closes curl#5397
This avoids using a pair of TCP ports to provide wakeup functionality for every multi instance on Windows, where socketpair() is emulated using a TCP socket on loopback which could in turn lead to socket resource exhaustion. A previous version of this patch failed to account for how in WinSock, FD_WRITE is set only once when writing becomes possible and not again until after a send has failed due to the buffer filling. This contrasts to how FD_READ and FD_OOB continue to be set until the conditions they refer to no longer apply. This meant that if a user wrote some data to a socket, but not enough data to completely fill its send buffer, then waited on that socket to become writable, we'd erroneously stall until their configured timeout rather than returning immediately. This version of the patch addresses that issue by checking each socket we're waiting on to become writable with select() before the wait, and zeroing the timeout if it's already writable. Assisted-by: Marc Hörsken Reviewed-by: Marcel Raad Reviewed-by: Daniel Stenberg Tested-by: Gergely Nagy Tested-by: Rasmus Melchior Jacobsen Tested-by: Tomas Berger Replaces #5397 Reverts #5632 Closes #5634
This avoids using a pair of TCP ports to provide wakeup functionality for every multi instance on Windows, where socketpair() is emulated using a TCP socket on loopback.
I'm hoping this can get some review and testing from people more familiar with Winsock than I am.