postgres: fix do_run error-path leaks (poll_ref hang + query/statement refs)

[alii]

Supersedes #32426 and #32273 — combines both, plus the release_statement() at the SYNC-failure site that #32273 missed.

Repro (the user-visible part)

import { SQL } from "bun";
const sql = new SQL({ url: "postgres://...", max: 1, idleTimeout: 0, maxLifetime: 0 });
await sql.connect();
await new Promise(r => setImmediate(r));
await sql`SELECT ${new Boolean(true)}`.catch(e => e);  // ERR_INVALID_ARG_TYPE
// process hangs here instead of exiting

Cause

PostgresSQLQuery::do_run did two pieces of speculative setup before validating its arguments:

1. connection.poll_ref.ref_(). KeepAlive is a two-state flag (src/io/keep_alive.rs), not a refcount, so when this query is the only in-flight work the call flips Inactive → Active. Every synchronous error return after that point left it stuck Active; nothing else on an idle connection touches poll_ref until the next server message, which never comes because nothing was written. The hang is masked when do_run runs inside the connection's on_data microtask drain (whose epilogue re-derives poll_ref from the queue), so it only shows up for queries issued on a later turn — the normal case for any query after the first on a pooled connection.
2. this.ref_(). The simple-query execute_query failure path correctly released it, but three other error exits did not: statements.get_or_put failure, writer.write(SYNC) failure, and the final requests.write_item failure. The latter two also leaked the just-allocated statement ref.

Fix

• Move poll_ref.ref_() to after connection.requests.write_item(this_ptr) succeeds, on both the simple-query and prepared-statement branches. On every error path the keepalive is now untouched; on the success path the request is enqueued so the ref is balanced by on_data/update_ref() when the server responds. Matches JSMySQLQuery::do_run, which never pre-refs.
• Extract the per-exit cleanup into two closures (release_query_ref, throw_write_error) and apply them at all eight error-return sites — the three previously-leaking sites now release what they took, and the five existing sites lose their copy-pasted blocks. Net −74 in PostgresSQLQuery.rs.

Verification

Regression test added to sql-onconnect-onclose-throw.test.ts's describeWithContainer("postgres", ...) block: a fixture connects to real Postgres, waits a tick so do_run runs outside the on_data drain, issues the boxed-Boolean query, prints the rejection and falls through. Without the fix the child hangs and the test times out; with the fix it prints rejected:ERR_INVALID_ARG_TYPE and exits 0.

cargo check -p bun_sql_jsc and cargo clippy --no-deps clean. postgres-multi-statement-fields.test.ts and sql-connect-error-reporting.test.ts (20 tests, both query-protocol paths) still pass.

[robobun]

^{Updated 2:18 PM PT - Jun 17th, 2026}

✅ @alii, your commit f2b76baf21d6c248fe522d398a44a480b3db0cd0 passed in Build #63186! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32464

That installs a local version of the PR into your bun-32464 executable, so you can run:

bun-32464 --bun

[github-actions]

Found 1 issue this PR may fix:

1. "bun test" hangs up in timeout with PostgreSQL and expect->toThrow() #19130 - Reports bun test hanging indefinitely with PostgreSQL when using expect().toThrow() — the query triggers a synchronous error path in do_run that leaves poll_ref stuck Active, matching the exact hang mechanism this PR fixes.

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #19130

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a21c355b-514a-4951-9f20-986bd56e738c

📥 Commits

Reviewing files that changed from the base of the PR and between df97fee and f2b76ba.

📒 Files selected for processing (1)

• test/js/sql/sql-onconnect-onclose-throw.test.ts

---

Walkthrough

PostgresSQLQuery::do_run is refactored to introduce two shared closures — release_query_ref and throw_write_error — that replace duplicated inline cleanup and error-throw logic across all early-return error paths. The poll_ref KeepAlive activation is moved to a unified post-enqueue section. A new test verifies that a synchronous do_run failure does not pin the event loop.

PostgresSQLQuery::do_run cleanup refactor and keepalive fix

Layer / File(s)	Summary
Shared cleanup closures and poll_ref deferral src/sql_jsc/postgres/PostgresSQLQuery.rs	Removes the early poll_ref.ref_ KeepAlive bump and introduces release_query_ref (deref statement + undo speculative query ref) and throw_write_error (throw Postgres JS error only when no exception is pending) as shared closures for all error paths.
Error-return sites updated to use shared helpers src/sql_jsc/postgres/PostgresSQLQuery.rs	All fallible return points — execute_query, request-enqueue OOM, prepared-statement map allocation, bind_and_execute, prepare_and_query_with_signature, write_query, SYNC write, and the final enqueue path — now call release_query_ref() and the standardized throw helpers. poll_ref.ref_ is activated only in the unified post-enqueue success section.
Regression test for event-loop pin test/js/sql/sql-onconnect-onclose-throw.test.ts	Adds a Docker-based Postgres container test that spawns a fixture triggering a synchronous do_run rejection via new Boolean(true), then asserts the process exits with ERR_INVALID_ARG_TYPE and does not hang.

Suggested reviewers

• cirospaciari
• dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main fix: addressing error-path resource leaks in PostgreSQL's do_run function related to poll_ref hang and query/statement reference management.
Description check	✅ Passed	The description comprehensively covers all required sections: what the PR does (detailed fix explanation), how it was verified (test case and existing tests passing), with clear cause analysis, fix details, and verification steps.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/sql/sql-postgres-run-error-pollref-fixture.ts`:
- Line 34: The server initialization in this fixture uses a Promise that only
resolves on success but never rejects on failure, causing potential hangs.
Replace the Promise-based approach where server.listen is passed a resolve
callback with await once(server, "listening") instead, which automatically wires
up both the 'listening' success event and the 'error' event for proper error
handling and rejection.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8f850b33-b098-491b-a958-cca0fbdc3f1e

📥 Commits

Reviewing files that changed from the base of the PR and between 0c537fe and 8fe4fd2.

📒 Files selected for processing (3)

• src/sql_jsc/postgres/PostgresSQLQuery.rs
• test/js/sql/sql-postgres-run-error-pollref-fixture.ts
• test/js/sql/sql-postgres-run-error-pollref.test.ts

[claude]

I didn't find any issues, but this reorders poll_ref activation relative to buffered writes and touches unsafe intrusive-refcount cleanup across eight error exits — worth a human pass to confirm the new ordering and the added release_statement() calls are sound on every branch.

Extended reasoning...

Overview

This PR fixes an event-loop hang and several ref leaks in PostgresSQLQuery::do_run (src/sql_jsc/postgres/PostgresSQLQuery.rs). It moves connection.poll_ref.ref_() from the top of the function to after requests.write_item() succeeds on both the simple-query and prepared-statement branches, and factors the per-exit cleanup (release_statement() + Self::deref(this_ptr) + throw) into two closures applied at all eight error-return sites — three of which previously leaked. Net −74 lines in the Rust file. A new mock-server regression test (sql-postgres-run-error-pollref.test.ts + fixture) reproduces the hang.

Security risks

None apparent. No auth, crypto, permissions, or untrusted-input parsing is touched; the change is confined to error-path resource release and keepalive ordering inside the native query-dispatch path.

Level of scrutiny

Medium-high. While the diff is largely a deduplication refactor, it changes behavior in two non-mechanical ways: (1) poll_ref is now activated after data may already have been buffered into writer (e.g. bind_and_execute / prepare_and_query_with_signature succeed before write_item), relying on the fact that the actual flush happens later in flush_data_and_reset_timeout(); and (2) release_query_ref() now unconditionally calls release_statement() on paths where no statement was ever set (e.g. get_or_put failure) — harmless because release_statement() is a no-op on None, but it also runs on the final write_item OOM path where a statement was stored in connection.statements, so dropping the query's ref there is correct but the map's ref remains (pre-existing). These are unsafe-adjacent intrusive-refcount edits in production-critical DB code, and the KeepAlive flag-not-counter semantics are subtle enough that a second pair of eyes is warranted.

Other factors

The PR description is thorough, cites the matching pattern in JSMySQLQuery::do_run, and includes a targeted regression test plus passing cargo check/clippy and existing protocol tests. The bug-hunting system found nothing. I'm deferring rather than approving solely because refcount/keepalive reordering in native code with multiple unsafe blocks falls outside the "simple and obvious" bar for bot approval.

[alii]

@robobun adopt

[robobun]

Adopted and merged. Verified locally: system bun hung after the synchronous do_run error, debug build with this diff exits 0. Build #63186 green on all 286 jobs.