From bb251c07d845eca83d3521355b9900fde3cb2bc4 Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 21 Jun 2026 16:40:46 +0000 Subject: [PATCH 1/5] sql(postgres,mysql): guard ref_and_close against re-entrant TLS close dispatch Closing a TLS socket from ref_and_close synchronously dispatches the on_handshake callback (when the handshake never completed) and then on_close. Both re-enter fail_with_js_value. When the outer entry was disconnect() the status is Disconnected, not Failed, so the status == Failed guard in fail_with_js_value does not trip and a nested ref_and_close runs on the same us_socket_t. The on_handshake dispatch happens before the C layer flips is_closed, so is_closed() alone does not prevent the second close. Add a monotonic CLOSE_INITIATED flag that ref_and_close sets before the first socket.close() and checks on every entry. Detach the stored socket handle in the socket-level on_close so nothing can read through it after the us_socket_t is freed at the end of the loop iteration. Apply the same copy-and-detach pattern to MySQLConnection::close, which had no is_closed() guard at all. Sentry BUN-3KKD / BUN-3GMS (29 events, macOS, 1.3.14): segfault at 0x0 in us_internal_ssl_close with two nested refAndClose frames on the stack. --- src/sql/shared/ConnectionFlags.rs | 5 + src/sql_jsc/mysql/MySQLConnection.rs | 11 +- src/sql_jsc/postgres/PostgresSQLConnection.rs | 17 ++- .../sql-postgres-tls-close-reentry.test.ts | 137 ++++++++++++++++++ 4 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 test/js/sql/sql-postgres-tls-close-reentry.test.ts diff --git a/src/sql/shared/ConnectionFlags.rs b/src/sql/shared/ConnectionFlags.rs index b06ab88756ae..d1c342f35903 100644 --- a/src/sql/shared/ConnectionFlags.rs +++ b/src/sql/shared/ConnectionFlags.rs @@ -8,6 +8,11 @@ bitflags! { const USE_UNNAMED_PREPARED_STATEMENTS = 1 << 2; const WAITING_TO_PREPARE = 1 << 3; const HAS_BACKPRESSURE = 1 << 4; + /// Set once when the connection initiates `socket.close()`; never + /// cleared. Guards `ref_and_close` against re-entering the close path + /// from the on_handshake/on_close callbacks that a TLS close + /// dispatches synchronously. + const CLOSE_INITIATED = 1 << 5; } } diff --git a/src/sql_jsc/mysql/MySQLConnection.rs b/src/sql_jsc/mysql/MySQLConnection.rs index 8bba3068c9fd..e17f14a7fcba 100644 --- a/src/sql_jsc/mysql/MySQLConnection.rs +++ b/src/sql_jsc/mysql/MySQLConnection.rs @@ -272,7 +272,16 @@ impl MySQLConnection { } pub fn close(&mut self) { - self.socket.close(uws::CloseKind::Normal); + // Closing a TLS socket synchronously dispatches on_handshake and + // on_close, both of which re-enter fail_with_js_value → close(). + // Detach the stored handle first and close through a local copy so + // the re-entrant call sees a closed socket, and so nothing reads + // through the stored pointer after the us_socket_t is freed. + let socket = core::mem::replace(&mut self.socket, Socket::SocketTcp(uws::SocketTCP::detached())); + if !socket.is_closed() && !self.flags.contains(ConnectionFlags::CLOSE_INITIATED) { + self.flags.insert(ConnectionFlags::CLOSE_INITIATED); + socket.close(uws::CloseKind::Normal); + } self.write_buffer = OffsetByteList::default(); } diff --git a/src/sql_jsc/postgres/PostgresSQLConnection.rs b/src/sql_jsc/postgres/PostgresSQLConnection.rs index 1d5b7d6344a6..d799f1c2ba78 100644 --- a/src/sql_jsc/postgres/PostgresSQLConnection.rs +++ b/src/sql_jsc/postgres/PostgresSQLConnection.rs @@ -1343,6 +1343,10 @@ impl SocketHandler { _: i32, _: Option<*mut c_void>, ) { + // The us_socket_t is freed at the end of this loop iteration; drop + // the stored handle so nothing can read through it afterwards. + this.socket + .set(Socket::SocketTcp(uws::SocketTCP::detached())); this.on_close(); } @@ -1525,11 +1529,20 @@ impl PostgresSQLConnection { fn ref_and_close(&self, js_reason: Option) { // refAndClose is always called when we wanna to disconnect or when we are closed - if !self.socket.get().is_closed() { + // Closing a TLS socket synchronously dispatches on_handshake (when the + // handshake never completed) and then on_close, both of which re-enter + // fail_with_js_value. When the outer entry was disconnect() the status + // is Disconnected, not Failed, so that guard does not trip and we + // reach a nested ref_and_close. The on_handshake dispatch runs before + // the C layer flips is_closed, so is_closed() alone is not sufficient; + // CLOSE_INITIATED is set once and never cleared. + let socket = *self.socket.get(); + if !socket.is_closed() && !self.flags.get().contains(ConnectionFlags::CLOSE_INITIATED) { + self.update_flags(|f| f.insert(ConnectionFlags::CLOSE_INITIATED)); // event loop need to be alive to close the socket self.poll_ref.with_mut(|r| r.ref_(self.vm_ctx())); // will unref on socket close - self.socket.get().close(uws::CloseKind::Normal); + socket.close(uws::CloseKind::Normal); } // cleanup requests diff --git a/test/js/sql/sql-postgres-tls-close-reentry.test.ts b/test/js/sql/sql-postgres-tls-close-reentry.test.ts new file mode 100644 index 000000000000..ad5fc4d843ae --- /dev/null +++ b/test/js/sql/sql-postgres-tls-close-reentry.test.ts @@ -0,0 +1,137 @@ +// Sentry BUN-3KKD / BUN-3GMS: segfault in us_internal_ssl_close when a +// Postgres TLS connection's refAndClose re-enters via the on_handshake / +// on_close callbacks that a TLS close dispatches synchronously. +// +// Closing a Connected TLS connection goes through disconnect() which sets +// status = Disconnected (not Failed), so fail_with_js_value's status == Failed +// guard does not trip when on_close re-enters, and a nested ref_and_close runs +// on the same us_socket_t. On the reported platform the on_handshake dispatch +// (which runs before the C layer flips is_closed) lets the nested close reach +// us_internal_ssl_close on a socket that is already being torn down. +// +// These tests exercise both re-entry shapes under ASAN without needing a live +// Postgres server: a mock server that speaks just enough of the protocol to +// drive the client through TLS upgrade and (for the disconnect path) into the +// Connected state. + +import { SQL } from "bun"; +import { heapStats } from "bun:jsc"; +import { expect, test } from "bun:test"; +import fs from "node:fs"; +import net from "node:net"; +import path from "node:path"; +import tls from "node:tls"; + +const certDir = path.join(import.meta.dir, "docker-tls"); +const cert = fs.readFileSync(path.join(certDir, "server.crt")); +const key = fs.readFileSync(path.join(certDir, "server.key")); + +// 'R' AuthenticationOk (len=8, type=0) + 'Z' ReadyForQuery (len=5, 'I') +const readyHandshake = Buffer.from([0x52, 0, 0, 0, 8, 0, 0, 0, 0, 0x5a, 0, 0, 0, 5, 0x49]); + +function mockPostgresTLSServer(afterUpgrade: (s: tls.TLSSocket) => void) { + const secureContext = tls.createSecureContext({ cert, key }); + const server = net.createServer(raw => { + raw.once("data", () => { + // Reply 'S' to the 8-byte SSLRequest, then upgrade the raw socket. + raw.write("S", () => { + const s = new tls.TLSSocket(raw, { isServer: true, secureContext }); + s.on("error", () => {}); + afterUpgrade(s); + }); + }); + raw.on("error", () => {}); + }); + return server; +} + +async function listen(server: net.Server) { + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + return (server.address() as net.AddressInfo).port; +} + +async function remainingConnectionsAfterGC(maxWait = 5000): Promise { + Bun.gc(true); + let count = heapStats().objectTypeCounts["PostgresSQLConnection"] || 0; + const deadline = performance.now() + maxWait; + while (count > 2 && performance.now() < deadline) { + await Bun.sleep(20); + Bun.gc(true); + count = heapStats().objectTypeCounts["PostgresSQLConnection"] || 0; + } + return count; +} + +// disconnect() path: the connection reaches Connected over TLS, then close() +// tears it down. on_close re-enters fail_with_js_value with status == +// Disconnected, which proceeds to a nested ref_and_close. +test("Postgres TLS connection close() after Connected survives re-entrant on_close", async () => { + const server = mockPostgresTLSServer(s => { + s.once("data", () => s.write(readyHandshake)); + }); + const port = await listen(server); + + try { + const iterations = 30; + let closes = 0; + for (let i = 0; i < iterations; i++) { + const sql = new SQL({ + url: `postgres://u@127.0.0.1:${port}/db?sslmode=require`, + tls: { rejectUnauthorized: false }, + max: 1, + connectionTimeout: 10, + idleTimeout: 0, + onclose: () => void closes++, + }); + await sql.connect(); + await sql.close({ timeout: 0 }).catch(() => {}); + Bun.gc(true); + } + // Every connection that reached Connected must have fired onclose exactly + // once on the way down; the nested ref_and_close must not have produced a + // second onclose or left the wrapper uncollectable. + expect(closes).toBe(iterations); + expect(await remainingConnectionsAfterGC()).toBeLessThanOrEqual(2); + } finally { + await new Promise(r => server.close(() => r())); + } +}, 60_000); + +// Timer path: the TLS handshake never completes, so connectionTimeout fires +// with status == SentStartupMessage. ref_and_close → socket.close() dispatches +// on_handshake(0, ECONNRESET) and then on_close synchronously; both re-enter +// fail_with_js_value. +test("Postgres TLS connectionTimeout during pending handshake survives re-entrant on_handshake/on_close", async () => { + // Server accepts the SSLRequest and replies 'S', then swallows the TLS + // ClientHello so the handshake never completes. + const server = net.createServer(raw => { + raw.once("data", () => raw.write("S")); + raw.on("data", () => {}); + raw.on("error", () => {}); + }); + const port = await listen(server); + + try { + const iterations = 10; + const seen: string[] = []; + for (let i = 0; i < iterations; i++) { + const sql = new SQL({ + url: `postgres://u@127.0.0.1:${port}/db?sslmode=require`, + tls: { rejectUnauthorized: false }, + max: 1, + connectionTimeout: 1, + idleTimeout: 0, + }); + const err = await sql`select 1`.catch(e => e); + seen.push(err?.code); + await sql.close({ timeout: 0 }).catch(() => {}); + Bun.gc(true); + } + // Every attempt must surface the connection-timeout error (not a crash, + // not a generic ConnectionClosed from the re-entrant path swallowing it). + expect(seen).toEqual(Array(iterations).fill("ERR_POSTGRES_CONNECTION_TIMEOUT")); + expect(await remainingConnectionsAfterGC()).toBeLessThanOrEqual(2); + } finally { + await new Promise(r => server.close(() => r())); + } +}, 60_000); From 35c0f4ea16d57055ade17568551eeec64a9e0ea4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:02:21 +0000 Subject: [PATCH 2/5] test: drop explicit timeouts, reduce iterations to fit default --- test/js/sql/sql-postgres-tls-close-reentry.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/js/sql/sql-postgres-tls-close-reentry.test.ts b/test/js/sql/sql-postgres-tls-close-reentry.test.ts index ad5fc4d843ae..90184a262212 100644 --- a/test/js/sql/sql-postgres-tls-close-reentry.test.ts +++ b/test/js/sql/sql-postgres-tls-close-reentry.test.ts @@ -72,7 +72,7 @@ test("Postgres TLS connection close() after Connected survives re-entrant on_clo const port = await listen(server); try { - const iterations = 30; + const iterations = 15; let closes = 0; for (let i = 0; i < iterations; i++) { const sql = new SQL({ @@ -95,7 +95,7 @@ test("Postgres TLS connection close() after Connected survives re-entrant on_clo } finally { await new Promise(r => server.close(() => r())); } -}, 60_000); +}); // Timer path: the TLS handshake never completes, so connectionTimeout fires // with status == SentStartupMessage. ref_and_close → socket.close() dispatches @@ -112,7 +112,7 @@ test("Postgres TLS connectionTimeout during pending handshake survives re-entran const port = await listen(server); try { - const iterations = 10; + const iterations = 3; const seen: string[] = []; for (let i = 0; i < iterations; i++) { const sql = new SQL({ @@ -134,4 +134,4 @@ test("Postgres TLS connectionTimeout during pending handshake survives re-entran } finally { await new Promise(r => server.close(() => r())); } -}, 60_000); +}); From d6e1751efa19476202299eea68ba606aff4d57b8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:11:04 +0000 Subject: [PATCH 3/5] test: reject on server.listen() error via once(server, 'listening') --- test/js/sql/sql-postgres-tls-close-reentry.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/js/sql/sql-postgres-tls-close-reentry.test.ts b/test/js/sql/sql-postgres-tls-close-reentry.test.ts index 90184a262212..55fcc2168d91 100644 --- a/test/js/sql/sql-postgres-tls-close-reentry.test.ts +++ b/test/js/sql/sql-postgres-tls-close-reentry.test.ts @@ -17,6 +17,7 @@ import { SQL } from "bun"; import { heapStats } from "bun:jsc"; import { expect, test } from "bun:test"; +import { once } from "node:events"; import fs from "node:fs"; import net from "node:net"; import path from "node:path"; @@ -46,7 +47,8 @@ function mockPostgresTLSServer(afterUpgrade: (s: tls.TLSSocket) => void) { } async function listen(server: net.Server) { - await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); return (server.address() as net.AddressInfo).port; } From 0e1943018eddcc19ebc243f640c11a199301e99c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:13:19 +0000 Subject: [PATCH 4/5] [autofix.ci] apply automated fixes --- src/sql_jsc/mysql/MySQLConnection.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sql_jsc/mysql/MySQLConnection.rs b/src/sql_jsc/mysql/MySQLConnection.rs index e17f14a7fcba..e64865f60721 100644 --- a/src/sql_jsc/mysql/MySQLConnection.rs +++ b/src/sql_jsc/mysql/MySQLConnection.rs @@ -277,7 +277,10 @@ impl MySQLConnection { // Detach the stored handle first and close through a local copy so // the re-entrant call sees a closed socket, and so nothing reads // through the stored pointer after the us_socket_t is freed. - let socket = core::mem::replace(&mut self.socket, Socket::SocketTcp(uws::SocketTCP::detached())); + let socket = core::mem::replace( + &mut self.socket, + Socket::SocketTcp(uws::SocketTCP::detached()), + ); if !socket.is_closed() && !self.flags.contains(ConnectionFlags::CLOSE_INITIATED) { self.flags.insert(ConnectionFlags::CLOSE_INITIATED); socket.close(uws::CloseKind::Normal); From c3214230e0bb2e8e4de76e3c17f19adabc328c18 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 21 Jun 2026 17:35:41 +0000 Subject: [PATCH 5/5] ci: retrigger