Skip to content

Commit

Permalink
Fix reporting refused connections with StreamSelectLoop on Windows
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
clue committed Dec 31, 2019
1 parent 6d45a19 commit 6a89446
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
22 changes: 21 additions & 1 deletion src/StreamSelectLoop.php
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,30 @@ private function waitForStreamActivity($timeout)
private function streamSelect(array &$read, array &$write, $timeout)
{
if ($read || $write) {
// 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.
// 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.
// @link https://docs.microsoft.com/de-de/windows/win32/api/winsock2/nf-winsock2-select
$except = null;
if (\DIRECTORY_SEPARATOR === '\\') {
$except = array();
foreach ($write as $key => $socket) {
if (!isset($read[$key]) && @\ftell($socket) === 0) {
$except[$key] = $socket;
}
}
}

// suppress warnings that occur, when stream_select is interrupted by a signal
return @\stream_select($read, $write, $except, $timeout === null ? null : 0, $timeout);
$ret = @\stream_select($read, $write, $except, $timeout === null ? null : 0, $timeout);

if ($except) {
$write = \array_merge($write, $except);
}
return $ret;
}

if ($timeout > 0) {
Expand Down
7 changes: 1 addition & 6 deletions tests/AbstractLoopTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,11 @@ public function testAddWriteStreamTriggersWhenSocketConnectionSucceeds()

public function testAddWriteStreamTriggersWhenSocketConnectionRefused()
{
// @link https://github.com/reactphp/event-loop/issues/206
if ($this->loop instanceof StreamSelectLoop && DIRECTORY_SEPARATOR === '\\') {
$this->markTestIncomplete('StreamSelectLoop does not currently support detecting connection refused errors on Windows');
}

// first verify the operating system actually refuses the connection and no firewall is in place
// use higher timeout because Windows retires multiple times and has a noticeable delay
// @link https://stackoverflow.com/questions/19440364/why-do-failed-attempts-of-socket-connect-take-1-sec-on-windows
$errno = $errstr = null;
if (@stream_socket_client('127.0.0.1:1', $errno, $errstr, 10.0) !== false || $errno !== SOCKET_ECONNREFUSED) {
if (@stream_socket_client('127.0.0.1:1', $errno, $errstr, 10.0) !== false || (defined('SOCKET_ECONNREFUSED') && $errno !== SOCKET_ECONNREFUSED)) {
$this->markTestSkipped('Expected host to refuse connection, but got error ' . $errno . ': ' . $errstr);
}

Expand Down

0 comments on commit 6a89446

Please sign in to comment.