From 09449e8d5b4b4c563d38ad745344f3c61bc650d1 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 21 Aug 2026 16:18:27 -0700 Subject: [PATCH 1/2] usockets (Windows): make each uv_run of the JS loop one event-loop scope libuv dispatches its callbacks from inside uv_run, and every handler they reach enters/exits the event loop on its own, so on Windows the outermost exit - the microtask checkpoint - ran on libuv's dispatch frame. A promise reaction queued by an I/O callback that then drives the loop again (waitForPromise: bun:test's expect(promise).resolves, for one) nested a second uv_run inside the callback, under a dispatch that still reads the socket/handle it dispatched on once the callback returns. That is the use-after-free test/bake/deinitialization.test.ts keeps hitting on Windows. us_loop_run and us_loop_pump now bracket uv_run with EventLoop::enter() / exit() when the loop belongs to a jsc::EventLoop whose VM is alive. Each callback's own enter/exit is then a nested pair, no checkpoint runs inside uv_run, and the callbacks' nextTicks and promise reactions run once uv_run has returned - so a continuation that drives the loop does it sequentially rather than nested. This covers microtask-driven re-entry only; a callback whose own body drives the loop still nests. --- packages/bun-usockets/src/eventing/libuv.c | 27 ++++++++++++++++++++-- src/jsc/event_loop.rs | 12 ++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/bun-usockets/src/eventing/libuv.c b/packages/bun-usockets/src/eventing/libuv.c index ea5e72904745..6c6f88183b3f 100644 --- a/packages/bun-usockets/src/eventing/libuv.c +++ b/packages/bun-usockets/src/eventing/libuv.c @@ -326,6 +326,29 @@ void us_internal_poll_set_type(struct us_poll_t *p, int poll_type) { LIBUS_SOCKET_DESCRIPTOR us_poll_fd(struct us_poll_t *p) { return p->fd; } +extern void Bun__JSEventLoop__enter(void *event_loop); +extern void Bun__JSEventLoop__exit(void *event_loop); + +/* The jsc::EventLoop this loop belongs to (parent_tag 1; 2 is a MiniEventLoop, + * which runs no JS), while its VM is alive (jsc_vm is cleared at teardown). */ +static void *us_internal_js_event_loop(struct us_loop_t *loop) { + return loop->data.parent_tag == 1 && loop->data.jsc_vm ? loop->data.parent_ptr : NULL; +} + +/* Every uv_run of a JS thread's loop is one event-loop scope (EventLoop::enter + * / exit). libuv dispatches its callbacks from inside uv_run, so each JS + * callback they run is a nested scope whose exit is not the outermost one and + * therefore not a microtask checkpoint: the nextTicks and promise reactions a + * callback queues run here, once uv_run has returned, instead of on libuv's + * dispatch frame - where a continuation that drives the loop again + * (waitForPromise) would nest uv_run inside the callback. */ +static void us_internal_uv_run(struct us_loop_t *loop, uv_run_mode mode) { + void *js_event_loop = us_internal_js_event_loop(loop); + if (js_event_loop) Bun__JSEventLoop__enter(js_event_loop); + uv_run(loop->uv_loop, mode); + if (js_event_loop) Bun__JSEventLoop__exit(js_event_loop); +} + void us_loop_pump(struct us_loop_t *loop) { /* POSIX parity: us_loop_run_bun_tick polls epoll/kqueue and dispatches * regardless of ref state (it only early-outs on num_polls == 0). libuv's @@ -335,7 +358,7 @@ void us_loop_pump(struct us_loop_t *loop) { * bun:test) supply their own keep-going predicate, so force exactly one * non-blocking iteration; UV_RUN_NOWAIT keeps the poll timeout at 0. */ loop->uv_loop->active_handles++; - uv_run(loop->uv_loop, UV_RUN_NOWAIT); + us_internal_uv_run(loop, UV_RUN_NOWAIT); loop->uv_loop->active_handles--; } @@ -414,7 +437,7 @@ void us_loop_run(struct us_loop_t *loop) { Bun__JSC_onBeforeWait(loop->data.jsc_vm, (uint64_t) uv_now(loop->uv_loop) * 1000000ULL); } - uv_run(loop->uv_loop, UV_RUN_ONCE); + us_internal_uv_run(loop, UV_RUN_ONCE); } struct us_poll_t *us_create_poll(struct us_loop_t *loop, int fallthrough, diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 31020cc33854..4d00b26f4527 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1414,6 +1414,18 @@ pub fn event_loop_exit(global: &JSGlobalObject) { global.bun_vm().event_loop_mut().exit(); } +/// `this` is the loop's `internal_loop_data.parent_ptr` (uSockets' libuv +/// backend brackets `uv_run` with these). +// HOST_EXPORT(Bun__JSEventLoop__enter, c) +pub fn js_event_loop_enter(this: &mut crate::event_loop::EventLoop) { + this.enter(); +} + +// HOST_EXPORT(Bun__JSEventLoop__exit, c) +pub fn js_event_loop_exit(this: &mut crate::event_loop::EventLoop) { + this.exit(); +} + // ────────────────────────────────────────────────────────────────────────── // `bun_event_loop::any_event_loop::js` extern impls // From 8eae08fbda0f8a4e4db75b2c97757a56bbb37677 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 21 Aug 2026 17:19:53 -0700 Subject: [PATCH 2/2] usockets (Windows): let only the outermost tick free what a nested tick closed The event-loop scope around uv_run only moves microtask-driven re-entry out of it. A callback whose own body drives the loop again - bun:test's un-awaited expect(promise).resolves inside a socket data() handler, process.exit()'s drain - still nests uv_run inside the outer run's dispatch, and that dispatch reads what it dispatched on once the callback returns. Two things a nested run freed under it on Windows: - The closed usockets socket. us_internal_loop_post already defers freeing the loop's closed sockets while loop->data.tick_depth > 1; the epoll/kqueue ticks maintain tick_depth, the libuv backend never did. us_loop_run / us_loop_pump now count it in the same bracket as the scope. - The libuv handle under it. uv_run completes handle closes (endgame: unlink, then the close callback that frees the handle) right after the check phase. A poll closed from its own poll_cb had its close completed by the nested run; once poll_cb returned, uv__fast_poll_process_poll_req found it "closing, nothing outstanding" again and queued the endgame a second time - close callback twice, uv__handle_close on an unlinked handle, and the us_poll_start_rc / allocator free-list crashes that follow. check_cb (which runs immediately before uv__process_endgames) now takes the queued endgames off the loop while tick_depth > 1 and hands them back at the outermost tick. New socket.test.ts case: a Bun.connect socket terminate()d inside data() followed by nested event-loop ticks and same-size-class allocation churn; it segfaulted every run on a Windows release build with the scope alone. --- packages/bun-usockets/src/eventing/libuv.c | 55 ++++++++++++++++++- .../bun-usockets/src/internal/loop_data.h | 15 +++-- src/uws_sys/InternalLoopData.rs | 4 ++ .../close-inside-data-reentrant-fixture.ts | 49 +++++++++++++++++ test/js/bun/net/socket.test.ts | 22 ++++++++ 5 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 test/js/bun/net/close-inside-data-reentrant-fixture.ts diff --git a/packages/bun-usockets/src/eventing/libuv.c b/packages/bun-usockets/src/eventing/libuv.c index 6c6f88183b3f..0f6f627b1a4a 100644 --- a/packages/bun-usockets/src/eventing/libuv.c +++ b/packages/bun-usockets/src/eventing/libuv.c @@ -170,10 +170,48 @@ static void prepare_cb(uv_prepare_t *p) { us_internal_loop_pre(loop); } -/* Note: libuv timers execute AFTER the post callback */ +/* uv_run finishes closing handles - unlink from the loop, then the close + * callback, which is where an owner frees its handle - right after the check + * phase (uv__process_endgames). A nested tick (see us_internal_uv_run) must + * not: it runs inside a callback that an outer uv_run dispatched, and libuv's + * dispatch reads that callback's handle again once it returns. A poll closed + * from its own poll_cb (us_socket_close -> us_poll_stop) is the common case: + * the nested run would complete its close, and uv__fast_poll_process_poll_req + * would then find the handle "closing, nothing outstanding" a second time and + * queue its endgame again - two close callbacks, and uv__handle_close on an + * already unlinked handle. So a nested tick takes what it queued for closing + * off the loop and the outermost tick puts it back. A held handle keeps + * UV_HANDLE_ENDGAME_QUEUED set, so uv__want_endgame leaves it alone. */ +static void us_internal_hold_endgames(struct us_loop_t *loop) { + uv_handle_t *queued = loop->uv_loop->endgame_handles; + if (!queued) return; + uv_handle_t *last = queued; + while (last->endgame_next) last = last->endgame_next; + last->endgame_next = (uv_handle_t *)loop->data.held_endgames; + loop->data.held_endgames = queued; + loop->uv_loop->endgame_handles = NULL; +} + +static void us_internal_release_held_endgames(struct us_loop_t *loop) { + uv_handle_t *held = (uv_handle_t *)loop->data.held_endgames; + if (!held) return; + uv_handle_t *last = held; + while (last->endgame_next) last = last->endgame_next; + last->endgame_next = loop->uv_loop->endgame_handles; + loop->uv_loop->endgame_handles = held; + loop->data.held_endgames = NULL; +} + +/* Note: libuv timers execute AFTER the post callback; uv__process_endgames is + * what runs right after it. */ static void check_cb(uv_check_t *p) { struct us_loop_t *loop = p->data; us_internal_loop_post(loop); + if (loop->data.tick_depth > 1) { + us_internal_hold_endgames(loop); + } else { + us_internal_release_held_endgames(loop); + } } /* Not used for polls, since polls need two frees */ @@ -345,7 +383,22 @@ static void *us_internal_js_event_loop(struct us_loop_t *loop) { static void us_internal_uv_run(struct us_loop_t *loop, uv_run_mode mode) { void *js_event_loop = us_internal_js_event_loop(loop); if (js_event_loop) Bun__JSEventLoop__enter(js_event_loop); + /* The scope above only moves microtask-driven re-entry out of uv_run. A + * callback whose own body drives the loop again (waitForPromise: bun:test's + * expect(promise).resolves, process.exit()'s drain) still nests a tick inside + * the outer uv_run's dispatch, which reads what it dispatched on once the + * callback returns. tick_depth > 1 marks that nested tick, and what the outer + * dispatch may still point at is then left for the outermost tick to free: + * closed sockets (us_internal_loop_post) and closing libuv handles + * (check_cb). Same bracket as us_loop_run / us_loop_run_bun_tick on + * epoll/kqueue. */ + loop->data.tick_depth++; uv_run(loop->uv_loop, mode); + loop->data.tick_depth--; + /* A nested tick run from a timer callback holds after this run's check + * phase; nothing of libuv is on the stack any more, so hand those back for + * the next run to close. */ + if (loop->data.tick_depth == 0) us_internal_release_held_endgames(loop); if (js_event_loop) Bun__JSEventLoop__exit(js_event_loop); } diff --git a/packages/bun-usockets/src/internal/loop_data.h b/packages/bun-usockets/src/internal/loop_data.h index 3937ea5d70d5..364916c19c9a 100644 --- a/packages/bun-usockets/src/internal/loop_data.h +++ b/packages/bun-usockets/src/internal/loop_data.h @@ -95,11 +95,18 @@ struct us_internal_loop_data_t { /* We do not care if this flips or not, it doesn't matter */ size_t iteration_nr; void* jsc_vm; - /* Reentrancy depth of us_loop_run_bun_tick. When >1, we are inside a - * nested tick (e.g. waitForPromise from a poll callback). Freeing closed - * sockets must be deferred to the outermost tick so the outer dispatch - * doesn't read a freed poll. */ + /* Reentrancy depth of the loop tick (us_loop_run_bun_tick on epoll/kqueue, + * us_loop_run / us_loop_pump on libuv). When >1, we are inside a nested + * tick (e.g. waitForPromise from a poll callback). Freeing closed sockets + * must be deferred to the outermost tick so the outer dispatch doesn't + * read a freed poll. */ int tick_depth; +#ifdef LIBUS_USE_LIBUV + /* uv_handle_t list (linked through endgame_next): handles a nested tick + * would have finished closing, held for the outermost tick. See check_cb + * in libuv.c. */ + void *held_endgames; +#endif }; #endif // LOOP_DATA_H diff --git a/src/uws_sys/InternalLoopData.rs b/src/uws_sys/InternalLoopData.rs index 5add2c03a003..9d39d073964c 100644 --- a/src/uws_sys/InternalLoopData.rs +++ b/src/uws_sys/InternalLoopData.rs @@ -63,6 +63,10 @@ pub struct InternalLoopData { // Higher tier (`bun_runtime`) casts this back when reading. pub jsc_vm: *const c_void, pub tick_depth: c_int, + /// `uv_handle_t *` list of closing handles held back from a nested tick + /// (libuv.c `check_cb`). + #[cfg(windows)] + pub held_endgames: *mut c_void, } impl InternalLoopData { diff --git a/test/js/bun/net/close-inside-data-reentrant-fixture.ts b/test/js/bun/net/close-inside-data-reentrant-fixture.ts new file mode 100644 index 000000000000..b56957a32542 --- /dev/null +++ b/test/js/bun/net/close-inside-data-reentrant-fixture.ts @@ -0,0 +1,49 @@ +// Spawned by socket.test.ts as `bun test `: it has to run under the +// test runner because expect(promise).resolves waits by driving the event loop +// synchronously, which is what nests event-loop ticks inside the socket's data +// callback while the dispatch for that socket is still on the stack. +import { expect, test } from "bun:test"; + +test("a socket closed inside its data callback survives nested event-loop ticks until the dispatch returns", async () => { + using server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + open(socket) { + socket.write("x"); + }, + data() {}, + }, + }); + + for (let i = 0; i < 8; i++) { + const returned = Promise.withResolvers(); + const churn: Promise[] = []; + await Bun.connect({ + hostname: "127.0.0.1", + port: server.port, + socket: { + data(socket) { + // Closing moves the socket to the loop's closed list; it may only be + // freed once this callback (and the dispatch that called it) is done. + socket.terminate(); + // Nested ticks: timers, I/O and the loop's post phase all run here. + expect(new Promise(resolve => setTimeout(resolve, 5))).resolves.toBeUndefined(); + // Allocations of the same size class as the closed socket, so a + // prematurely freed block is likely to be handed out again before + // the outer dispatch looks at it. + for (let j = 0; j < 16; j++) { + churn.push( + Bun.connect({ hostname: "127.0.0.1", port: server.port, socket: { data() {} } }).then(s => s.terminate()), + ); + } + returned.resolve(); + }, + }, + }); + await returned.promise; + await Promise.all(churn); + // Let the outer dispatch unwind and the loop reach its post phase. + await new Promise(resolve => setImmediate(resolve)); + } +}); diff --git a/test/js/bun/net/socket.test.ts b/test/js/bun/net/socket.test.ts index a6ab1896db09..074acc7f136a 100644 --- a/test/js/bun/net/socket.test.ts +++ b/test/js/bun/net/socket.test.ts @@ -4433,3 +4433,25 @@ describe.concurrent("a socket closed by data() while its peer's reset is being d expect(exitCode).toBe(0); }); }); + +describe.concurrent("a socket closed by data() which then re-enters the event loop before returning", () => { + // The fixture runs under `bun test` so that expect(promise).resolves can drive + // nested event-loop ticks from inside the data callback. The closed socket must + // stay allocated until the dispatch that invoked data() has returned; the loop + // used to free it from a nested tick on Windows, and the outer dispatch then + // read (and the allocator reused) freed memory. + it("is not freed until the dispatch that called data() has returned", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", fileURLToPath(new URL("./close-inside-data-reentrant-fixture.ts", import.meta.url))], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // stdout carries only the runner's version banner; results go to stderr. + expect(stdout).toMatch(/^bun test v\S+ \(\S+\)\n$/); + expect(stderr).toContain(" 1 pass"); + expect(proc.signalCode).toBeNull(); + expect(exitCode).toBe(0); + }); +});