Skip to content

Keep a terminated reserved connection out of the pool (fixes #1199) - #1240

Open
Mariomarquezt wants to merge 1 commit into
porsager:masterfrom
Mariomarquezt:fix-1199-dead-reserved
Open

Mariomarquezt wants to merge 1 commit into
porsager:masterfrom
Mariomarquezt:fix-1199-dead-reserved

Conversation

@Mariomarquezt

Copy link
Copy Markdown

Fixes #1199.

The defect

When the server terminates the backend of a reserved connection, onclose() moves the connection to the closed queue and sets c.reserved = null. It can also reconnect it at once for a waiting query. The ReservedSql handle does not know any of this:

The fix

onclose() calls a closed hook on the reservation before it clears c.reserved. The hook records the close error and rejects the handle's own overflow queue. After that, the handle is dead for good:

  • queries on it reject with the close error (CONNECTION_CLOSED);
  • release() does nothing, because the connection already belongs to the pool's closed queue, which reconnects it as usual.

Healthy reservations do not change.

Tests

Two new tests in tests/index.js against a real server:

  • Reserved connection rejects queries after its backend is terminated
  • Releasing a terminated reserved connection does not return it to the pool

On a CI-like setup (PostgreSQL 17 with the workflow's pg_hba.conf, ssl=on, wal_level=logical, max_prepared_transactions=100, and a second server on 5433; Node 20):

  • without the fix, node tests/index.js dies with the uncaught TypeError above;
  • with the fix, all 266 tests pass in test:esm and in test:cjs (264 existing + 2 new).

Deno was not run locally.

Related open PRs

These fix nearby parts of the same close path and do not overlap with this change: #1209 (guard nextWrite() itself), #1215 (reject transaction work after its connection closes), #1229 (a queued reserve() stranded by a termination). This PR only covers the reserved handle and release().

We have run this change as a local patch against 3.4.9 in production since 2026-09-30.

🤖 Generated with Claude Code

When the server terminates the backend of a reserved connection, onclose
moves the connection to the closed queue, and may reconnect it at once
for a waiting query. The ReservedSql handle still pointed at it:

- later queries on the handle called execute() on a connection with a
  null socket, so nextWrite() threw an uncaught TypeError and the query
  never settled;
- release() called onopen() and put the dead connection in the open
  queue, so the next pool query hung (porsager#1199).

onclose now marks the handle closed. Its queued queries are rejected,
later queries reject with the close error, and release() leaves the
connection with the pool, which reconnects it as usual.

Fixes porsager#1199
@GrahamCampbell

Copy link
Copy Markdown

reserve() and begin(): a reserved connection keeps being used after the server ends its session

Versions. postgres 3.4.9, Node 24.19.0, PostgreSQL 17.11 (the postgres:17 image), no TLS.

Related. #1199, #1208 and #1209, #1240, #1246, #1195 and #1229, #1216 and #1215, #1223, #1248. This is one more data point for this PR, #1246 and #1215 (#1248 reports the transaction case too), from a session-level advisory lock held on a reserved connection, with two consequences we did not see described: release() through the stale handle takes over another borrower's reservation, and a second statement in flight on the handle is lost in its private queue. A self-contained script below shows each case against a plain server.

What happens

A ReservedSql handle is not told when the server ends its session. onclose (src/index.js:421-427) moves the connection to closed, clears c.reserved and, if anything is queued, reconnects it for that borrower at once, but the handle keeps its handler and release (src/index.js:219-231). What the handle does next depends on the connection's state at that moment:

  1. Still closed. execute writes through a null socket from an Immediate, so TypeError: Cannot read properties of null (reading 'write') is thrown at src/connection.js:255 outside any promise, and the statement never settles. If its text is not prepared on that connection, the handler also moves the connection to full (src/index.js:230), where nothing moves it out again, so the pool has lost it. If it is prepared, its bytes stay in chunk and the next reconnect's startup message is never sent, so later borrowers of that connection fail with CONNECT_TIMEOUT.
  2. Reconnected for another borrower. The statement runs on that borrower's session (an advisory unlock answers false with a warning). Then the handle's release() (src/index.js:220-223) sets c.reserved = null and calls onopen(c). If the new borrower is a reserve(), its reservation is overwritten: statements it has queued are never run, and its session goes back into the pool while it still holds session state, such as an advisory lock, which the idle timeout can then end under it. When every connection is reserved at the close, onclose re-lends the connection to the queued borrower immediately (src/index.js:426), which makes this the likely case.
  3. Two statements in flight on the handle. The first moves the connection to full, so the second waits in the handle's private queue (src/index.js:204, :228-229), which only c.reserved reads. onclose clears c.reserved, so the second never settles.

Separately, closed() (src/connection.js:436-458) leaves the ended session behind:

  1. After a session ends under a statement. query and errorResponse (the FATAL) survive the close. The first ReadyForQuery after the reconnect replays the FATAL onto initial (src/connection.js:536-542), so the next borrower of that connection fails with 57P01 terminating connection due to administrator command without being sent. If that borrower is reserve(), its pseudo-query is also still in queries; onopen resolves the already-rejected promise (src/index.js:410-411) and the connection stays in connecting, serving nobody, until max_lifetime ends it.
  2. After a TCP reset. On Node the socket emits error before close. A statement sent from the rejection handler in between goes into sent, and closed(true) does not reject it (src/connection.js:453). After the reconnect, ReadyForQuery promotes it to query (src/connection.js:573-577), so the connection serves one borrower and then waits for a reply that never comes.

And begin() has the reserved handle's gap without its release() (#1216, which #1215 fixes; #1248 reports it too):

  1. A session ends under a statement in a transaction. A statement queued behind it in the transaction (the first, being parameterised, moved the connection to full) waits in the transaction's private queue, which nothing reads after the close, so it never settles, as in case 3. begin() rejects at once through connection.onclose (src/index.js:245), but the callback is not stopped, and the scope's catch sends rollback (src/index.js:267-271) through the transaction's handler (:291-296) to the closed connection. It is written through the null socket from an Immediate, the same TypeError as case 1, thrown before nextWrite clears chunk and nextWriteTimer (src/connection.js:254-259). The reconnect's startup message is appended behind the rollback's bytes and never written, so the next borrowers of that connection wait out connect_timeout and fail with CONNECT_TIMEOUT (within the script's 3 s wait they are still pending). A callback still running after the close does the same with its next statement, or, once the pool has reconnected the connection for someone else, runs it on that session, outside the transaction (case 8).

And closed() cancels a pending write but keeps its bytes (#1215 clears them):

  1. A statement handed to a connection in the turn its socket closes. Its write waits for the next check phase (src/connection.js:250), so the close callback runs first and cancels it (:440), but chunk and nextWriteTimer survive. The reconnect's startup message is appended behind the statement's bytes and never sent, so the next borrowers of that connection fail with CONNECT_TIMEOUT.

Every case is traced from the source above, and the output below is the script's own, run once on 3.4.9 and once with the change.

Reproduction

Only postgres and node:net, against a throwaway server started from the stock image:

docker run --rm -d --name pg-reserve-repro -p 127.0.0.1:55432:5432 -e POSTGRES_PASSWORD=repro postgres:17
node repro.mjs
import net from 'node:net'
import postgres from 'postgres'

const PORT = 55432
const admin = postgres({ host: '127.0.0.1', port: PORT, user: 'postgres', pass: 'repro', database: 'postgres', max: 1, onnotice() {} })
const sleep = ms => new Promise(r => setTimeout(r, ms))
const settle = (p, ms = 3000) => Promise.race([
  Promise.resolve(p).then(x => 'resolved ' + JSON.stringify(x), e => 'rejected ' + (e.code || e.message)),
  sleep(ms).then(() => `STILL PENDING after ${ ms } ms`)
])
process.on('uncaughtException', e => console.log('  uncaughtException:', e.message))

// A relay in front of the server, so case 5 can reset the client's socket.
const sockets = new Set()
const relay = net.createServer(client => {
  const server = net.connect(PORT, '127.0.0.1')
  client.on('error', () => server.destroy())
  server.on('error', () => client.destroy())
  client.on('close', () => (server.destroy(), sockets.delete(client)))
  client.pipe(server).pipe(client)
  sockets.add(client)
})
await new Promise(r => relay.listen(0, '127.0.0.1', r))
const pool = () => postgres({ host: '127.0.0.1', port: relay.address().port, user: 'postgres', pass: 'repro', database: 'postgres', max: 1, onnotice() {} })

const pid = async sql => (await sql`select pg_backend_pid() as pid`)[0].pid
const end = async p => { await admin`select pg_terminate_backend(${ p })`; await sleep(300) }
const waiting = async key => {
  for (;;) {
    const [row] = await admin`select pid from pg_locks where locktype = 'advisory' and not granted and objid = ${ key }`
    if (row) return row.pid
    await sleep(20)
  }
}

console.log('1. The session ends while the reserved connection is idle; then a statement on the handle')
{
  const sql = pool()
  const conn = await sql.reserve()
  await end(await pid(conn))
  console.log('  statement on the handle:', await settle(conn`select ${ 1 }::int as x`))
  console.log('  next pool query        :', await settle(sql`select 1 as x`))
}

console.log('2. The session ends while reserved and idle; the pool reserves the connection again; then the old handle')
{
  const sql = pool()
  const stale = await sql.reserve()
  await end(await pid(stale))
  const next = await sql.reserve() // max: 1, so the same connection, reconnected
  const nextPid = await pid(next)
  console.log('  old handle on next\'s session  :', await settle(stale`select pg_backend_pid() = ${ nextPid } as same`))
  stale.release()
  console.log('  pool query while next holds it:', await settle(sql`select pg_backend_pid() = ${ nextPid } as same`))
}

console.log('3. Two statements in flight on a reserved connection when its session ends')
{
  const sql = pool()
  await admin`select pg_advisory_lock(${ 3 })`
  const conn = await sql.reserve()
  const first = settle(conn`select pg_advisory_lock(${ 3 }) as waits`, 5000)
  const second = settle(conn`select 1 as queued`, 5000)
  await end(await waiting(3))
  console.log('  first :', await first)
  console.log('  second:', await second)
  await admin`select pg_advisory_unlock(${ 3 })`
}

console.log('4. A session ends under a statement; then the next borrower of that connection')
{
  const sql = pool()
  await admin`select pg_advisory_lock(${ 4 })`
  const conn = await sql.reserve()
  const first = settle(conn`select pg_advisory_lock(${ 4 }) as waits_again`, 5000)
  await end(await waiting(4))
  console.log('  statement in flight:', await first)
  await admin`select pg_advisory_unlock(${ 4 })`
  console.log('  next reserve()     :', await settle(sql.reserve().then(c => (c.release(), 'reserved'))))
  console.log('  then a pool query  :', await settle(sql`select 1 as x`))
}

console.log('5. A reset under a statement; a statement sent from the rejection handler; then two pool queries')
{
  const sql = pool()
  await admin`select pg_advisory_lock(${ 5 })`
  const conn = await sql.reserve()
  let fromHandler
  const first = conn.unsafe('select pg_advisory_lock(5)').catch(e => {
    fromHandler = settle(conn`select 1 as after_reset`) // after the socket's error, before its close
    return 'rejected ' + e.code
  })
  await waiting(5)
  for (const s of sockets) s.resetAndDestroy()
  console.log('  statement in flight  :', await first)
  console.log('  sent from the handler:', await fromHandler)
  await admin`select pg_advisory_unlock(${ 5 })`
  console.log('  pool query 1         :', await settle(sql`select 1 as x`))
  console.log('  pool query 2         :', await settle(sql`select 2 as x`))
}

console.log('6. A session ends under a statement in a transaction, with a second statement queued behind it; then two pool queries')
{
  const sql = pool()
  await admin`select pg_advisory_lock(${ 6 })`
  let queued
  const tx = settle(sql.begin(t => {
    const first = t`select pg_advisory_lock(${ 6 }) as waits_in_transaction`.execute()
    queued = settle(t`select 1 as queued`, 5000)
    return first
  }), 5000)
  await end(await waiting(6))
  console.log('  begin()     :', await tx)
  console.log('  queued      :', await queued)
  await admin`select pg_advisory_unlock(${ 6 })`
  console.log('  pool query 1:', await settle(sql`select 1 as x`))
  console.log('  pool query 2:', await settle(sql`select 2 as x`))
}

console.log('7. A statement handed to a connection in the turn its socket closes; then a pool query')
{
  let socket
  const sql = postgres({ host: '127.0.0.1', port: PORT, user: 'postgres', pass: 'repro', database: 'postgres', max: 1, connect_timeout: 3, onnotice() {}, socket: () => (socket = net.connect(PORT, '127.0.0.1')) })
  await sql`select 1 as x`
  const handed = new Promise(r => setImmediate(() => {
    socket.destroy()
    r(settle(sql`select 1 as handed`)) // its write waits for the next check phase, after the close
  }))
  console.log('  statement handed over:', await handed)
  console.log('  pool query           :', await settle(sql`select 2 as x`, 5000))
}

console.log('8. The session ends while a transaction\'s callback awaits a timer; the pool reserves the connection again; then the callback\'s next statement')
{
  const sql = pool()
  let nextPid, gotPid, resumed
  const txPid = new Promise(r => gotPid = r)
  const late = new Promise(r => resumed = r)
  const tx = settle(sql.begin(async t => {
    gotPid(await pid(t))
    await sleep(1000)
    resumed(settle(t`select pg_backend_pid() = ${ nextPid } as same`))
  }), 5000)
  await end(await txPid)
  console.log('  begin()                   :', await tx)
  const next = await sql.reserve()
  nextPid = await pid(next)
  console.log('  callback on next\'s session:', await late)
}

process.exit(0)

Each case uses a pool of its own with max: 1, so the connection whose session ended is the one the next borrower gets; case 7's takes its socket from the socket option, so the script can close it, and a 3 s connect_timeout, so a reconnect that sends nothing fails within the wait.

On 3.4.9:

1. The session ends while the reserved connection is idle; then a statement on the handle
  uncaughtException: Cannot read properties of null (reading 'write')
  statement on the handle: STILL PENDING after 3000 ms
  next pool query        : STILL PENDING after 3000 ms
2. The session ends while reserved and idle; the pool reserves the connection again; then the old handle
  old handle on next's session  : resolved [{"same":true}]
  pool query while next holds it: resolved [{"same":true}]
3. Two statements in flight on a reserved connection when its session ends
  first : rejected CONNECTION_CLOSED
  second: STILL PENDING after 5000 ms
4. A session ends under a statement; then the next borrower of that connection
  statement in flight: rejected CONNECTION_CLOSED
  next reserve()     : rejected 57P01
  then a pool query  : STILL PENDING after 3000 ms
5. A reset under a statement; a statement sent from the rejection handler; then two pool queries
  statement in flight  : rejected ECONNRESET
  sent from the handler: STILL PENDING after 3000 ms
  pool query 1         : resolved [{"x":1}]
  pool query 2         : STILL PENDING after 3000 ms
6. A session ends under a statement in a transaction, with a second statement queued behind it; then two pool queries
  uncaughtException: Cannot read properties of null (reading 'write')
  begin()     : rejected CONNECTION_CLOSED
  queued      : STILL PENDING after 5000 ms
  pool query 1: STILL PENDING after 3000 ms
  pool query 2: STILL PENDING after 3000 ms
7. A statement handed to a connection in the turn its socket closes; then a pool query
  statement handed over: rejected CONNECTION_CLOSED
  pool query           : rejected CONNECT_TIMEOUT
8. The session ends while a transaction's callback awaits a timer; the pool reserves the connection again; then the callback's next statement
  begin()                   : rejected CONNECTION_CLOSED
  callback on next's session: resolved [{"same":true}]

With the change below:

1. The session ends while the reserved connection is idle; then a statement on the handle
  statement on the handle: rejected CONNECTION_CLOSED
  next pool query        : resolved [{"x":1}]
2. The session ends while reserved and idle; the pool reserves the connection again; then the old handle
  old handle on next's session  : rejected CONNECTION_CLOSED
  pool query while next holds it: STILL PENDING after 3000 ms
3. Two statements in flight on a reserved connection when its session ends
  first : rejected CONNECTION_CLOSED
  second: rejected CONNECTION_CLOSED
4. A session ends under a statement; then the next borrower of that connection
  statement in flight: rejected CONNECTION_CLOSED
  next reserve()     : resolved "reserved"
  then a pool query  : resolved [{"x":1}]
5. A reset under a statement; a statement sent from the rejection handler; then two pool queries
  statement in flight  : rejected ECONNRESET
  sent from the handler: rejected CONNECTION_CLOSED
  pool query 1         : resolved [{"x":1}]
  pool query 2         : resolved [{"x":2}]
6. A session ends under a statement in a transaction, with a second statement queued behind it; then two pool queries
  begin()     : rejected CONNECTION_CLOSED
  queued      : rejected CONNECTION_CLOSED
  pool query 1: resolved [{"x":1}]
  pool query 2: resolved [{"x":2}]
7. A statement handed to a connection in the turn its socket closes; then a pool query
  statement handed over: rejected CONNECTION_CLOSED
  pool query           : resolved [{"x":2}]
8. The session ends while a transaction's callback awaits a timer; the pool reserves the connection again; then the callback's next statement
  begin()                   : rejected CONNECTION_CLOSED
  callback on next's session: rejected CONNECTION_CLOSED

In case 2 the pool query still waits because next holds the pool's one connection, which the script never releases.

Proposed change

Three hunks. The first is #1240's change (its hook named close here), with one more guard: release() is also a no-op once the connection is no longer this handle's, which covers a second or late release(). The second is #1246's change to closed(), together with the state #1229 and #1215 clear and #1215's reset of the write buffer. The third is #1215's begin() change, line for line but for the flag's name (lost, as in the first hunk, for its closedError) and where it is declared; the patch in #1248's thread repeats it. The first alone leaves cases 4 to 8, the second alone cases 1 to 3, 6 and 8, and the third alone cases 1 to 7; case 6 needs the second and the third together, since with the third alone its first pool query still meets case 4's stale 57P01. cjs/src/ follows from the build.

--- a/src/index.js
+++ b/src/index.js
@@ -210,14 +210,22 @@
         closed.length && connect(closed.shift(), query)
       })
 
+    let lost = null
     move(c, reserved)
-    c.reserved = () => queue.length
+    const own = c.reserved = () => queue.length
       ? c.execute(queue.shift())
       : move(c, reserved)
-    c.reserved.release = true
+    own.release = true
+    own.close = e => {
+      lost = e
+      while (queue.length)
+        queue.shift().reject(e)
+    }
 
     const sql = Sql(handler)
     sql.release = () => {
+      if (lost || c.reserved !== own)
+        return
       c.reserved = null
       onopen(c)
     }
@@ -225,6 +233,8 @@
     return sql
 
     function handler(q) {
+      if (lost)
+        return q.reject(lost)
       c.queue === full
         ? queue.push(q)
         : c.execute(q) || move(c, full)
@@ -237,12 +247,18 @@
     let savepoints = 0
       , connection
       , prepare = null
+      , lost = null
 
     try {
       await sql.unsafe('begin ' + options.replace(/[^a-z ]/ig, ''), [], { onexecute }).execute()
       return await Promise.race([
         scope(connection, fn),
-        new Promise((_, reject) => connection.onclose = reject)
+        new Promise((_, reject) => connection.onclose = e => {
+          lost = e
+          while (queries.length)
+            queries.shift().reject(e)
+          reject(e)
+        })
       ])
     } catch (error) {
       throw error
@@ -290,6 +306,8 @@
 
       function handler(q) {
         q.catch(e => uncaughtError || (uncaughtError = e))
+        if (lost)
+          return q.reject(lost)
         c.queue === full
           ? queries.push(q)
           : c.execute(q) || move(c, full)
@@ -420,6 +438,7 @@
 
   function onclose(c, e) {
     move(c, closed)
+    c.reserved && c.reserved.close && c.reserved.close(e)
     c.reserved = null
     c.onclose && (c.onclose(e), c.onclose = null)
     options.onclose && options.onclose(c.id)
--- a/src/connection.js
+++ b/src/connection.js
@@ -438,6 +438,7 @@
     remaining = 0
     incomings = null
     clearImmediate(nextWriteTimer)
+    chunk = nextWriteTimer = null
     socket.removeListener('data', data)
     socket.removeListener('connect', connected)
     idleTimer.cancel()
@@ -450,7 +451,9 @@
     if (initial)
       return reconnect()
 
-    !hadError && (query || sent.length) && error(Errors.connection('CONNECTION_CLOSED', options, socket))
+    if (query || sent.length)
+      error(Errors.connection('CONNECTION_CLOSED', options, socket))
+    query = results = errorResponse = null
     closedTime = performance.now()
     hadError && options.shared.retries++
     delay = (typeof backoff === 'function' ? backoff(options.shared.retries) : backoff) * 1000

In the first hunk, e is the CONNECTION_CLOSED error closed() passes to onclose (src/connection.js:457), so everything the handle had queued, and everything sent through it afterwards, rejects with the code a caller already handles for a lost connection, and the connection stays with the pool's closed queue to be reconnected as usual. The hook runs before c.reserved is cleared and before the re-lend at src/index.js:426. begin()'s c.reserved has no close, so the first hunk leaves transactions to the third, and a healthy reservation never sets lost.

In the second, the statement in flight on a reset has already been rejected by the socket's error handler, so rejecting it again does nothing; what changes is that a statement that reached sent after that is rejected too. Clearing query and errorResponse stops the replay onto the reconnect's first borrower, and results is cleared with them, as #1215 does. Clearing chunk and nextWriteTimer, as #1215 does, covers a statement handed to a connection in the same turn of the event loop as its close, as when a restart ends every pooled session at once and a microtask after one connection's close callback sends a statement to another whose close callback is still pending: closed() cancels its write (src/connection.js:440) but keeps its bytes and its timer, so the reconnect's startup message is queued behind them and never sent, and the next borrowers of that connection fail with CONNECT_TIMEOUT. Whatever chunk holds at the close belongs to a statement in query or sent, which this hunk rejects, or to the startup message the reconnect writes again. Case 7 produces that race on demand, by destroying the socket and handing over a statement in one setImmediate callback. #1215 also renews result and rows there; we left them out because ReadyForQuery renews result, and a stale rows only reaches the array-type fetch the driver runs first on each new session when fetch_types is on, whose forEach skips the holes. #1215's version is the more complete.

In the third, onclose already calls connection.onclose with the same CONNECTION_CLOSED (src/index.js:424), so begin() still rejects at once. Once lost is set, the scope's rollback and anything the callback sends afterwards are rejected before c.execute, so nothing is written through the null socket or reaches the session the connection opens next, and what the transaction had queued while the connection was full is rejected at the close. Every scope, savepoints included, uses this handler, and, as in #1215, the guard comes after q.catch, so a statement the callback never awaits still has a handler. We prefer it to #1209's nextWrite guard, which settles the write after it is made: its error() also rejects initial, the borrower the pool may just have handed the connection to, and once the connection has reconnected, a callback that outlived the close still writes onto the next session.

The diff has not been run against the package's own test suite.

Not covered here

#1195 (a reserve() queued when the connection closes is handed to the reconnect as initial and dropped; #1229), #1223 and #1241 (a connection ended during startup), and #1209's nextWrite guard. With the hunks above we know of no other path, besides a cursor or COPY callback that outlives the close, that writes through a closed connection or leaves its bytes ahead of a reconnect. The script does not use TLS.

@GravyaDev

Copy link
Copy Markdown

The third bullet in the description, "a late query on the old handle could run on that caller's connection", is worth calling out on its own: it is a session-isolation issue, not only a hang or a crash. The stale handle reads and resets another caller's session state (settings, temp tables, an open transaction), and release() can hand that caller's live connection to a third one.

I measured it on master (411429e) and with this PR applied (7616069). Setup: Node 24.22, PostgreSQL 17 in Docker, max: 1.

Repro (standalone, node repro.mjs <path to src/index.js> <port> read|reset|release):

import { pathToFileURL } from 'node:url'

const [entry, port, mode] = process.argv.slice(2)
const { default: postgres } = await import(pathToFileURL(entry).href)
const opts = { host: '127.0.0.1', port: Number(port), user: 'postgres', password: 'postgres', database: 'postgres', onnotice: () => {} }

let closed
const closedOnce = new Promise(r => (closed = r))
const sql = postgres({ ...opts, max: 1, onclose: () => closed() })
const admin = postgres({ ...opts, max: 1 })
const timeout = (p, ms, label) => Promise.race([p, new Promise((_, rej) => setTimeout(() => rej(new Error(label + ' pending after ' + ms + 'ms')), ms))])

process.on('uncaughtException', e => { console.log('UNCAUGHT', e.message); process.exit(1) })

// A reserves, sets session state, and its backend is terminated.
const a = await sql.reserve()
const [{ pid: pidA }] = await a`select pg_backend_pid() as pid, set_config('app.marker', 'A', false)`
await admin`select pg_terminate_backend(${ pidA })`
await timeout(closedOnce, 10000, 'onclose')

// B reserves: the pool reconnects the same Connection object for it.
const b = await sql.reserve()
const [{ pid: pidB }] = await b`select pg_backend_pid() as pid, set_config('app.marker', 'B', false)`
await b`begin`
await b`create temp table b_marker (x int)`

const bState = async () => {
  const [s] = await b`select current_setting('app.marker', true) as marker, to_regclass('pg_temp.b_marker')::text as temp_table`
  s.in_transaction = await b`savepoint probe`.then(() => true, e => e.code === '25P01' ? false : e.code)
  return JSON.stringify(s)
}

try {
  if (mode === 'read') {
    const [r] = await timeout(a`select pg_backend_pid() as pid, current_setting('app.marker', true) as marker`, 3000, 'stale query')
    console.log(`STALE_QUERY_RAN on pid=${ r.pid } (B is ${ pidB }) and read app.marker=${ r.marker }`)
  } else if (mode === 'reset') {
    await timeout(a.unsafe('rollback; reset all'), 3000, 'stale query')
    console.log(`STALE_QUERY_RAN; B now sees ${ await bState() }`)
  } else if (mode === 'release') {
    a.release()
    const r = await Promise.race([sql.reserve(), new Promise(res => setTimeout(() => res(null), 1000))])
    if (r) {
      const [x] = await r`select pg_backend_pid() as pid, current_setting('app.marker', true) as marker`
      console.log(`STALE_RELEASE_REPOOLED: a third reserve() got pid=${ x.pid } (B is ${ pidB }) while B still holds it, app.marker=${ x.marker }`)
    } else {
      console.log('STALE_RELEASE_NOOP: a third reserve() stays pending while B holds the connection')
    }
  }
} catch (e) {
  console.log(`STALE_HANDLE_REJECTED ${ e.code || e.name }: ${ e.message }`)
}
process.exit(0)

Results

Mode master 411429e with this PR (7616069)
read STALE_QUERY_RAN on pid=92 (B is 92) and read app.marker=B STALE_HANDLE_REJECTED CONNECTION_CLOSED
reset STALE_QUERY_RAN; B now sees {"marker":"","temp_table":null,"in_transaction":false}: B's setting is cleared, its temp table is gone and its open transaction is rolled back STALE_HANDLE_REJECTED CONNECTION_CLOSED
release STALE_RELEASE_REPOOLED: a third reserve() got pid=98 (B is 98) while B still holds it, app.marker=B STALE_RELEASE_NOOP

So on master, after a backend of a reserved connection is terminated (a server restart, pg_terminate_backend, a network drop), the stale handle:

  • runs its queries on the next caller's session and sees its state;
  • can reset that session and roll back its transaction;
  • through release(), puts the other caller's in-use connection back in the pool, where a third caller shares it.

It takes a backend death while a handle is held, plus another reserve() (or a queued waiter) before the stale handle is used or released.

This PR closes all three paths in my runs. Thank you for it. I'd suggest adding the read and release cases as tests, since they pin the isolation property and not just "doesn't hang". I'd also love to see it merged and released, since 3.4.9 is affected.

(The crash variant of the same root cause is #1208 / #1209.)


Disclaimer: this comment and the repro in it were drafted by @kloud-ai under human review and approval. Human in the loop: GravyaDev.

smonn added a commit to smonn/db that referenced this pull request Oct 23, 2026
## Issue

Closes #154. Upstream: porsager/postgres#1208, #1199 (crash, dead
connection released to the pool), #1226 (stale 57P01, `sql.end()` never
settles); fix PR porsager/postgres#1240. This guard stays until
postgres.js ships them.

## Why

postgres.js throws an uncaught TypeError on a statement sent to a
connection whose backend ended. The driver still sent `rollback` and the
reset after such a failure, so `migrate` crashed the process. After
that, `sql.end()` in the CLI never settled, so Node exited 13 without
printing the error.

## What

- When `fn` rejects with `CONNECTION_CLOSED` or `CONNECTION_DESTROYED`,
the transaction and session send nothing more and rethrow.
`backendAlive` can't judge this: it's false for any error `fn` throws
itself.
- A failed rollback now rejects with an AggregateError of it and `fn`'s
error.
- `Driver.session` notes that SQL `prepare` survives the reset, and that
a pool kept open after such a failure may give a stale 57P01 (#1226).
- The CLI's `run()` and `postgresJsConnect`'s `close` end with `end({
timeout: 0 })`: every statement has settled by then, and a graceful
`end()` hangs (#1226).
- A separate commit moves the client-option tests out of
`driver.test.ts`, which hit 300 lines.

## Testing

New tests end the backend mid-statement in a session, a transaction,
`migrate` through `main()` (asserts exit 1 and CONNECTION_CLOSED on
stderr), and a `connect()` close; each hangs or crashes without its fix.
`pnpm check` passes.
smonn added a commit to smonn/db that referenced this pull request Oct 24, 2026
…error (#359)

## Issue

Closes #221. Stacked on #356.

## Why

The driver judged a reserved connection dead by the error `fn` threw.
After a socket reset (plain ECONNRESET), it sent `rollback` and the
reset to the closed socket, which postgres.js 3.4.9 never answers. When
`fn` rethrew another connection's `CONNECTION_CLOSED`, the driver
skipped rollback and release and leaked a live connection.

## What

- `reservedConnection` records the first statement that fails with
`CONNECTION_CLOSED`, `CONNECTION_DESTROYED`, `ECONNRESET`, `EPIPE`, or
`ETIMEDOUT`. Later statements reject with that error, unsent.
- Session and transaction skip rollback, reset, and release only when
their own connection recorded a failure.
- Still open: the issue comment's third case, a backend that dies while
`fn` sends nothing. That needs a close signal postgres.js doesn't expose
(porsager/postgres#1240). Tracked in #364.

## Testing

A TCP proxy (`tcp-proxy.test-helper.ts`) resets the socket
mid-statement. Session, transaction, later-statement, and caught-error
tests hung before the fix. A one-connection pool covers a self-made
ECONNRESET and a foreign `CONNECTION_CLOSED`. Five mutations of the fix
each fail a test. `pnpm check` passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: pool never recovers after releasing a reserved connection whose backend was terminated.

3 participants