[HttpClient] fix monitoring timeouts when other streams are active

symfony · Jun 1, 2020 · efb3cbb · efb3cbb
1 parent 6a8b22d
commit efb3cbb
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 18 deletions.
diff --git a/src/Symfony/Component/HttpClient/Internal/NativeClientState.php b/src/Symfony/Component/HttpClient/Internal/NativeClientState.php
@@ -28,8 +28,6 @@ final class NativeClientState extends ClientState
     public $responseCount = 0;
     /** @var string[] */
     public $dnsCache = [];
-    /** @var resource[] */
-    public $handles = [];
     /** @var bool */
     public $sleep = false;
 

diff --git a/src/Symfony/Component/HttpClient/Response/NativeResponse.php b/src/Symfony/Component/HttpClient/Response/NativeResponse.php
@@ -220,11 +220,6 @@ private static function schedule(self $response, array &$runningResponses): void
      */
     private static function perform(ClientState $multi, array &$responses = null): void
     {
-        // List of native handles for stream_select()
-        if (null !== $responses) {
-            $multi->handles = [];
-        }
-
         foreach ($multi->openHandles as $i => [$h, $buffer, $onProgress]) {
             $hasActivity = false;
             $remaining = &$multi->openHandles[$i][3];
@@ -291,8 +286,6 @@ private static function perform(ClientState $multi, array &$responses = null): v
                 $multi->handlesActivity[$i][] = $e;
                 unset($multi->openHandles[$i]);
                 $multi->sleep = false;
-            } elseif (null !== $responses) {
-                $multi->handles[] = $h;
             }
         }
 
@@ -307,7 +300,7 @@ private static function perform(ClientState $multi, array &$responses = null): v
             }
         }
 
-        if (\count($multi->handles) >= $multi->maxHostConnections) {
+        if (\count($multi->openHandles) >= $multi->maxHostConnections) {
             return;
         }
 
@@ -318,10 +311,6 @@ private static function perform(ClientState $multi, array &$responses = null): v
                 $multi->sleep = false;
                 self::perform($multi);
 
-                if (null !== $response->handle) {
-                    $multi->handles[] = $response->handle;
-                }
-
                 break;
             }
         }
@@ -335,7 +324,8 @@ private static function perform(ClientState $multi, array &$responses = null): v
     private static function select(ClientState $multi, float $timeout): int
     {
         $_ = [];
+        $handles = array_column($multi->openHandles, 0);
 
-        return (!$multi->sleep = !$multi->sleep) ? -1 : stream_select($multi->handles, $_, $_, (int) $timeout, (int) (1E6 * ($timeout - (int) $timeout)));
+        return (!$multi->sleep = !$multi->sleep) ? -1 : stream_select($handles, $_, $_, (int) $timeout, (int) (1E6 * ($timeout - (int) $timeout)));
     }
 }
diff --git a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php
@@ -447,10 +447,11 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
                 continue;
             }
 
-            switch (self::select($multi, $timeoutMin)) {
-                case -1: usleep(min(500, 1E6 * $timeoutMin)); break;
-                case 0: $isTimeout = microtime(true) - $lastActivity > $timeoutMax; break;
+            if (-1 === self::select($multi, $timeoutMin)) {
+                usleep(min(500, 1E6 * $timeoutMin));
             }
+
+            $isTimeout = microtime(true) - $lastActivity > $timeoutMax;
         }
     }
 }
diff --git a/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php
@@ -69,6 +69,10 @@ protected function getHttpClient(string $testCase): HttpClientInterface
                 $this->markTestSkipped("MockHttpClient doesn't unzip");
                 break;
 
+            case 'testTimeoutWithActiveConcurrentStream':
+                $this->markTestSkipped('Real transport required');
+                break;
+
             case 'testDestruct':
                 $this->markTestSkipped("MockHttpClient doesn't timeout on destruct");
                 break;

diff --git a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
@@ -786,6 +786,20 @@ public function testUncheckedTimeoutThrows()
         }
     }
 
+    public function testTimeoutWithActiveConcurrentStream()
+    {
+        $client = $this->getHttpClient(__FUNCTION__);
+        $streamingResponse = $client->request('GET', 'http://localhost:8057/max-duration');
+        $blockingResponse = $client->request('GET', 'http://localhost:8057/timeout-body', [
+            'timeout' => 0.25,
+        ]);
+
+        $this->assertSame(200, $streamingResponse->getStatusCode());
+
+        $this->expectException(TransportExceptionInterface::class);
+        $blockingResponse->getContent();
+    }
+
     public function testDestruct()
     {
         $client = $this->getHttpClient(__FUNCTION__);