8000 bug #37048 [HttpClient] fix monitoring timeouts when other streams ar… · symfony/symfony@3755efd · GitHub
[go: up one dir, main page]
Skip to content

Commit 3755efd

Browse files
nicolas-grekasnicolas-grekas
committed
bug #37048 [HttpClient] fix monitoring timeouts when other streams are active (nicolas-grekas)
This PR was merged into the 4.4 branch. Discussion ---------- [HttpClient] fix monitoring timeouts when other streams are active | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Commits ------- d2a53f0 [HttpClient] fix monitoring timeouts when other streams are active
2 parents 1e7f3e2 + d2a53f0 commit 3755efd

File tree

8 files changed

+64
-31
lines changed

8 files changed

+64
-31
lines changed

‎src/Symfony/Component/HttpClient/Internal/NativeClientState.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ final class NativeClientState extends ClientState
2828
public $responseCount = 0;
2929
/** @var string[] */
3030
public $dnsCache = [];
31-
/** @var resource[] */
32-
public $handles = [];
3331
/** @var bool */
3432
public $sleep = false;
3533

‎src/Symfony/Component/HttpClient/Response/NativeResponse.php‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,6 @@ private static function schedule(self $response, array &$runningResponses): void
220220
*/
221221
private static function perform(ClientState $multi, array &$responses = null): void
222222
{
223-
// List of native handles for stream_select()
224-
if (null !== $responses) {
225-
$multi->handles = [];
226-
}
227-
228223
foreach ($multi->openHandles as $i => [$h, $buffer, $onProgress]) {
229224
$hasActivity = false;
230225
$remaining = &$multi->openHandles[$i][3];
@@ -291,8 +286,6 @@ private static function perform(ClientState $multi, array &$responses = null): v
291286
$multi->handlesActivity[$i][] = $e;
292287
unset($multi->openHandles[$i]);
293288
$multi->sleep = false;
294-
} elseif (null !== $responses) {
295-
$multi->handles[] = $h;
296289
}
297290
}
298291

@@ -307,7 +300,7 @@ private static function perform(ClientState $multi, array &$responses = null): v
307300
}
308301
}
309302

310-
if (\count($multi->handles) >= $multi->maxHostConnections) {
303+
if (\count($multi->openHandles) >= $multi->maxHostConnections) {
311304
return;
312305
}
313306

@@ -318,10 +311,6 @@ private static function perform(ClientState $multi, array &$responses = null): v
318311
$multi->sleep = false;
319312
self::perform($multi);
320313

321-
if (null !== $response->handle) {
322-
$multi->handles[] = $response->handle;
323-
}
324-
325314
break;
326315
}
327316
}
@@ -335,7 +324,8 @@ private static function perform(ClientState $multi, array &$responses = null): v
335324
private static function select(ClientState $multi, float $timeout): int
336325
{
337326
$_ = [];
327+
$handles = array_column($multi->openHandles, 0);
338328

339-
return (!$multi->sleep = !$multi->sleep) ? -1 : stream_select($multi->handles, $_, $_, (int) $timeout, (int) (1E6 * ($timeout - (int) $timeout)));
329+
return (!$multi->sleep = !$multi->sleep) ? -1 : stream_select($handles, $_, $_, (int) $timeout, (int) (1E6 * ($timeout - (int) $timeout)));
340330
}
341331
}

‎src/Symfony/Component/HttpClient/Response/ResponseTrait.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
316316
}
317317

318318
$lastActivity = microtime(true);
319-
$isTimeout = false;
319+
$enlapsedTimeout = 0;
320320

321321
while (true) {
322322
$hasActivity = false;
@@ -338,15 +338,15 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
338338
} elseif (!isset($multi->openHandles[$j])) {
339339
unset($responses[$j]);
340340
continue;
341-
} elseif ($isTimeout) {
341+
} elseif ($enlapsedTimeout >= $timeoutMax) {
342342
$multi->handlesActivity[$j] = [new ErrorChunk($response->offset, sprintf('Idle timeout reached for "%s".', $response->getInfo('url')))];
343343
} else {
344344
continue;
345345
}
346346

347347
while ($multi->handlesActivity[$j] ?? false) {
348348
$hasActivity = true;
349-
$isTimeout = false;
349+
$enlapsedTimeout = 0;
350350

351351
if (\is_string($chunk = array_shift($multi->handlesActivity[$j]))) {
352352
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
379379
}
380380
} elseif ($chunk instanceof ErrorChunk) {
381381
unset($responses[$j]);
382-
$isTimeout = true;
382+
$enlapsedTimeout = $timeoutMax;
383383
} elseif ($chunk instanceof FirstChunk) {
384384
if ($response->logger) {
385385
$info = $response->getInfo();
@@ -447,10 +447,11 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
447447
continue;
448448
}
449449

450-
switch (self::select($multi, $timeoutMin)) {
451-
case -1: usleep(min(500, 1E6 * $timeoutMin)); break;
452-
case 0: $isTimeout = microtime(true) - $lastActivity > $timeoutMax; break;
450+
if (-1 === self::select($multi, min($timeoutMin, $timeoutMax - $enlapsedTimeout))) {
451+
usleep(min(500, 1E6 * $timeoutMin));
453452
}
453+
454+
$enlapsedTimeout = microtime(true) - $lastActivity;
454455
}
455456
}
456457
}

‎src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ public function testHttp2PushVulcainWithUnusedResponse()
112112
$this->assertSame($expected, $logger->logs);
113113
}
114114

115+
public function testTimeoutIsNotAFatalError()
116+
{
117+
if ('\\' === \DIRECTORY_SEPARATOR) {
118+
$this->markTestSkipped('Too transient on Windows');
119+
}
120+
121+
parent::testTimeoutIsNotAFatalError();
122+
}
123+
115124
private function getVulcainClient(): CurlHttpClient
116125
{
117126
if (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304) {

‎src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,17 @@ public function testToStream404()
7272
$this->assertSame($response, stream_get_meta_data($stream)['wrapper_data']->getResponse());
7373
$this->assertSame(404, $response->getStatusCode());
7474

75-
$this->expectException(ClientException::class);
7675
$response = $client->request('GET', 'http://localhost:8057/404');
77-
$stream = $response->toStream();
76+
$this->expectException(ClientException::class);
77+
$response->toStream();
7878
}
7979

8080
public function testNonBlockingStream()
8181
{
8282
$client = $this->getHttpClient(__FUNCTION__);
8383
$response = $client->request('GET', 'http://localhost:8057/timeout-body');
8484
$stream = $response->toStream();
85+
usleep(10000);
8586

8687
$this->assertTrue(stream_set_blocking($stream, false));
8788
$this->assertSame('<1>', fread($stream, 8192));
@@ -99,6 +100,7 @@ public function testTimeoutIsNotAFatalError()
99100
$response = $client->request('GET', 'http://localhost:8057/timeout-body', [
100101
'timeout' => 0.25,
101102
]);
103+
$this->assertSame(200, $response->getStatusCode());
102104

103105
try {
104106
$response->getContent();

‎src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ protected function getHttpClient(string $testCase): HttpClientInterface
6969
$this->markTestSkipped("MockHttpClient doesn't unzip");
7070
break;
7171

72+
case 'testTimeoutWithActiveConcurrentStream':
73+
$this->markTestSkipped('Real transport required');
74+
break;
75+
7276
case 'testDestruct':
7377
$this->markTestSkipped("MockHttpClient doesn't timeout on destruct");
7478
break;

‎src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,30 @@ public function testUncheckedTimeoutThrows()
786786
}
787787
}
788788

789+
public function testTimeoutWithActiveConcurrentStream()
790+
{
791+
$p1 = TestHttpServer::start(8067);
792+
$p2 = TestHttpServer::start(8077);
793+
794+
$client = $this->getHttpClient(__FUNCTION__);
795+
$streamingResponse = $client->request('GET', 'http://localhost:8067/max-duration');
796+
$blockingResponse = $client->request('GET', 'http://localhost:8077/timeout-body', [
797+
'timeout' => 0.25,
798+
]);
799+
800+
$this->assertSame(200, $streamingResponse->getStatusCode());
801+
$this->assertSame(200, $blockingResponse->getStatusCode());
802+
803+
$this->expectException(TransportExceptionInterface::class);
804+
805+
try {
806+
$blockingResponse->getContent();
807+
} finally {
808+
$p1->stop();
809+
$p2->stop();
810+
}
811+
}
812+
789813
public function testDestruct()
790814
{
791815
$client = $this->getHttpClient(__FUNCTION__);

‎src/Symfony/Contracts/HttpClient/Test/TestHttpServer.php‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,28 @@
1919
*/
2020
class TestHttpServer
2121
{
22-
private static $process;
22+
private static $process = [];
2323

24-
public static function start()
24+
public static function start(int $port = 8057)
2525
{
26-
if (self::$process) {
27-
self::$process->stop();
26+
if (isset(self::$process[$port])) {
27+
self::$process[$port]->stop();
28+
} else {
29+
register_shutdown_function(static function () use ($port) {
30+
self::$process[$port]->stop();
31+
});
2832
}
2933

3034
$finder = new PhpExecutableFinder();
31-
$process = new Process(array_merge([$finder->find(false)], $finder->findArguments(), ['-dopcache.enable=0', '-dvariables_order=EGPCS', '-S', '127.0.0.1:8057']));
35+
$process = new Process(array_merge([$finder->find(false)], $finder->findArguments(), ['-dopcache.enable=0', '-dvariables_order=EGPCS', '-S', '127.0.0.1:'.$port]));
3236
$process->setWorkingDirectory(__DIR__.'/Fixtures/web');
3337
$process->start();
38+
self::$process[$port] = $process;
3439

3540
do {
3641
usleep(50000);
37-
} while (!@fopen('http://127.0.0.1:8057/', 'r'));
42+
} while (!@fopen('http://127.0.0.1:'.$port, 'r'));
3843

39-
self::$process = $process;
44+
return $process;
4045
}
4146
}

0 commit comments

Comments
 (0)
0