diff --git a/src/Adapter/ConnectionPool.php b/src/Adapter/ConnectionPool.php index aa6ea5d..c406b44 100644 --- a/src/Adapter/ConnectionPool.php +++ b/src/Adapter/ConnectionPool.php @@ -89,7 +89,22 @@ public function pop(float $timeout = -1): \PDO if ($this->created->cmpset($current, $current + 1)) { // We won the race — create and return the new connection // without pushing it to the channel first. - return ($this->factory)(); + try { + return ($this->factory)(); + } catch (\Throwable $e) { + // The factory (a real PDO connect) can throw: a transient + // network blip, MySQL momentarily refusing, an auth hiccup. + // The slot was already claimed by the cmpset above, so it + // MUST be released here — nothing else ever decrements + // `created`. Without this rollback every failed connect + // permanently burns a slot; after `size` failures the pool + // is full-but-empty forever, so pop() falls through to the + // Channel->pop() below and (with timeout -1) blocks every + // caller indefinitely. That ratcheting deadlock took down + // production with all workers asleep in pop(). + $this->created->sub(1); + throw $e; + } } } @@ -150,7 +165,16 @@ public function fill(): void while ($current < $this->size) { if ($this->created->cmpset($current, $current + 1)) { - $pool->push(($this->factory)()); + // Same slot-leak guard as pop(): if the factory throws (DB not + // yet reachable at worker boot) release the claimed slot before + // propagating, so a later retry/pop() can still fill the pool + // instead of it being permanently short one connection. + try { + $pool->push(($this->factory)()); + } catch (\Throwable $e) { + $this->created->sub(1); + throw $e; + } } $current = $this->created->get(); diff --git a/tests/Unit/Adapter/ConnectionPoolTest.php b/tests/Unit/Adapter/ConnectionPoolTest.php index 8e31629..cbd111f 100644 --- a/tests/Unit/Adapter/ConnectionPoolTest.php +++ b/tests/Unit/Adapter/ConnectionPoolTest.php @@ -139,6 +139,58 @@ public function it_resets_state_even_on_cleanup_error(): void }); } + #[Test] + public function pop_releases_the_claimed_slot_when_the_factory_throws(): void + { + if (!class_exists(\Swoole\Coroutine::class)) { + self::markTestSkipped('Swoole extension is required.'); + } + + Coroutine\run(function () { + $fail = true; + $pool = new ConnectionPool(1, static function () use (&$fail): \PDO { + if ($fail) { + // Simulate a transient PDO connect failure — a network blip + // or MySQL momentarily refusing the connection. + throw new \PDOException('simulated connect failure'); + } + return new \PDO('sqlite::memory:'); + }); + + $ref = new \ReflectionClass($pool); + $createdProp = $ref->getProperty('created'); + $createdProp->setAccessible(true); + + // Repeated connect failures must NOT accumulate on the slot counter. + // Before the fix each failure leaked one slot (cmpset ran, the factory + // threw, nothing rolled it back); after `size` failures the pool was + // full-but-empty and every pop() blocked forever on the Channel — the + // production ratcheting deadlock this guards against. + for ($i = 0; $i < 5; ++$i) { + try { + $pool->pop(); + self::fail('Expected the factory exception to propagate.'); + } catch (\PDOException $e) { + self::assertSame('simulated connect failure', $e->getMessage()); + } + + self::assertSame( + 0, + $createdProp->getValue($pool)->get(), + 'A failed factory call must release the slot it optimistically claimed.', + ); + } + + // Not wedged: once the factory recovers, pop() creates a real + // connection via the fast path instead of blocking on an empty + // Channel (which, with timeout -1, would hang forever). + $fail = false; + $conn = $pool->pop(); + self::assertInstanceOf(\PDO::class, $conn); + self::assertSame(1, $createdProp->getValue($pool)->get()); + }); + } + protected function tearDown(): void { if (class_exists(\Swoole\Runtime::class)) {