Closing the database clients never completes when a reservation is taken or released around the shutdown

[Nitjsefnie]

Description

sql.end() never settles when a reservation is taken or released around the
shutdown, against a database that is fully reachable throughout.

Two orderings, one symptom. In both, the reservation itself settles and the
shutdown does not.

• A reservation taken in the same tick as the shutdown. sql.end() yields
  once before it does anything, so a sql.reserve() issued in the same tick
  reaches the pool first and takes a connection the shutdown has already
  counted. The reservation resolves; sql.end() is still pending after 20
  seconds.
• A reservation held across the shutdown and released afterwards.
  sql.end() is registered while the reservation is held, and release() is
  called half a second later. The release returns the connection to the pool
  rather than closing it, so the shutdown is never completed by it. sql.end()
  is still pending after 20 seconds.

Neither is affected by the work on #164: both reproduce identically on main
and on the branch of #222. They are recorded here because a shutdown audit on
that branch enumerated them, and they are the remaining ways this client hands
back a promise it will never settle.

scripts/migrate.ts and scripts/reconcile.ts both await closeSql() in the
finally of their entry points, and src/lib/db/postgres-store.ts reserves a
coordination connection per repository reconciliation, so a shutdown that
overlaps a reservation leaves those processes unable to exit.

Reproduced 2 of 2 in each ordering, on each build.

Expected Behavior

sql.end() settles whatever a reservation does around it. A reservation taken
after the shutdown has begun is refused, as an ordinary query in that position
already is; a reservation released after the shutdown has begun closes its
connection rather than returning it to a pool that is shutting down.

Reproduction Steps

1. Start a reachable database:

   docker run -d --rm --name probe-pg -e POSTGRES_DB=probe -e POSTGRES_USER=probe \
     -e POSTGRES_PASSWORD=probe -p 32802:5432 postgres:17-alpine

2. Save the probe below beside a node_modules that resolves postgres to a
   checkout's install, and run both modes:

   node probe-reserve-holes.mjs same-tick 20000
   node probe-reserve-holes.mjs released 20000

// Two ways a reserve() leaves sql.end() itself permanently pending, against a
// REACHABLE server. The observation windows exist so the probe terminates; they
// are not assertions of a timing margin.
import net from "node:net"; import fs from "node:fs"; import path from "node:path";
import { fileURLToPath } from "node:url"; import postgres from "postgres";

const here = path.dirname(fileURLToPath(import.meta.url));
const PG_PORT = Number(process.env.PG_PORT || 32802);
const MODE = process.argv[2] ?? "same-tick";
const WINDOW_MS = Number(process.argv[3] ?? 20000);
const t0 = Date.now(); const log = (...a) => console.log(`+${String(Date.now()-t0).padStart(6)}ms`, ...a);

// A plain forwarder, so the server is genuinely reachable throughout.
const srv = net.createServer((c) => {
  c.on("error", () => {});
  const u = net.connect(PG_PORT, "127.0.0.1");
  u.on("error", () => {});
  c.on("data", d => u.write(d)); u.on("data", d => c.write(d));
  c.on("close", () => u.destroy()); u.on("close", () => c.end());
});
await new Promise(r => srv.listen(0, "127.0.0.1", r));
const port = srv.address().port;
const sql = postgres({ host: "127.0.0.1", port, database: "probe", user: "probe", password: "probe", max: 2 });

await sql`select 1 as ok`;                       // the pool is known live before anything is shut down
log("warm-up query ok");

let endSettled = null, reserveSettled = null;
if (MODE === "same-tick") {
  sql.end().then(() => { endSettled = Date.now(); log("sql.end(): RESOLVED"); },
                 e => { endSettled = Date.now(); log("sql.end(): REJECTED " + e.message); });
  sql.reserve().then(() => { reserveSettled = Date.now(); log("reserve(): RESOLVED"); },
                    e => { reserveSettled = Date.now(); log("reserve(): REJECTED " + e.message); });
  log("end() and reserve() issued in the same tick");
} else {
  const r = await sql.reserve(); log("reserve() resolved and held");
  sql.end().then(() => { endSettled = Date.now(); log("sql.end(): RESOLVED"); },
                 e => { endSettled = Date.now(); log("sql.end(): REJECTED " + e.message); });
  await new Promise(x => setTimeout(x, 500));
  r.release(); log("release() called");
  reserveSettled = Date.now();
}

const deadline = Date.now() + WINDOW_MS;
while (Date.now() < deadline && !endSettled) await new Promise(x => setTimeout(x, 200));
console.log(`RESULT mode=${MODE} end=${endSettled ? "settled" : `PENDING>${WINDOW_MS}ms`} `
  + `reserve=${reserveSettled ? "settled" : `PENDING>${WINDOW_MS}ms`}`);
process.exit(0);

Observed on the branch of #222 (9682e6a):

+    80ms warm-up query ok
+    80ms end() and reserve() issued in the same tick
+    81ms reserve(): RESOLVED
RESULT mode=same-tick end=PENDING>20000ms reserve=settled

+    45ms warm-up query ok
+    46ms reserve() resolved and held
+   548ms release() called
RESULT mode=released end=PENDING>20000ms reserve=settled

3. Repeat on main at 0b306d8, re-running pnpm install --frozen-lockfile
   first so the pre-fix(patch): settle and space a connect-phase socket death #222 patch is installed. Identical:

RESULT build=PRE mode=same-tick end=PENDING>20000ms reserve=settled
RESULT build=PRE mode=released end=PENDING>20000ms reserve=settled

Environment / Context

• Node 24.17.0, Linux, postgres:17-alpine for the server.
• postgres 3.4.9 with patches/postgres@3.4.9.patch applied, as installed by
  pnpm install --frozen-lockfile.
• The client is built with library defaults except max: 2.

Discovered During

A shutdown-uniformity audit on the branch of #222, which fixes #164. Enumerated
by an adversary agent and reproduced independently by the session lead, against
both builds, with the runs quoted above.

Suggested Fix

Unverified, and the two orderings need different repairs even though they share
a cause: nothing on the reservation path consults the pool's shutdown state.

For the same-tick ordering, reserve() pushes into the pool's queries
without checking ending, where the ordinary query path in handler() already
refuses. Note that #222 adds that check for the post-shutdown case; it does
not help here, because a same-tick reservation wins the await 1 at the top of
end() and so arrives before ending is assigned.

For the release ordering, the patched release() in the package's
src/index.js clears the reservation and calls onopen(c), which returns the
connection to the pool. Every other route that finishes with a connection goes
through ending ? terminate() : onopen(connection); this one does not.