diff --git a/async_postgres/pg_pool.nim b/async_postgres/pg_pool.nim index 98efa80..c485875 100644 --- a/async_postgres/pg_pool.nim +++ b/async_postgres/pg_pool.nim @@ -821,17 +821,13 @@ proc acquireImpl(pool: PgPool): Future[AcquireResult] {.async.} = while pool.idle.len > 0: let pc = pool.idle.popFirst() if pc.conn.state != csReady or pc.conn.socketHasFin(): - pool.metrics.closeCount.inc - await pool.tracedClose(pc.conn) - if pool.closed: - raisePoolClosed() + # closeNoWait: avoid an await point where a cancellation could be + # swallowed by tracedClose and leak the next acquired conn (see ping guard). + pool.closeNoWait(pc.conn) continue if pool.config.maxLifetime > ZeroDuration and now - pc.conn.createdAt > pool.config.maxLifetime: - pool.metrics.closeCount.inc - await pool.tracedClose(pc.conn) - if pool.closed: - raisePoolClosed() + pool.closeNoWait(pc.conn) continue # Health check: ping connections that have been idle too long. # TLS connections use the tighter `tlsHealthCheckTimeout` window because @@ -880,10 +876,7 @@ proc acquireImpl(pool: PgPool): Future[AcquireResult] {.async.} = raise e except CatchableError: pool.active.dec - pool.metrics.closeCount.inc - await pool.tracedClose(pc.conn) - if pool.closed: - raisePoolClosed() + pool.closeNoWait(pc.conn) continue # A close() during the ping didn't drain the popped conn — discard it. if pool.closed: diff --git a/tests/test_pool.nim b/tests/test_pool.nim index b897536..b942787 100644 --- a/tests/test_pool.nim +++ b/tests/test_pool.nim @@ -597,6 +597,34 @@ suite "Pool acquire": check pool.active == 1 check pool.idle.len == 0 + test "acquire disposes idle broken conn via closeNoWait (no await point)": + # Regression: `await tracedClose` here would let a caller's cancellation + # be swallowed by tracedClose's `except CatchableError`, leaking the + # next-acquired conn to a departed caller. + let pool = makePool() + let broken = mockConn(csClosed) + let good = mockConn(csReady) + pool.idle.addLast(broken.toPooled()) + pool.idle.addLast(good.toPooled()) + + let acquired = waitFor pool.acquire() + check acquired == good + check pool.pendingBackgroundTasks.len >= 1 + + test "acquire disposes maxLifetime-expired idle conn via closeNoWait": + let pool = makePool() + pool.config.maxLifetime = seconds(1) + let expired = mockConn() + expired.createdAt = Moment.now() - seconds(5) + expired.state = csClosed + let good = mockConn() + pool.idle.addLast(expired.toPooled()) + pool.idle.addLast(good.toPooled()) + + let acquired = waitFor pool.acquire() + check acquired == good + check pool.pendingBackgroundTasks.len >= 1 + test "acquire registers waiter when at max": let pool = makePool(maxSize = 1) pool.active = 1