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

Fix reporting refused connections with StreamSelectLoop on Windows #208

Merged
merged 4 commits into from Dec 31, 2019

Conversation

@clue
Copy link
Member

clue commented Dec 31, 2019

We do not usually use or expose the exceptfds parameter passed to the
underlying select. However, Windows does not report failed connection
attempts in writefds passed to select like most other platforms.
Instead, it uses writefds only for successful connection attempts and
exceptfds for failed connection attempts. See also
https://docs.microsoft.com/de-de/windows/win32/api/winsock2/nf-winsock2-select

We work around this by adding all sockets that look like a pending
connection attempt to exceptfds automatically on Windows and merge it
back later. This ensures the public API matches other loop
implementations across all platforms (see also test suite or rather test
matrix). Lacking better APIs, every write-only socket that has not yet
read any data is assumed to be in a pending connection attempt state.

Resolves #206
Builds on top of #205 and #207 (see last commit to omit unrelated changes)

clue added 3 commits Dec 24, 2019
The underlying `epoll_wait()` reports `EPOLLOUT|EPOLLERR|EPOLLHUP` on
the affected file descriptor, which `ext-uv` emits as an error code
`EBADF` with no events attached. We explicitly re-enable all active
events on this error event to invoke the writable listener for this
condition to match other event loop implementations and successfully
detect this as a refused connection attempt. All tests are now green.
@clue clue added the bug label Dec 31, 2019
@clue clue added this to the v1.1.1 milestone Dec 31, 2019
@clue clue requested review from WyriHaximus and jsor Dec 31, 2019
We do not usually use or expose the `exceptfds` parameter passed to the
underlying `select`. However, Windows does not report failed connection
attempts in `writefds` passed to `select` like most other platforms.
Instead, it uses `writefds` only for successful connection attempts and
`exceptfds` for failed connection attempts. See also
https://docs.microsoft.com/de-de/windows/win32/api/winsock2/nf-winsock2-select

We work around this by adding all sockets that look like a pending
connection attempt to `exceptfds` automatically on Windows and merge it
back later. This ensures the public API matches other loop
implementations across all platforms (see also test suite or rather test
matrix). Lacking better APIs, every write-only socket that has not yet
read any data is assumed to be in a pending connection attempt state.
@clue clue force-pushed the clue-labs:windows-connections branch from 120e1bb to 6a89446 Dec 31, 2019
@jsor
jsor approved these changes Dec 31, 2019
@jsor jsor merged commit 646fbbd into reactphp:master Dec 31, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@clue clue deleted the clue-labs:windows-connections branch Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.