From ee3451b3c7a523d8d088164277e395778bd5e653 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 7 Feb 2025 15:14:19 +0100 Subject: [PATCH] [HttpClient] Fix activity tracking leading to negative timeout errors --- .../HttpClient/Response/AmpResponse.php | 20 +++++------ .../HttpClient/Response/CurlResponse.php | 4 +-- .../HttpClient/Response/MockResponse.php | 2 +- .../HttpClient/Response/NativeResponse.php | 2 +- .../Response/TransportResponseTrait.php | 36 ++++++++++--------- 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/Symfony/Component/HttpClient/Response/AmpResponse.php b/src/Symfony/Component/HttpClient/Response/AmpResponse.php index e01d97eb868e4..00001ccecba06 100644 --- a/src/Symfony/Component/HttpClient/Response/AmpResponse.php +++ b/src/Symfony/Component/HttpClient/Response/AmpResponse.php @@ -179,19 +179,17 @@ private static function schedule(self $response, array &$runningResponses): void /** * @param AmpClientState $multi */ - private static function perform(ClientState $multi, ?array &$responses = null): void + private static function perform(ClientState $multi, ?array $responses = null): void { - if ($responses) { - foreach ($responses as $response) { - try { - if ($response->info['start_time']) { - $response->info['total_time'] = microtime(true) - $response->info['start_time']; - ($response->onProgress)(); - } - } catch (\Throwable $e) { - $multi->handlesActivity[$response->id][] = null; - $multi->handlesActivity[$response->id][] = $e; + foreach ($responses ?? [] as $response) { + try { + if ($response->info['start_time']) { + $response->info['total_time'] = microtime(true) - $response->info['start_time']; + ($response->onProgress)(); } + } catch (\Throwable $e) { + $multi->handlesActivity[$response->id][] = null; + $multi->handlesActivity[$response->id][] = $e; } } } diff --git a/src/Symfony/Component/HttpClient/Response/CurlResponse.php b/src/Symfony/Component/HttpClient/Response/CurlResponse.php index 88cb764384dad..e5dfd3e52d3c1 100644 --- a/src/Symfony/Component/HttpClient/Response/CurlResponse.php +++ b/src/Symfony/Component/HttpClient/Response/CurlResponse.php @@ -265,11 +265,11 @@ private static function schedule(self $response, array &$runningResponses): void /** * @param CurlClientState $multi */ - private static function perform(ClientState $multi, ?array &$responses = null): void + private static function perform(ClientState $multi, ?array $responses = null): void { if ($multi->performing) { if ($responses) { - $response = current($responses); + $response = $responses[array_key_first($responses)]; $multi->handlesActivity[(int) $response->handle][] = null; $multi->handlesActivity[(int) $response->handle][] = new TransportException(sprintf('Userland callback cannot use the client nor the response while processing "%s".', curl_getinfo($response->handle, \CURLINFO_EFFECTIVE_URL))); } diff --git a/src/Symfony/Component/HttpClient/Response/MockResponse.php b/src/Symfony/Component/HttpClient/Response/MockResponse.php index 0493bcb7c6fc2..c6b96c0b18416 100644 --- a/src/Symfony/Component/HttpClient/Response/MockResponse.php +++ b/src/Symfony/Component/HttpClient/Response/MockResponse.php @@ -167,7 +167,7 @@ protected static function schedule(self $response, array &$runningResponses): vo $runningResponses[0][1][$response->id] = $response; } - protected static function perform(ClientState $multi, array &$responses): void + protected static function perform(ClientState $multi, array $responses): void { foreach ($responses as $response) { $id = $response->id; diff --git a/src/Symfony/Component/HttpClient/Response/NativeResponse.php b/src/Symfony/Component/HttpClient/Response/NativeResponse.php index 9a5184ed6f6d4..54312884cd957 100644 --- a/src/Symfony/Component/HttpClient/Response/NativeResponse.php +++ b/src/Symfony/Component/HttpClient/Response/NativeResponse.php @@ -228,7 +228,7 @@ private static function schedule(self $response, array &$runningResponses): void /** * @param NativeClientState $multi */ - private static function perform(ClientState $multi, ?array &$responses = null): void + private static function perform(ClientState $multi, ?array $responses = null): void { foreach ($multi->openHandles as $i => [$pauseExpiry, $h, $buffer, $onProgress]) { if ($pauseExpiry) { diff --git a/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php b/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php index 7b65fd7990464..95f7e9624c912 100644 --- a/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php +++ b/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php @@ -92,7 +92,7 @@ abstract protected static function schedule(self $response, array &$runningRespo /** * Performs all pending non-blocking operations. */ - abstract protected static function perform(ClientState $multi, array &$responses): void; + abstract protected static function perform(ClientState $multi, array $responses): void; /** * Waits for network activity. @@ -150,10 +150,15 @@ public static function stream(iterable $responses, ?float $timeout = null): \Gen $lastActivity = hrtime(true) / 1E9; $elapsedTimeout = 0; - if ($fromLastTimeout = 0.0 === $timeout && '-0' === (string) $timeout) { - $timeout = null; - } elseif ($fromLastTimeout = 0 > $timeout) { - $timeout = -$timeout; + if ((0.0 === $timeout && '-0' === (string) $timeout) || 0 > $timeout) { + $timeout = $timeout ? -$timeout : null; + + /** @var ClientState $multi */ + foreach ($runningResponses as [$multi]) { + if (null !== $multi->lastTimeout) { + $elapsedTimeout = max($elapsedTimeout, $lastActivity - $multi->lastTimeout); + } + } } while (true) { @@ -162,8 +167,7 @@ public static function stream(iterable $responses, ?float $timeout = null): \Gen $timeoutMin = $timeout ?? \INF; /** @var ClientState $multi */ - foreach ($runningResponses as $i => [$multi]) { - $responses = &$runningResponses[$i][1]; + foreach ($runningResponses as $i => [$multi, &$responses]) { self::perform($multi, $responses); foreach ($responses as $j => $response) { @@ -171,26 +175,25 @@ public static function stream(iterable $responses, ?float $timeout = null): \Gen $timeoutMin = min($timeoutMin, $response->timeout, 1); $chunk = false; - if ($fromLastTimeout && null !== $multi->lastTimeout) { - $elapsedTimeout = hrtime(true) / 1E9 - $multi->lastTimeout; - } - if (isset($multi->handlesActivity[$j])) { $multi->lastTimeout = null; + $elapsedTimeout = 0; } elseif (!isset($multi->openHandles[$j])) { + $hasActivity = true; unset($responses[$j]); continue; } elseif ($elapsedTimeout >= $timeoutMax) { $multi->handlesActivity[$j] = [new ErrorChunk($response->offset, sprintf('Idle timeout reached for "%s".', $response->getInfo('url')))]; $multi->lastTimeout ??= $lastActivity; + $elapsedTimeout = $timeoutMax; } else { continue; } - while ($multi->handlesActivity[$j] ?? false) { - $hasActivity = true; - $elapsedTimeout = 0; + $lastActivity = null; + $hasActivity = true; + while ($multi->handlesActivity[$j] ?? false) { if (\is_string($chunk = array_shift($multi->handlesActivity[$j]))) { if (null !== $response->inflate && false === $chunk = @inflate_add($response->inflate, $chunk)) { $multi->handlesActivity[$j] = [null, new TransportException(sprintf('Error while processing content unencoding for "%s".', $response->getInfo('url')))]; @@ -227,7 +230,6 @@ public static function stream(iterable $responses, ?float $timeout = null): \Gen } } elseif ($chunk instanceof ErrorChunk) { unset($responses[$j]); - $elapsedTimeout = $timeoutMax; } elseif ($chunk instanceof FirstChunk) { if ($response->logger) { $info = $response->getInfo(); @@ -274,10 +276,12 @@ public static function stream(iterable $responses, ?float $timeout = null): \Gen if ($chunk instanceof ErrorChunk && !$chunk->didThrow()) { // Ensure transport exceptions are always thrown $chunk->getContent(); + throw new \LogicException('A transport exception should have been thrown.'); } } if (!$responses) { + $hasActivity = true; unset($runningResponses[$i]); } @@ -291,7 +295,7 @@ public static function stream(iterable $responses, ?float $timeout = null): \Gen } if ($hasActivity) { - $lastActivity = hrtime(true) / 1E9; + $lastActivity ??= hrtime(true) / 1E9; continue; }