Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[Http] Better handling of nested scope requests in CurlMulti

Requests are now prepared in the send() method rather than the add()
method when adding a request during a transfer.  The send() method now
only prepares requests in the current scope in which the send method was
called.  This allows for better handling of commands that require a
request in order to prepare themselves for sending (e.g. a request that
requires a token that requires an HTTP request).  The BatchQueuePlugin
and CommandSet no longer add requests using async as that was a hack to
support the previous implementation.
  • Loading branch information...
commit 99a4756af089d353696da25b9a9d692c96839bb9 1 parent 3663512
Michael Dowling mtdowling authored

Showing 2 changed files with 29 additions and 28 deletions. Show diff stats Hide diff stats

  1. +28 27 Curl/CurlMulti.php
  2. +1 1  Plugin/BatchQueuePlugin.php
55 Curl/CurlMulti.php
@@ -147,11 +147,13 @@ public function __destruct()
147 147 /**
148 148 * {@inheritdoc}
149 149 *
150   - * Adds a request to the next scope (or batch or requests to be sent). If
151   - * a request is added using async, then the request is added to the current
152   - * scope. This means that the request will be sent and polled if requests
153   - * are currently being sent, or that the request will be sent in the next
154   - * send operation.
  150 + * Adds a request to a batch of requests to be sent in parallel.
  151 + *
  152 + * Async requests adds a request to the current scope to be executed in
  153 + * parallel with any currently executing cURL handles. You may only add an
  154 + * async request while other requests are transferring. Attempting to add
  155 + * an async request while no requests are transferring will add the request
  156 + * normally in the next available scope (typically 0).
155 157 *
156 158 * @param RequestInterface $request Request to add
157 159 * @param bool $async Set to TRUE to add to the current scope
@@ -160,8 +162,13 @@ public function __destruct()
160 162 */
161 163 public function add(RequestInterface $request, $async = false)
162 164 {
  165 + if ($async && $this->state != self::STATE_SENDING) {
  166 + $async = false;
  167 + }
  168 +
163 169 $this->requestCache = null;
164 170 $scope = $async ? $this->scope : $this->scope + 1;
  171 +
165 172 if (!isset($this->requests[$scope])) {
166 173 $this->requests[$scope] = array();
167 174 }
@@ -170,7 +177,9 @@ public function add(RequestInterface $request, $async = false)
170 177 'request' => $request
171 178 ));
172 179
173   - if ($this->state == self::STATE_SENDING) {
  180 + // If requests are currently transferring and this is async, then the
  181 + // request must be prepared now as the send() method is not called.
  182 + if ($this->state == self::STATE_SENDING && $async) {
174 183 $this->beforeSend($request);
175 184 }
176 185
@@ -262,27 +271,29 @@ public function reset($hard = false)
262 271 public function send()
263 272 {
264 273 $this->scope++;
  274 + $this->state = self::STATE_SENDING;
  275 +
  276 + // Only prepare and send requests that are in the current recursion scope
  277 + // Only enter the main perform() loop if there are requests in scope
  278 + if (!empty($this->requests[$this->scope])) {
265 279
266   - // Don't prepare for sending again if send() is called while sending
267   - if ($this->state != self::STATE_SENDING) {
268   - $requests = $this->all();
269 280 // Any exceptions thrown from this event should break the entire
270 281 // flow of sending requests in parallel to prevent weird errors
271 282 $this->dispatch(self::BEFORE_SEND, array(
272   - 'requests' => $requests
  283 + 'requests' => $this->requests[$this->scope]
273 284 ));
274   - $this->state = self::STATE_SENDING;
275   - foreach ($requests as $request) {
  285 +
  286 + foreach ($this->requests[$this->scope] as $request) {
276 287 if ($request->getState() != RequestInterface::STATE_TRANSFER) {
277 288 $this->beforeSend($request);
278 289 }
279 290 }
280   - }
281 291
282   - try {
283   - $this->perform();
284   - } catch (\Exception $e) {
285   - $this->exceptions[] = $e;
  292 + try {
  293 + $this->perform();
  294 + } catch (\Exception $e) {
  295 + $this->exceptions[] = $e;
  296 + }
286 297 }
287 298
288 299 $this->scope--;
@@ -391,8 +402,6 @@ protected function perform()
391 402
392 403 $active = $this->executeHandles();
393 404
394   - $curlErrors = false;
395   -
396 405 // Get messages from curl handles
397 406 while ($done = curl_multi_info_read($this->multiHandle)) {
398 407 foreach ($this->all() as $request) {
@@ -402,20 +411,12 @@ protected function perform()
402 411 $this->processResponse($request, $handle, $done);
403 412 } catch (\Exception $e) {
404 413 $this->removeErroredRequest($request, $e);
405   - $curlErrors = true;
406 414 }
407 415 break;
408 416 }
409 417 }
410 418 }
411 419
412   - // We need to check if every request has been fulfilled or has
413   - // encountered an error when any curl errors are encountered to
414   - // avoind an endless loop.
415   - if ($curlErrors && empty($this->requestCache)) {
416   - break;
417   - }
418   -
419 420 // Notify each request as polling and handled queued responses
420 421 if ($this->scope <= 0) {
421 422 $scopedPolling = $this->all();
2  Plugin/BatchQueuePlugin.php
@@ -115,7 +115,7 @@ public function flush()
115 115 // Prepare each request for their respective curl multi objects
116 116 while ($request = array_shift($this->queue)) {
117 117 $multi = $request->getClient()->getCurlMulti();
118   - $multi->add($request, true);
  118 + $multi->add($request);
119 119 if (!in_array($multi, $multis)) {
120 120 $multis[] = $multi;
121 121 }

0 comments on commit 99a4756

Please sign in to comment.
Something went wrong with that request. Please try again.