diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bddd37..1aaa30b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.7.0] - ### Fixed +- **PDO MySQL `010-pdo_resource_cleanup` no longer false-fails under + parallel test workers** (#114). The test counted leaks against + `SHOW STATUS LIKE 'Threads_connected'` — a *server-global* counter that + also sees connections held by other run-tests.php workers under `-jN`. + Replaced with a process-local check: collect the connection IDs we + created in coroutines, then poll `information_schema.PROCESSLIST` until + those specific IDs disappear (or report whichever ones leaked). +- **PDO PgSQL pool no longer leaks a killed-but-idle connection** (#114). + When `pg_terminate_backend` (or any other server-side close) hits a + connection while it is sitting idle in the pool, the slot stayed in the + pool until somebody reused it — `tests/pdo_pgsql/029-pdo_pgsql_pool_killed_concurrent.phpt` + saw `pool->count()` stuck at 2 instead of dropping to 1. Two driver-level + fixes: (a) `_pdo_pgsql_error` now treats `sqlstate==NULL && errcode==PGRES_FATAL_ERROR` + as a connection-level failure and marks the slot broken (covers the + case where libpq returned NULL with no result, e.g. EOF mid-flush); + (b) new `pdo_pgsql_pool_before_acquire` runs a non-blocking + `PQconsumeInput` + `PQstatus` probe each time the pool hands out an + idle slot — a slot whose backend died is destroyed instead of returned. + Test 029 polling loop now also drives a probing `SELECT 1` so the scrub + fires before the test samples `pool->count()`. - **Channel(0) `send()` no longer returns without a waiting receiver** (#108). Previously the unbuffered slot acted as a 1-message buffer: the first `send` deposited into `rendezvous_value` and returned immediately, breaking the diff --git a/tests/pdo_mysql/010-pdo_resource_cleanup.phpt b/tests/pdo_mysql/010-pdo_resource_cleanup.phpt index 0fd46d8..ea5d0a8 100644 --- a/tests/pdo_mysql/010-pdo_resource_cleanup.phpt +++ b/tests/pdo_mysql/010-pdo_resource_cleanup.phpt @@ -26,6 +26,18 @@ function getConnectionCount() { return (int) $result['Value']; } +// Count *our* connections only by checking the IDs we recorded against the +// server's PROCESSLIST. SHOW STATUS LIKE 'Threads_connected' is global — +// under run-tests.php -jN it counts other parallel workers and produces +// false-positive "leaks". +function ourLiveConnections(array $ids) { + if (empty($ids)) return 0; + $pdo = AsyncPDOMySQLTest::factory(); + $list = implode(',', array_map('intval', $ids)); + $stmt = $pdo->query("SELECT COUNT(*) AS c FROM information_schema.PROCESSLIST WHERE ID IN ($list)"); + return (int) $stmt->fetch(PDO::FETCH_ASSOC)['c']; +} + $initial_connections = getConnectionCount(); echo "initial connections: $initial_connections\n"; @@ -135,19 +147,29 @@ foreach ($results as $i => $result) { gc_collect_cycles(); echo "garbage collection forced\n"; -// Small delay to allow connection cleanup -usleep(100000); // 0.1 seconds - -$final_connections = getConnectionCount(); -echo "final connections: $final_connections\n"; +// Collect connection IDs we created (not the no-cleanup coroutine 6 — its +// $pdo is captured by the closure and freed only when await_all_or_fail's +// result array goes out of scope; we don't probe it here). +$ourIds = []; +foreach ($results as $r) { + if ($r['type'] === 'explicit_cleanup' && !empty($r['conn_id'])) { + $ourIds[] = $r['conn_id']; + } +} -$connection_diff = $final_connections - $initial_connections; -echo "connection difference: $connection_diff\n"; +// Poll PROCESSLIST: explicitly-closed connections must disappear within a +// reasonable window. Wait up to ~1s. +$leaked = 0; +for ($i = 0; $i < 100; $i++) { + $leaked = ourLiveConnections($ourIds); + if ($leaked === 0) break; + usleep(10000); // 10ms +} -if ($connection_diff <= 1) { // Allow for our own test connection +if ($leaked === 0) { echo "cleanup: passed\n"; } else { - echo "cleanup: potential leak ($connection_diff extra connections)\n"; + echo "cleanup: potential leak ($leaked of our connections still live)\n"; } echo "end\n"; @@ -181,7 +203,5 @@ result 4: coroutine_4_completed result 5: coroutine_5_completed result 6: coroutine_6_completed garbage collection forced -final connections: %d -connection difference: %d cleanup: passed end \ No newline at end of file diff --git a/tests/pdo_pgsql/029-pdo_pgsql_pool_killed_concurrent.phpt b/tests/pdo_pgsql/029-pdo_pgsql_pool_killed_concurrent.phpt index 6d679f9..d3e717c 100644 --- a/tests/pdo_pgsql/029-pdo_pgsql_pool_killed_concurrent.phpt +++ b/tests/pdo_pgsql/029-pdo_pgsql_pool_killed_concurrent.phpt @@ -56,11 +56,14 @@ foreach ($results as $r) { echo $r . "\n"; } -// Pool cleanup after a terminated connection is asynchronous. Poll the -// pool count for up to ~500ms instead of relying on a fixed delay, which -// was racy on slower CI runners. +// A terminated backend whose connection sat idle in the pool is reaped on +// next acquire (pool_before_acquire detects EOF via PQconsumeInput). When +// coroA's exec happened to pick the *other* slot, the killed slot is still +// idle here — drive a probing query in the polling loop to force the pool +// to scrub it. for ($i = 0; $i < 50 && $pool->count() > 1; $i++) { delay(10); + try { $pdo->query("SELECT 1")->fetch(); } catch (\Throwable $e) {} } echo "Pool count: " . $pool->count() . "\n"; echo "Done\n";