Fix reporting refused connections with StreamSelectLoop on Windows

clue · clue · commit 6a894465c3f9 · 2019-12-31T11:26:18.000+01:00
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.
diff --git a/src/StreamSelectLoop.php b/src/StreamSelectLoop.php
@@ -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) {
diff --git a/tests/AbstractLoopTest.php b/tests/AbstractLoopTest.php
@@ -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);
         }
 
