diff --git a/src/Symfony/Component/HttpClient/Internal/NativeClientState.php b/src/Symfony/Component/HttpClient/Internal/NativeClientState.php index 6578929dc5466..2a47dbcca0ec0 100644 --- 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 index 639957415f0dd..2ba1b010e41fb 100644 --- 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 index 786e995b16ffd..2eb92180e33db 100644 --- a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php +++ b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php @@ -316,7 +316,7 @@ public static function stream(iterable $responses, float $timeout = null): \Gene } $lastActivity = microtime(true); - $isTimeout = false; + $enlapsedTimeout = 0; while (true) { $hasActivity = false; @@ -338,7 +338,7 @@ public static function stream(iterable $responses, float $timeout = null): \Gene } elseif (!isset($multi->openHandles[$j])) { unset($responses[$j]); continue; - } elseif ($isTimeout) { + } elseif ($enlapsedTimeout >= $timeoutMax) { $multi->handlesActivity[$j] = [new ErrorChunk($response->offset, sprintf('Idle timeout reached for "%s".', $response->getInfo('url')))]; } else { continue; @@ -346,7 +346,7 @@ public static function stream(iterable $responses, float $timeout = null): \Gene while ($multi->handlesActivity[$j] ?? false) { $hasActivity = true; - $isTimeout = false; + $enlapsedTimeout = 0; if (\is_string($chunk = array_shift($multi->handlesActivity[$j]))) { if (null !== $response->inflate && false === $chunk = @inflate_add($response->inflate, $chunk)) { @@ -379,7 +379,7 @@ public static function stream(iterable $responses, float $timeout = null): \Gene } } elseif ($chunk instanceof ErrorChunk) { unset($responses[$j]); - $isTimeout = true; + $enlapsedTimeout = $timeoutMax; } elseif ($chunk instanceof FirstChunk) { if ($response->logger) { $info = $response->getInfo(); @@ -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, min($timeoutMin, $timeoutMax - $enlapsedTimeout))) { + usleep(min(500, 1E6 * $timeoutMin)); } + + $enlapsedTimeout = microtime(true) - $lastActivity; } } } diff --git a/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php index f83edf91b6f39..19d4b683345bf 100644 --- a/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php +++ b/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php @@ -112,6 +112,15 @@ public function testHttp2PushVulcainWithUnusedResponse() $this->assertSame($expected, $logger->logs); } + public function testTimeoutIsNotAFatalError() + { + if ('\\' === \PHP_VERSION_ID) { + $this->markTestSkipped('Too transient on Windows'); + } + + parent::testTimeoutIsNotAFatalError(); + } + private function getVulcainClient(): CurlHttpClient { if (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304) { diff --git a/src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php b/src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php index 7cfe26e9dfe82..94b35470e338f 100644 --- a/src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php +++ b/src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php @@ -72,9 +72,9 @@ public function testToStream404() $this->assertSame($response, stream_get_meta_data($stream)['wrapper_data']->getResponse()); $this->assertSame(404, $response->getStatusCode()); - $this->expectException(ClientException::class); $response = $client->request('GET', 'http://localhost:8057/404'); - $stream = $response->toStream(); + $this->expectException(ClientException::class); + $response->toStream(); } public function testNonBlockingStream() @@ -82,6 +82,7 @@ public function testNonBlockingStream() $client = $this->getHttpClient(__FUNCTION__); $response = $client->request('GET', 'http://localhost:8057/timeout-body'); $stream = $response->toStream(); + usleep(10000); $this->assertTrue(stream_set_blocking($stream, false)); $this->assertSame('<1>', fread($stream, 8192)); @@ -99,6 +100,7 @@ public function testTimeoutIsNotAFatalError() $response = $client->request('GET', 'http://localhost:8057/timeout-body', [ 'timeout' => 0.25, ]); + $this->assertSame(200, $response->getStatusCode()); try { $response->getContent(); diff --git a/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php index b8125e6716cfd..f81322e0bb0b3 100644 --- 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 index 8ef46a9a90eeb..c1ad3ffec5014 100644 --- a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php +++ b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php @@ -786,6 +786,30 @@ public function testUncheckedTimeoutThrows() } } + public function testTimeoutWithActiveConcurrentStream() + { + $p1 = TestHttpServer::start(8067); + $p2 = TestHttpServer::start(8077); + + $client = $this->getHttpClient(__FUNCTION__); + $streamingResponse = $client->request('GET', 'http://localhost:8067/max-duration'); + $blockingResponse = $client->request('GET', 'http://localhost:8077/timeout-body', [ + 'timeout' => 0.25, + ]); + + $this->assertSame(200, $streamingResponse->getStatusCode()); + $this->assertSame(200, $blockingResponse->getStatusCode()); + + $this->expectException(TransportExceptionInterface::class); + + try { + $blockingResponse->getContent(); + } finally { + $p1->stop(); + $p2->stop(); + } + } + public function testDestruct() { $client = $this->getHttpClient(__FUNCTION__); diff --git a/src/Symfony/Contracts/HttpClient/Test/TestHttpServer.php b/src/Symfony/Contracts/HttpClient/Test/TestHttpServer.php index cfc100d80ce6c..06a11444e35e4 100644 --- a/src/Symfony/Contracts/HttpClient/Test/TestHttpServer.php +++ b/src/Symfony/Contracts/HttpClient/Test/TestHttpServer.php @@ -19,23 +19,28 @@ */ class TestHttpServer { - private static $process; + private static $process = []; - public static function start() + public static function start(int $port = 8057) { - if (self::$process) { - self::$process->stop(); + if (isset(self::$process[$port])) { + self::$process[$port]->stop(); + } else { + register_shutdown_function(static function () use ($port) { + self::$process[$port]->stop(); + }); } $finder = new PhpExecutableFinder(); - $process = new Process(array_merge([$finder->find(false)], $finder->findArguments(), ['-dopcache.enable=0', '-dvariables_order=EGPCS', '-S', '127.0.0.1:8057'])); + $process = new Process(array_merge([$finder->find(false)], $finder->findArguments(), ['-dopcache.enable=0', '-dvariables_order=EGPCS', '-S', '127.0.0.1:'.$port])); $process->setWorkingDirectory(__DIR__.'/Fixtures/web'); $process->start(); + self::$process[$port] = $process; do { usleep(50000); - } while (!@fopen('http://127.0.0.1:8057/', 'r')); + } while (!@fopen('http://127.0.0.1:'.$port, 'r')); - self::$process = $process; + return $process; } }