Skip to content

Commit

Permalink
bug #35120 [HttpClient] fix scheduling pending NativeResponse (nicola…
Browse files Browse the repository at this point in the history
…s-grekas)

This PR was merged into the 4.3 branch.

Discussion
----------

[HttpClient] fix scheduling pending NativeResponse

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

There must be one pending list per `ResponseStream` instance.
Currently, we start unrelated responses and this can lead to broken iterators when the unrelated response throws because it is a 3/4/5xx.

Commits
-------

a90a6c9 [HttpClient] fix scheduling pending NativeResponse
  • Loading branch information
nicolas-grekas committed Dec 28, 2019
2 parents b6403d8 + a90a6c9 commit f3d8fd2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 25 deletions.
Expand Up @@ -11,8 +11,6 @@

namespace Symfony\Component\HttpClient\Internal;

use Symfony\Component\HttpClient\Response\NativeResponse;

/**
* Internal representation of the native client's state.
*
Expand All @@ -24,8 +22,6 @@ final class NativeClientState extends ClientState
{
/** @var int */
public $id;
/** @var NativeResponse[] */
public $pendingResponses = [];
/** @var int */
public $maxHostConnections = PHP_INT_MAX;
/** @var int */
Expand Down
43 changes: 22 additions & 21 deletions src/Symfony/Component/HttpClient/Response/NativeResponse.php
Expand Up @@ -209,11 +209,7 @@ private static function schedule(self $response, array &$runningResponses): void
$runningResponses[$i] = [$response->multi, []];
}

if (null === $response->remaining) {
$response->multi->pendingResponses[] = $response;
} else {
$runningResponses[$i][1][$response->id] = $response;
}
$runningResponses[$i][1][$response->id] = $response;

if (null === $response->buffer) {
// Response already completed
Expand Down Expand Up @@ -315,25 +311,30 @@ private static function perform(NativeClientState $multi, array &$responses = nu
return;
}

if ($multi->pendingResponses && \count($multi->handles) < $multi->maxHostConnections) {
// Open the next pending request - this is a blocking operation so we do only one of them
/** @var self $response */
$response = array_shift($multi->pendingResponses);
$response->open();
$responses[$response->id] = $response;
$multi->sleep = false;
self::perform($response->multi);

if (null !== $response->handle) {
$multi->handles[] = $response->handle;
// Create empty activity lists to tell ResponseTrait::stream() we still have pending requests
foreach ($responses as $i => $response) {
if (null === $response->remaining && null !== $response->buffer) {
$multi->handlesActivity[$i] = [];
}
}

if ($multi->pendingResponses) {
// Create empty activity list to tell ResponseTrait::stream() we still have pending requests
$response = $multi->pendingResponses[0];
$responses[$response->id] = $response;
$multi->handlesActivity[$response->id] = [];
if (\count($multi->handles) >= $multi->maxHostConnections) {
return;
}

// Open the next pending request - this is a blocking operation so we do only one of them
foreach ($responses as $i => $response) {
if (null === $response->remaining && null !== $response->buffer) {
$response->open();
$multi->sleep = false;
self::perform($multi);

if (null !== $response->handle) {
$multi->handles[] = $response->handle;
}

break;
}
}
}

Expand Down

0 comments on commit f3d8fd2

Please sign in to comment.