From 041533d4237fc93abaddf19afee3ca9fed4481bf Mon Sep 17 00:00:00 2001
From: Demin Yin <deminy@deminy.net>
Date: Fri, 1 Mar 2024 13:59:10 -0800
Subject: [PATCH] #170: misc updates after the PR gets merged

---
 CHANGELOG.md                        |  1 +
 src/__init__.php                    |  1 +
 src/core/ConnectionPool.php         |  6 +++++
 src/core/Database/PDOPool.php       | 12 +++++++---
 tests/unit/Database/PDOPoolTest.php | 37 +++++++++++++++++------------
 5 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 80bc563..5540ad9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 Fixed:
 
 * MR swoole/library#169: Fix broken requests when keep-alive is turned on in the FastCGI client. (by @NathanFreeman)
+* MR swoole/library#170: Enhance database pool stability by verifying PDO connection existence while fetching. (by @DevZer0x00)
 * Fix accessing undefined properties in method \Swoole\NameResolver::checkResponse(). ([commit](https://github.com/swoole/library/commit/7a6396e45f4d4517a049584a746285d6501cf71d))
 * Fix the implementation of method `\Swoole\MultibyteStringObject::chunk()`. ([commit](https://github.com/swoole/library/commit/031eba5f6db2ffac66ce1cca6d1d63a213203724))
 * Connection pool in Swoole does not support in-memory or temporary SQLite databases. ([commit](https://github.com/swoole/library/commit/eaf6a43f2fdd403e7d4968fd6f4bd0d1b05e48c3))
diff --git a/src/__init__.php b/src/__init__.php
index 669b2c2..d41b566 100644
--- a/src/__init__.php
+++ b/src/__init__.php
@@ -24,6 +24,7 @@
         'core/StringObject.php',
         'core/MultibyteStringObject.php',
         'core/Exception/ArrayKeyNotExists.php',
+        'core/Exception/TimeoutException.php',
         'core/ArrayObject.php',
         'core/ObjectProxy.php',
         'core/Coroutine/WaitGroup.php',
diff --git a/src/core/ConnectionPool.php b/src/core/ConnectionPool.php
index 61ceccc..3904885 100644
--- a/src/core/ConnectionPool.php
+++ b/src/core/ConnectionPool.php
@@ -39,6 +39,12 @@ public function fill(): void
         }
     }
 
+    /**
+     * Get a connection from the pool.
+     *
+     * @param float $timeout > 0 means waiting for the specified number of seconds. other means no waiting.
+     * @return mixed|false Returns a connection object from the pool, or false if the pool is full and the timeout is reached.
+     */
     public function get(float $timeout = -1)
     {
         if ($this->pool === null) {
diff --git a/src/core/Database/PDOPool.php b/src/core/Database/PDOPool.php
index c8e2d2d..8987c2a 100644
--- a/src/core/Database/PDOPool.php
+++ b/src/core/Database/PDOPool.php
@@ -32,13 +32,19 @@ public function __construct(protected PDOConfig $config, int $size = self::DEFAU
         }, $size, PDOProxy::class);
     }
 
+    /**
+     * Get a PDO connection from the pool. The PDO connection (a PDO object) is wrapped in a PDOProxy object returned.
+     *
+     * @param float $timeout > 0 means waiting for the specified number of seconds. other means no waiting.
+     * @return PDOProxy|false Returns a PDOProxy object from the pool, or false if the pool is full and the timeout is reached.
+     *                        {@inheritDoc}
+     */
     public function get(float $timeout = -1)
     {
-        /* @var \Swoole\Database\PDOProxy|bool $pdo */
+        /* @var \Swoole\Database\PDOProxy|false $pdo */
         $pdo = parent::get($timeout);
-
         if ($pdo === false) {
-            throw new TimeoutException();
+            throw new TimeoutException('Failed to get a PDO connection: The pool is at full capacity, yet all connections are currently in use.');
         }
 
         $pdo->reset();
diff --git a/tests/unit/Database/PDOPoolTest.php b/tests/unit/Database/PDOPoolTest.php
index bc7b4a8..a85408b 100644
--- a/tests/unit/Database/PDOPoolTest.php
+++ b/tests/unit/Database/PDOPoolTest.php
@@ -218,11 +218,12 @@ public function testSqlite(): void
         });
     }
 
-    public function testTimeoutException()
+    public function testTimeoutException(): void
     {
         self::saveHookFlags();
         self::setHookFlags(SWOOLE_HOOK_ALL);
-        run(function () use (&$timeoutOccured) {
+        $failed = false;
+        run(function () use (&$failed) {
             $config = (new PDOConfig())
                 ->withHost(MYSQL_SERVER_HOST)
                 ->withPort(MYSQL_SERVER_PORT)
@@ -232,24 +233,30 @@ public function testTimeoutException()
                 ->withPassword(MYSQL_SERVER_PWD)
             ;
 
-            $pool = new PDOPool($config, 1);
-            $timeoutOccured = false;
-            go(function () use ($pool, &$timeoutOccured) {
+            $pool      = new PDOPool($config, 1);
+            $waitGroup = new WaitGroup(2); // A wait group to wait for the next 2 coroutines to finish.
+
+            go(function () use ($pool, $waitGroup) {
+                $pool->get()->exec('SELECT SLEEP(1)'); // Hold the connection for 1 second before putting it back into the pool.
+                $waitGroup->done();
+            });
+
+            go(function () use ($pool, $waitGroup, &$failed) {
+                Coroutine::sleep(0.1); // Sleep for 0.1 second to ensure the 1st connection is in use by the 1st coroutine.
                 try {
-                    $pool->get(2);
+                    $pool->get(0.5); // Try to get a 2nd connection from the pool within 0.5 seconds.
                 } catch (TimeoutException) {
-                    $timeoutOccured = true;
+                    $failed = true;
+                } finally {
+                    $waitGroup->done();
                 }
             });
 
-            go(function () use ($pool) {
-                $pdo = $pool->get(1);
-                $pdo->exec('SELECT SLEEP(2)');
-            });
-        });
-
-        $this->assertTrue($timeoutOccured);
+            $waitGroup->wait();
+            $pool->close();
+            self::restoreHookFlags();
 
-        self::restoreHookFlags();
+            self::assertTrue($failed, 'Failed to get a 2nd connection from the pool within 0.5 seconds');
+        });
     }
 }