Skip to content
Closed
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
7 changes: 4 additions & 3 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1325,9 +1325,10 @@ impl WebWorker {
}

// The finalizers JSC just ran close the sockets that `close_all_socket_groups` leaves
// alone (a Listener owns its listen socket and closes it in `finalize`). `us_socket_close`
// only queues onto `loop->data.closed_head`; step 5's `on_thread_exit()` frees the loop
// out from under whatever is still queued, so drain it now, while the loop is alive.
// alone (a `Listener` / `NewServer` owns its listen socket and closes it in `finalize`).
// `us_socket_close` only queues onto `loop->data.closed_head`; step 5's `on_thread_exit()`
// frees the loop out from under whatever is still queued, so drain it now, while the loop
// is alive.
Comment thread
robobun marked this conversation as resolved.
if !vm_ptr.is_null() {
// SAFETY: `vm_ptr` was unpublished under `vm_lock`; sole owner, `destroy()` is below.
unsafe { (*vm_ptr).uws_loop_mut().drain_closed_sockets() };
Expand Down
5 changes: 4 additions & 1 deletion src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1771,7 +1771,10 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
}

// Only free the memory if the JS reference has been freed too.
if matches!(self.js_value, jsc::JsRef::Finalized) {
// Same `is_shutting_down` gate as the promise block above:
// `schedule_deinit` enqueues tasks no tick will ever drain once
// this is reached from `lastChanceToFinalize()`.
Comment thread
robobun marked this conversation as resolved.
if matches!(self.js_value, jsc::JsRef::Finalized) && !self.vm().is_shutting_down() {
self.schedule_deinit();
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/runtime/server/server_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2751,6 +2751,27 @@ where
// hold a ref), so hand ownership back to the raw teardown path.
let this = bun_core::heap::release(self);
this.js_value.finalize();
// `listener` set here ⇒ VM teardown (`lastChanceToFinalize`): the
// Strong `js_value` root kept the wrapper alive through every ordinary
// GC. `close_all_socket_groups` skipped listen sockets for us to close
// (matching `Listener::finalize`); not doing so leaks the fd/port past
// worker exit.
Comment thread
robobun marked this conversation as resolved.
if let Some(listener) = this.listener.take() {
if let server_config::Address::Unix(path) = &this.config.address {
let bytes = path.as_bytes();
if !bytes.is_empty() && bytes[0] != 0 {
let _ = bun_sys::unlink(path.as_zstr());
}
}
bun_opaque::opaque_deref_mut(listener).close();
}
Comment thread
robobun marked this conversation as resolved.
// Not `h3_listener.close()`: `us_quic_listen_socket_close` runs
// `lsquic_conn_abort` + `us_quic_process` → JS `on_abort` for any live
// QUIC conn (which `close_all_socket_groups` never drained), and JS is
// unsound inside `lastChanceToFinalize`.
Comment thread
robobun marked this conversation as resolved.
if Self::HAS_H3 {
this.h3_listener = None;
}
Comment thread
robobun marked this conversation as resolved.
this.deinit_if_we_can();
Comment thread
robobun marked this conversation as resolved.
}

Expand Down
81 changes: 80 additions & 1 deletion test/js/web/workers/worker-terminate-lifetime.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isASAN, isDebug } from "harness";
import { bunEnv, bunExe, isASAN, isDebug, isLinux } from "harness";
import { join } from "path";

// Worker VM startup/teardown is much slower under debug and/or ASAN; these
Expand Down Expand Up @@ -176,3 +176,82 @@ test.skipIf(!isASAN)(
},
timeout,
);

// Regression: NewServer::finalize() never closed its listen socket. Worker
// shutdown's close_all_socket_groups() deliberately skips listen sockets on
// the assumption the owner closes them in finalize (Bun.listen's Listener
// does), so a Bun.serve() server in a terminated worker leaked its listen fd
// and the port stayed bound for the life of the process. All three exit modes
// below converge on WebWorker::shutdown() -> lastChanceToFinalize().
test.each([
["parent terminate()", "", true],
["worker process.exit()", " setImmediate(() => process.exit(0));", false],
["worker unref() + drain", " s.unref();", false],
])(
"Bun.serve() in a worker releases the listening port on %s",
async (_name, tail, parentTerminates) => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const serveWorker =
"const s = Bun.serve({ port: 0, fetch: () => new Response('ok') });" +
"postMessage(s.port);" + ${JSON.stringify(tail)};

async function cycle() {
const w = new Worker("data:text/javascript," + encodeURIComponent(serveWorker));
const port = await new Promise(r => w.addEventListener("message", e => r(e.data), { once: true }));
const closed = new Promise(r => w.addEventListener("close", r, { once: true }));
${parentTerminates ? "w.terminate();" : ""}
await closed;
Comment thread
robobun marked this conversation as resolved.
// Worker has fully exited; its listen socket must be gone. Rebind
// fails with EADDRINUSE if the old listener is still alive.
const s = Bun.serve({ port, reusePort: false, fetch: () => new Response("x") });
s.stop(true);
return port;
}

// One warm-up cycle so lazily-created per-process fds (DNS, event loop
// timers, module cache) do not count against the leak delta below.
await cycle();

const fdCount = ${isLinux}
? () => require("fs").readdirSync("/proc/self/fd").length
: () => 0;
const before = fdCount();

const ports = [];
for (let i = 0; i < 5; i++) ports.push(await cycle());

// The 'close' event fires from WebWorker__dispatchExit (shutdown step
// 4); the detached worker thread then still runs step 5 (vm.destroy +
// on_thread_exit) which closes the per-worker epoll/eventfd. Poll the
// fd count with a bounded deadline so the last cycle's worker has
// reached that point instead of relying on gc() as an implicit sleep.
let fdDelta = fdCount() - before;
for (let i = 0; fdDelta > 1 && i < ${slow ? 500 : 100}; i++) {
await Bun.sleep(10);
fdDelta = fdCount() - before;
Comment thread
robobun marked this conversation as resolved.
}
console.log(JSON.stringify({ ports: ports.length, fdDelta }));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
const { ports, fdDelta } = JSON.parse(stdout.trim());
// Every cycle rebound its port: EADDRINUSE would have thrown before here.
expect(ports).toBe(5);
if (isLinux) {
// Unfixed: one listen fd per cycle (>= 5).
expect(fdDelta).toBeLessThanOrEqual(1);
}
Comment thread
robobun marked this conversation as resolved.
expect(exitCode).toBe(0);
},
timeout,
);
Loading