Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 5 additions & 5 deletions src/runtime/socket/Handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ impl Handlers {
}

/// The accepting listener, or `None` for client-mode handlers and for a
/// listener already torn down by `Listener::deinit`.
/// listener already torn down by `Listener::finalize`.
pub fn listener(&self) -> Option<&SocketListener> {
// SAFETY: `Listener::listen` stores its `heap::into_raw` allocation root
// here, and `Listener::deinit` clears it before freeing that allocation
// (after force-closing every accepted socket), so a `Some` is live. The
// borrow cannot outlive `&self`, and nothing frees a `Listener` while a
// caller holds one — `deinit` runs from GC finalize, not from dispatch.
// here; `Listener::finalize` clears it inside the GC sweep before
// deferring `deinit` to the event loop, and `deinit` clears it again
// before `heap::take`, so a `Some` is always a live allocation and no
// caller can hold this borrow across the free.
Comment thread
robobun marked this conversation as resolved.
self.listener.get().map(|l| unsafe { &*l.as_ptr() })
}

Expand Down
36 changes: 35 additions & 1 deletion src/runtime/socket/Listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,41 @@ impl Listener {
}
// `deinit` frees the allocation itself (`heap::take`); hand ownership
// back so its existing raw-ptr teardown path stays intact.
Self::deinit(Box::into_raw(self));
let this = Box::into_raw(self);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// SAFETY: just leaked; exclusive until `deinit` reclaims it.
let this_ref = unsafe { &*this };

// Retire the swept wrapper's `JsRef` and the back-pointer now so
// nothing between here and the deferred `deinit` can observe the dead
// `JSListener` via `socket.listener`. Both writes are plain Rust state
// (no JSC heap access); `deinit` repeats them idempotently.
Comment thread
robobun marked this conversation as resolved.
this_ref.this_value.with_mut(|r| r.finalize());
this_ref.handlers.set_listener(None);

// With live connections `deinit` force-closes the socket group, which
// synchronously dispatches each accepted socket's `on_close` (and for
// TLS, `on_handshake`) and reads the JS handler from the
// `JSSocketHandlers` cell. Reading a cell from inside the GC sweep
// trips JSC's `validateIsNotSweeping` assertion, so defer the whole
// teardown to the event loop. The task owns `this` until it runs; the
// accepted sockets' own JS wrappers keep the handlers cell alive.
if this_ref.handlers.active_connections.get() > 0 {
let vm = this_ref.handlers.vm;
if vm.is_shutting_down() {
// The event loop won't tick again, and running `close_all`
// inline would still read the cell mid-sweep. Leak the
// Listener; fds are reclaimed either by process exit or by the
// worker's `close_all_socket_groups` via the loop's group list.
Comment thread
robobun marked this conversation as resolved.
return;
}
Comment thread
robobun marked this conversation as resolved.
vm.event_loop_mut()
.enqueue_task(jsc::ManagedTask::ManagedTask::new(this, |this| {
Self::deinit(this);
Ok(())
}));
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
return;
}
Comment thread
robobun marked this conversation as resolved.
Self::deinit(this);
}

/// Match Node.js/libuv: unlink the unix socket file before closing the listening fd.
Expand Down
102 changes: 102 additions & 0 deletions test/js/bun/net/socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3330,3 +3330,105 @@ describe("TLS handshake callback throw", () => {
}
});
});

it("GC of an unref'd Bun.listen() with live connections does not run the close handler inside the heap sweep", async () => {
// The Listener finalizer used to force-close its socket group from inside
// the GC sweep, which synchronously dispatched each accepted socket's
// on_close and read the JS close handler from a JSC cell mid-sweep,
// tripping JSC's validateIsNotSweeping assertion (SIGABRT in release). The
// finalizer now defers the group close to the event loop, so the accepted
// sockets' close handlers fire normally on the next tick.
const src = `
let serverClose = 0, serverOpen = 0, clientClose = 0;
const accepted = Promise.withResolvers();
function makeUnreffedListener() {
const srv = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: {
data() {}, error() {},
open() { if (++serverOpen === 3) accepted.resolve(); },
close() { serverClose++; },
}});
const port = srv.port;
srv.unref();
return port;
}
const conns = [];
for (let i = 0; i < 3; i++) {
const port = makeUnreffedListener();
conns.push(await Bun.connect({ hostname: "127.0.0.1", port,
socket: { data() {}, open(s) { s.write("x"); }, error() {},
close() { clientClose++; } } }));
}
Comment thread
robobun marked this conversation as resolved.
await accepted.promise;
// Drive GC until the finalized listeners have force-closed their accepted
// sockets (observed via the server close handler and the client close),
// bounded so an unfixed build that SIGABRTs on the first sweep fails the
// stdout/exitCode assertions instead of hanging.
for (let i = 0; (serverClose < 3 || clientClose < 3) && i < 200; i++) { Bun.gc(true); await Bun.sleep(5); }
Comment thread
coderabbitai[bot] marked this conversation as resolved.
console.log("serverClose=" + serverClose + " clientClose=" + clientClose);
for (const c of conns) c.end();
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stderr: "pipe",
stdout: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// The deferred close fires the server's close handler from the event loop
// (the accepted sockets are not themselves being finalized), and the client
// sees a normal close.
expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({
stdout: "serverClose=3 clientClose=3",
stderr: "",
exitCode: 0,
});
});

it("GC of an unref'd TLS Bun.listen() with live connections does not run the handshake handler inside the heap sweep", async () => {
// TLS sibling of the plain-TCP test above. us_internal_ssl_close fires a
// synthetic ECONNRESET on_handshake for a socket whose handshake has not
// yet completed, on its way to raw-close, so without deferral the
// Listener finalizer reaches on_handshake from inside the GC sweep too.
const src = `
const { key, cert } = JSON.parse(process.env.TLS_JSON);
let serverClose = 0, serverOpen = 0, clientClose = 0;
const accepted = Promise.withResolvers();
const socket = {
data() {}, error() {}, handshake() {},
open() { if (++serverOpen === 3) accepted.resolve(); },
close() { serverClose++; },
};
const conns = [];
async function makeDroppedTlsListenerWithLiveConn() {
const srv = Bun.listen({ hostname: "127.0.0.1", port: 0, tls: { key, cert }, socket });
const port = srv.port;
srv.unref();
conns.push(await Bun.connect({ hostname: "127.0.0.1", port,
tls: { rejectUnauthorized: false },
socket: { data() {}, open() {}, error() {}, handshake() {},
close() { clientClose++; } } }));
void srv.port;
}
for (let i = 0; i < 3; i++) await makeDroppedTlsListenerWithLiveConn();
await accepted.promise;
// Drive GC until the finalized listeners have force-closed their accepted
// sockets (observed via the server close handler and the client close),
// bounded so an unfixed build SIGABRTs on an early sweep and fails the
// assertions instead of hanging.
for (let i = 0; (serverClose < 3 || clientClose < 3) && i < 400; i++) { Bun.gc(true); await Bun.sleep(5); }
console.log("serverClose=" + serverClose + " clientClose=" + clientClose);
for (const c of conns) c.end();
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: { ...bunEnv, TLS_JSON: JSON.stringify(tls) },
stderr: "pipe",
stdout: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({
stdout: "serverClose=3 clientClose=3",
stderr: "",
exitCode: 0,
});
});
Loading