WireHttpMulti::execute() has three interconnected bugs in its curl_multi loop that cause queued HTTP requests to be silently discarded without ever being attempted.
Bug 1 — Refill only fires on completion events. The logic that pulls new requests from the pool only runs inside the curl_multi_info_read() loop. If every initial spawn() call fails (for example, curl_init() returns null), no handle is added to the multi handle, no completion event ever fires, and the remaining pool is silently dropped. Those requests receive no result object at all.
Bug 2 — Exit condition doesn't check the pool. The while($running > 0 || !empty($active)) condition can become false when all active requests finish in a single curl_multi_info_read() pass, even if items remain in the pool. Those items are never attempted.
Bug 3 — Duplicated fill logic. The initial ramp-up for loop duplicates the spawn logic that also lives inside curl_multi_info_read(), making the code harder to reason about and maintain.
Fix
Restructure the loop so slot-filling runs unconditionally at the top of every iteration, decoupled from completion events, and extend the exit condition to include the pool:
- Remove the separate ramp-up loop — fold it into a single fill step.
- Move the fill step to the top of the
do-while body so it runs on every iteration.
- Remove the refill from inside
curl_multi_info_read().
- Add
if(empty($active)) break guard after fill.
- Add
|| !empty($pool) to the exit condition.
-OpenCode Zen / Big Pickle
--- a/wire/core/Tools/WireHttp/WireHttpMulti/WireHttpMulti.php
+++ b/wire/core/Tools/WireHttp/WireHttpMulti/WireHttpMulti.php
@@ -213,21 +213,13 @@ class WireHttpMulti extends WireHttp {
$this->queue = [];
$nextSeq = 0;
- $ramp = min($this->maxConcurrent, count($pool));
- for($i = 0; $i < $ramp; $i++) {
- /** @var WireHttpRequestSpec $nextSpec */
- $nextSpec = array_shift($pool);
- $seq = $nextSeq++;
- $row = $this->spawn($mh, $nextSpec, $seq);
- if($row !== null) {
- $active[(int) $row['handle']] = $row;
- } else {
- $this->resultsBySeq[$seq] ??= $this->buildSpawnFailureResult($nextSpec);
- }
- }
-
$running = 0;
do {
+ // Fill available slots unconditionally, independent of completion events.
+ while(count($active) < $this->maxConcurrent && !empty($pool)) {
+ /** @var WireHttpRequestSpec $nextSpec */
+ $nextSpec = array_shift($pool);
+ $seq = $nextSeq++;
+ $row = $this->spawn($mh, $nextSpec, $seq);
+ if($row !== null) {
+ $active[(int) $row['handle']] = $row;
+ } else {
+ $this->resultsBySeq[$seq] ??= $this->buildSpawnFailureResult($nextSpec);
+ }
+ }
+
+ if(empty($active)) break;
+
curl_multi_exec($mh, $running);
while(($info = curl_multi_info_read($mh)) !== false) {
@@ -249,16 +241,6 @@ class WireHttpMulti extends WireHttp {
unset($active[$id]);
}
}
- if(!empty($pool)) {
- /** @var WireHttpRequestSpec $nextSpec */
- $nextSpec = array_shift($pool);
- $seq = $nextSeq++;
- $row = $this->spawn($mh, $nextSpec, $seq);
- if($row !== null) {
- $active[(int) $row['handle']] = $row;
- } else {
- $this->resultsBySeq[$seq] ??= $this->buildSpawnFailureResult($nextSpec);
- }
- }
}
if($running > 0) {
@@ -270,7 +252,7 @@ class WireHttpMulti extends WireHttp {
$this->log(sprintf('[WireHttpMulti] active=%d, queued=%d', count($active), count($pool)));
}
}
- } while($running > 0 || !empty($active));
+ } while($running > 0 || !empty($active) || !empty($pool));
/** @var array $active */
foreach($active as $row) {
WireHttpMulti::execute()has three interconnected bugs in itscurl_multiloop that cause queued HTTP requests to be silently discarded without ever being attempted.Bug 1 — Refill only fires on completion events. The logic that pulls new requests from the pool only runs inside the
curl_multi_info_read()loop. If every initialspawn()call fails (for example,curl_init()returns null), no handle is added to the multi handle, no completion event ever fires, and the remaining pool is silently dropped. Those requests receive no result object at all.Bug 2 — Exit condition doesn't check the pool. The
while($running > 0 || !empty($active))condition can become false when all active requests finish in a singlecurl_multi_info_read()pass, even if items remain in the pool. Those items are never attempted.Bug 3 — Duplicated fill logic. The initial ramp-up
forloop duplicates the spawn logic that also lives insidecurl_multi_info_read(), making the code harder to reason about and maintain.Fix
Restructure the loop so slot-filling runs unconditionally at the top of every iteration, decoupled from completion events, and extend the exit condition to include the pool:
do-whilebody so it runs on every iteration.curl_multi_info_read().if(empty($active)) breakguard after fill.|| !empty($pool)to the exit condition.-OpenCode Zen / Big Pickle