Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/sql/shared/ConnectionFlags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
11 changes: 10 additions & 1 deletion src/sql_jsc/mysql/MySQLConnection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
17 changes: 15 additions & 2 deletions src/sql_jsc/postgres/PostgresSQLConnection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,10 @@ impl<const SSL: bool> SocketHandler<SSL> {
_: 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();
}

Expand Down Expand Up @@ -1525,11 +1529,20 @@ impl PostgresSQLConnection {
fn ref_and_close(&self, js_reason: Option<JSValue>) {
// 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
Expand Down
137 changes: 137 additions & 0 deletions test/js/sql/sql-postgres-tls-close-reentry.test.ts
Original file line number Diff line number Diff line change
@@ -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<void>(resolve => server.listen(0, "127.0.0.1", resolve));
return (server.address() as net.AddressInfo).port;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

async function remainingConnectionsAfterGC(maxWait = 5000): Promise<number> {
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 = 15;
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<void>(r => server.close(() => r()));
}
});

// 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 = 3;
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<void>(r => server.close(() => r()));
}
});