sql(postgres): stop dispatching messages once the connection has failed

[robobun]

A backend message that fails the connection can share a TCP read with messages that follow it. PostgresRequest::on_data's message loop had no bail-out once fail() had run, so the trailing messages in that read kept being dispatched against the already-failed connection.

Repro

A mock backend that answers the StartupMessage with one write carrying two messages:

R  int32(8) int32(99)   Authentication, unrecognized type
Z  int32(5) 'I'         ReadyForQuery

const sql = new SQL({ url: `postgres://u@127.0.0.1:${port}/db`, max: 1, idleTimeout: 1, connectionTimeout: 5 });
await sql`select 1`.catch(() => {});
await Bun.sleep(1600);

Cause

The unrecognized Authentication type calls fail(), which sets the status to Failed, closes the socket, and rejects the pending requests, but the message loop keeps going and dispatches the ReadyForQuery from the same read. That calls set_status(Status::Connected), which has no guard against leaving Failed, so the dead connection is flipped back to Connected and the on_data epilogue re-arms its idle timer. uSockets frees a closed us_socket_t at the end of the event-loop iteration, so when the timer later fires, ref_and_close reads the freed socket:

ERROR: AddressSanitizer: heap-use-after-free
READ of size 1 at 0x71f2125605d2 thread T0
    #0 us_socket_is_closed                              packages/bun-usockets/src/socket.c:143:21
    #4 PostgresSQLConnection::ref_and_close             src/sql_jsc/postgres/PostgresSQLConnection.rs:1528:31
    #5 PostgresSQLConnection::fail_with_js_value        src/sql_jsc/postgres/PostgresSQLConnection.rs:726:14
    #6 PostgresSQLConnection::fail_fmt                  src/sql_jsc/postgres/PostgresSQLConnection.rs:749:14
    #7 PostgresSQLConnection::on_connection_timeout     src/sql_jsc/postgres/PostgresSQLConnection.rs:557:14
    #8 __bun_fire_timer                                 src/runtime/dispatch.rs:1020:35
0x71f2125605d2 is located 18 bytes inside of 104-byte region
freed by thread T0 here:
    #2 us_internal_free_closed_sockets                  packages/bun-usockets/src/loop.c:305:9

Fix

• PostgresRequest::on_data: the message loop returns once the connection's status is Failed. fail() is terminal; nothing after it in the same read should be handled (a DataRow, CommandComplete, or ErrorResponse in that position would be just as wrong as the ReadyForQuery).
• PostgresSQLConnection::set_status: refuses to transition out of Failed. The transition function owns that invariant; every other consumer of Status (the timer interval, update_has_pending_activity, the idempotency check in fail_with_js_value) already assumes Failed is terminal.

Verification

test/js/sql/postgres-failed-connection-resurrection.test.ts runs a fixture against the mock backend above and lets it outlive the idle-timer window. Without the fix the fixture dies with the ASan report above; with it the fixture exits 0. Gated to ASan builds because the bug is a read of freed memory, which release lanes do not detect.

The postgres fault-injection and integration suites still pass locally (90 tests across test/js/sql/postgres-*.test.ts, sql*.test.ts, tls-sql.test.ts).

Related

• sql: detach the stored socket in on_close/on_connect_error #32861 detaches the stored socket handle in on_close / on_connect_error so nothing can dereference the freed us_socket_t regardless of how the stale read is reached. It removes the last step of this chain from the other end; this PR stops the failed connection from being resurrected at all.
• sql: ignore stale on_connect firing after #onClose in pooled Postgres/MySQL #30950 guards the JS pool's handleConnected against the reverse ordering within one read (a legitimately queued onconnect microtask arriving after a synchronous onclose).

[robobun]

^{Updated 2:50 PM PT - Jun 28th, 2026}

✅ @robobun, your commit dfd2a3cfcc0183da10167ff45565e16d27f69a2b passed in Build #66472! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 33016

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

bun-33016 --bun

[coderabbitai]

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: df2c8a6b-dd12-4e52-976f-8b07ee3235be

📥 Commits

Reviewing files that changed from the base of the PR and between a1c39de and dfd2a3c.

📒 Files selected for processing (4)

• src/sql_jsc/postgres/PostgresRequest.rs
• src/sql_jsc/postgres/PostgresSQLConnection.rs
• test/js/sql/postgres-failed-connection-resurrection.fixture.ts
• test/js/sql/postgres-failed-connection-resurrection.test.ts

---

Walkthrough

Adds a terminal-state guard to Status::Failed in set_status and an early-exit check in on_data's dispatch loop so no further backend messages are processed after a connection fails. A new ASan regression test with a mock Postgres backend verifies the fix.

Changes

Postgres Failed-Connection Resurrection Fix

Layer / File(s)	Summary
Terminal Failed state enforcement src/sql_jsc/postgres/PostgresSQLConnection.rs, src/sql_jsc/postgres/PostgresRequest.rs	set_status now caches the current status and returns early if it is Status::Failed, blocking any transition out of the terminal state. on_data's message-dispatch loop imports Status and inserts an early Ok(()) return when the connection is already Failed, preventing further handler execution in the same read batch.
ASan regression test and fixture test/js/sql/postgres-failed-connection-resurrection.fixture.ts, test/js/sql/postgres-failed-connection-resurrection.test.ts	A new mock-server fixture sends an unrecognized authentication type (99) immediately followed by ReadyForQuery in a single write, then waits past the idle-timer window and logs the error code and SURVIVED. The test runner spawns the fixture under ASan and asserts ERR_POSTGRES_UNKNOWN_AUTHENTICATION_METHOD then SURVIVED on stdout with exit code 0.

Possibly related PRs

• oven-sh/bun#32027: Touches the same Postgres connection-failure state machine, changing early handshake failures to emit ERR_POSTGRES_CONNECTION_FAILED instead of generic closed-connection errors.

Suggested reviewers

• cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main fix: stopping Postgres message dispatch after a connection failure.
Description check	✅ Passed	The description explains the bug, fix, and verification, even though it doesn't use the template's exact headings.
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.

---

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

[claude]

LGTM — two narrow guards making Failed terminal, with an ASan-gated regression test.

Extended reasoning...

Overview

This PR adds two small defensive guards in the Postgres connection state machine to fix a heap-use-after-free:

• PostgresRequest::on_data (src/sql_jsc/postgres/PostgresRequest.rs): the message-dispatch loop now bails out as soon as connection.status == Failed, so trailing backend messages in the same TCP read are not dispatched against a connection that fail() has already torn down.
• PostgresSQLConnection::set_status (src/sql_jsc/postgres/PostgresSQLConnection.rs): refuses to transition out of Failed, enforcing the invariant every other consumer (timers, update_has_pending_activity, fail_with_js_value's idempotency check) already assumes.

A new ASan-gated test (postgres-failed-connection-resurrection.test.ts + fixture) reproduces the original crash with a mock backend that sends an unrecognized Authentication message followed by ReadyForQuery in a single write.

Security risks

None introduced. This is a memory-safety fix — it eliminates a use-after-free where a freed us_socket_t was dereferenced from a stale idle-timer callback. No auth, crypto, injection, or data-exposure surface is touched.

Level of scrutiny

Low-to-moderate. The production diff is ~10 lines: two early-returns that fire only when the connection is already in the terminal Failed state. fail_with_js_value sets Failed via a direct self.status.set(...) (not set_status), so the new guard cannot interfere with entering the failed state — it only prevents leaving it. There is no legitimate path that un-fails a connection, so the behavior change is strictly a narrowing of an already-invalid state transition.

Other factors

• No bugs surfaced by the bug-hunting system.
• No CODEOWNERS for these paths and no outstanding reviewer comments.
• The PR description is unusually thorough: full ASan trace, minimal repro, root-cause analysis, and cross-references to the related #32861 / #30950 fixes that close the same class of bug from the other end.
• The test reuses the existing wire-frames.ts helpers and follows the established fault-injection pattern in test/js/sql/, gated to ASan because the failure mode is undetectable in release builds.

Given the minimal, conservative nature of the change and the clear regression coverage, I'm comfortable approving without further human review.