sql: close the socket at once when postgres or mysql fail a TLS connection whose peer stopped responding

Closed
RO
2mo ago
+232-9

Problem

  • When the postgres or mysql client fails a connection itself (connection, idle or lifetime timeout, protocol violation, forced close() while still connecting) over TLS against a peer that has stopped responding, the failure is reported but the socket stays open for good: on postgres it holds a poll ref, so the process never exits; on mysql it holds the ref the socket has on the native connection, so the connection (and the fd) is never released. Noted by @alii while reviewing #37993, where the valkey client had the same bug.
  • Cause: the close issued from fail() used CloseCode::Normal. PostgresSQLConnection::fail_with_js_value -> ref_and_close (src/sql_jsc/postgres/PostgresSQLConnection.rs) and JSMySQLConnection::fail_with_js_value -> MySQLConnection::clean_queue_and_close -> close (src/sql_jsc/mysql/MySQLConnection.rs). For a TLS socket past its handshake, Normal sends close_notify and defers the fd close, and with it the on_close dispatch, until the peer answers (us_internal_ssl_close, packages/bun-usockets/src/crypto/openssl.c). A peer that stopped responding never answers, so the poll ref ref_and_close takes (postgres) and the ref the socket holds on the connection (mysql) are never released. Over plain TCP the same close dispatches on_close synchronously, which is why only TLS is affected.

Fix

  • ref_and_close and clean_queue_and_close take the close code. fail_with_js_value passes CloseCode::Failure in both drivers; the user-facing close() (disconnect() on postgres, do_close on mysql) keeps Normal.
  • Failure is the one code usockets never defers (Normal waits for the peer's close_notify; FastShutdown is still held back while undelivered ciphertext is pending, i.e. exactly when the peer stopped reading), so on_close now runs inside the close call, the way it already does over TCP. Every JS-visible effect of the failure (onclose, rejected queries) happened before the close either way; the only visible difference is that a failed connection now ends with an RST instead of a FIN on TCP, which does not matter once everything on the connection has been rejected. Same shape and reasoning as the valkey change in #37993, which also updates the CloseCode docs this relies on.
  • Verified with test/js/sql/sql-close-pending-connection.test.ts: the new test spawns test/js/sql/sql-tls-peer-stopped-responding.fixture.ts, which runs postgres and mysql, each against a mock that completes the TLS handshake, takes the startup message and then stops responding, with two triggers per driver (pool force-closed while the connection waits for the startup reply; a protocol violation as the startup reply). Each client has to report the failure, each mock's TCP socket has to see the connection close, and the fixture has to exit by itself. Without the fix all four clients report the failure, none of the four mocks ever sees its connection close, and the fixture has to be killed by the spawn timeout.
  • Also run: test/js/sql/wire-frames.test.ts, postgres-tls-ctx-leak, sql-connect-error-reporting, sql-onconnect-onclose-throw, sql-mysql-clean-reentry, sql-mysql-tls-plaintext-injection, sql-mariadb-json, postgres-failed-connection-resurrection, postgres-listen-notify and the other fault-injection files in test/js/sql (all green; the mysql files that need a real server fail identically on main here because the local MariaDB rejects root, unrelated).
  • A connection timeout is deliberately not one of the test's triggers: in a debug build four concurrent handshakes take about two seconds, so a short timeout fires before the handshake is done, where there is nothing to defer and the test would pass without the fix. Both triggers used happen strictly after the mock has the startup message and reach the same fail() a timeout reaches.
  • Overlaps with #32573 and #32861, which touch ref_and_close / on_close for different reasons (re-entrancy guard, detaching the stored socket); whichever lands second needs a trivial rebase. The user-facing close() of an established TLS connection whose peer stopped responding still waits for the peer (the pool's close() promise never resolves); that is a separate follow-up, as is the postgres poll ref leaking when a connection timeout fires while the TCP connect itself is still pending (no TLS involved).

Background

  • usockets close codes (CloseCode in src/uws_sys/us_socket_t.rs): Normal (0) is a graceful close: TCP FIN; on TLS, send close_notify and keep the socket until the peer's close_notify or FIN arrives, then close and dispatch on_close. Failure (1) closes immediately: on TLS a fast shutdown without waiting, on TCP SO_LINGER{1,0} so the close is an RST. FastShutdown (2) closes without waiting for the peer but is still deferred while the socket owns ciphertext the kernel has not accepted yet.
  • Poll ref: the KeepAlive a native object holds to keep the event loop (and so the process) alive while it expects further socket events. ref_and_close takes one before closing and relies on on_close to release it.
  • Mock that does not answer close_notify: Bun's own TLS sockets answer a peer's close_notify natively, and pause() does not stop that, so the fixture's mocks (startTlsServerSide in test/js/sql/wire-frames.ts) run the TLS engine over a Duplex they feed by hand and simply stop feeding it once they have the startup message; the TCP connection stays open, nothing is ever answered, which is what a hung server looks like to the client.