Permalink
Browse files

[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...
1 parent 3663512 commit 99a4756af089d353696da25b9a9d692c96839bb9 @mtdowling mtdowling committed May 19, 2012
Showing with 29 additions and 28 deletions.
  1. +28 −27 Curl/CurlMulti.php
  2. +1 −1 Plugin/BatchQueuePlugin.php
View
55 Curl/CurlMulti.php
@@ -147,11 +147,13 @@ public function __destruct()
/**
* {@inheritdoc}
*
- * Adds a request to the next scope (or batch or requests to be sent). If
- * a request is added using async, then the request is added to the current
- * scope. This means that the request will be sent and polled if requests
- * are currently being sent, or that the request will be sent in the next
- * send operation.
+ * Adds a request to a batch of requests to be sent in parallel.
+ *
+ * Async requests adds a request to the current scope to be executed in
+ * parallel with any currently executing cURL handles. You may only add an
+ * async request while other requests are transferring. Attempting to add
+ * an async request while no requests are transferring will add the request
+ * normally in the next available scope (typically 0).
*
* @param RequestInterface $request Request to add
* @param bool $async Set to TRUE to add to the current scope
@@ -160,8 +162,13 @@ public function __destruct()
*/
public function add(RequestInterface $request, $async = false)
{
+ if ($async && $this->state != self::STATE_SENDING) {
+ $async = false;
+ }
+
$this->requestCache = null;
$scope = $async ? $this->scope : $this->scope + 1;
+
if (!isset($this->requests[$scope])) {
$this->requests[$scope] = array();
}
@@ -170,7 +177,9 @@ public function add(RequestInterface $request, $async = false)
'request' => $request
));
- if ($this->state == self::STATE_SENDING) {
+ // If requests are currently transferring and this is async, then the
+ // request must be prepared now as the send() method is not called.
+ if ($this->state == self::STATE_SENDING && $async) {
$this->beforeSend($request);
}
@@ -262,27 +271,29 @@ public function reset($hard = false)
public function send()
{
$this->scope++;
+ $this->state = self::STATE_SENDING;
+
+ // Only prepare and send requests that are in the current recursion scope
+ // Only enter the main perform() loop if there are requests in scope
+ if (!empty($this->requests[$this->scope])) {
- // Don't prepare for sending again if send() is called while sending
- if ($this->state != self::STATE_SENDING) {
- $requests = $this->all();
// Any exceptions thrown from this event should break the entire
// flow of sending requests in parallel to prevent weird errors
$this->dispatch(self::BEFORE_SEND, array(
- 'requests' => $requests
+ 'requests' => $this->requests[$this->scope]
));
- $this->state = self::STATE_SENDING;
- foreach ($requests as $request) {
+
+ foreach ($this->requests[$this->scope] as $request) {
if ($request->getState() != RequestInterface::STATE_TRANSFER) {
$this->beforeSend($request);
}
}
- }
- try {
- $this->perform();
- } catch (\Exception $e) {
- $this->exceptions[] = $e;
+ try {
+ $this->perform();
+ } catch (\Exception $e) {
+ $this->exceptions[] = $e;
+ }
}
$this->scope--;
@@ -391,8 +402,6 @@ protected function perform()
$active = $this->executeHandles();
- $curlErrors = false;
-
// Get messages from curl handles
while ($done = curl_multi_info_read($this->multiHandle)) {
foreach ($this->all() as $request) {
@@ -402,20 +411,12 @@ protected function perform()
$this->processResponse($request, $handle, $done);
} catch (\Exception $e) {
$this->removeErroredRequest($request, $e);
- $curlErrors = true;
}
break;
}
}
}
- // We need to check if every request has been fulfilled or has
- // encountered an error when any curl errors are encountered to
- // avoind an endless loop.
- if ($curlErrors && empty($this->requestCache)) {
- break;
- }
-
// Notify each request as polling and handled queued responses
if ($this->scope <= 0) {
$scopedPolling = $this->all();
View
2 Plugin/BatchQueuePlugin.php
@@ -115,7 +115,7 @@ public function flush()
// Prepare each request for their respective curl multi objects
while ($request = array_shift($this->queue)) {
$multi = $request->getClient()->getCurlMulti();
- $multi->add($request, true);
+ $multi->add($request);
if (!in_array($multi, $multis)) {
$multis[] = $multi;
}

0 comments on commit 99a4756

Please sign in to comment.