Update pool connection creation

Author: trowski

lib/AbstractConnection.php

@@ -58,11 +58,11 @@ private function send(string $methodName, ...$args): Promise {
                     yield $this->busy->promise();
                 }
 
-                return yield $this->handle->{$methodName}(...$args);
+                return yield ([$this->handle, $methodName])(...$args);
             });
         }
 
-        return $this->handle->{$methodName}(...$args);
+        return ([$this->handle, $methodName])(...$args);
     }
 
     /**

lib/AbstractPool.php

@@ -32,6 +32,9 @@ abstract class AbstractPool implements Pool {
     /** @var callable */
     private $push;
 
+    /** @var int */
+    private $pending = 0;
+
     /**
      * @return \Amp\Promise<\Amp\Postgres\Connection>
      *
@@ -72,10 +75,16 @@ public function getIdleConnectionCount(): int {
 
     /**
      * @param \Amp\Postgres\Connection $connection
+     *
+     * @throws \Error if the connection is already part of this pool or if the connection is dead.
      */
     protected function addConnection(Connection $connection) {
         if (isset($this->connections[$connection])) {
-            return;
+            throw new \Error("Connection is already a part of this pool");
+        }
+
+        if (!$connection->isAlive()) {
+            throw new \Error("The connection is dead");
         }
 
         $this->connections->attach($connection);
@@ -92,17 +101,13 @@ protected function addConnection(Connection $connection) {
      * @resolve \Amp\Postgres\Connection
      */
     private function pop(): \Generator {
-        while ($this->promise !== null) {
-            try {
-                yield $this->promise; // Prevent simultaneous connection creation.
-            } catch (\Throwable $exception) {
-                // Ignore failure or cancellation of other operations.
-            }
+        while ($this->promise !== null && $this->connections->count() + $this->pending >= $this->getMaxConnections()) {
+            yield $this->promise; // Prevent simultaneous connection creation when connection count is at maximum - 1.
         }
 
         while ($this->idle->isEmpty()) { // While loop to ensure an idle connection is available after promises below are resolved.
             try {
-                if ($this->connections->count() >= $this->getMaxConnections()) {
+                if ($this->connections->count() + $this->pending >= $this->getMaxConnections()) {
                     // All possible connections busy, so wait until one becomes available.
                     $this->deferred = new Deferred;
                     yield $this->promise = $this->deferred->promise(); // May be resolved with defunct connection.

test/AbstractPoolTest.php

@@ -27,12 +27,7 @@ abstract protected function createPool(array $connections): Pool;
      */
     protected function createConnection(): Connection {
         $mock = $this->createMock(Connection::class);
-        $mock->method('isAlive')
-            ->willReturnCallback(static function () {
-                static $count = 0;
-                return $count++ < 3; // Force defunct connection after 3 operations.
-            });
-
+        $mock->method('isAlive')->willReturn(true);
         return $mock;
     }
 

test/AggregatePoolTest.php

@@ -3,6 +3,7 @@
 namespace Amp\Postgres\Test;
 
 use Amp\Postgres\AggregatePool;
+use Amp\Postgres\Connection;
 use Amp\Postgres\Pool;
 
 class AggregatePoolTest extends AbstractPoolTest {
@@ -47,4 +48,26 @@ public function testGetIdleConnectionCount() {
         $promise = $pool->query("SELECT 1");
         $this->assertSame(1, $pool->getIdleConnectionCount());
     }
+
+    /**
+     * @expectedException \Error
+     * @expectedExceptionMessage Connection is already a part of this pool
+     */
+    public function testDoubleAddConnection() {
+        $pool = $this->createPool([]);
+        $connection = $this->createConnection();
+        $pool->addConnection($connection);
+        $pool->addConnection($connection);
+    }
+
+    /**
+     * @expectedException \Error
+     * @expectedExceptionMessage The connection is dead
+     */
+    public function testAddDeadConnection() {
+        $pool = $this->createPool([]);
+        $connection = $this->createMock(Connection::class);
+        $connection->method('isAlive')->willReturn(false);
+        $pool->addConnection($connection);
+    }
 }