usockets(windows): keep a closed poll alive until the outer tick and libuv are done with it - #39643
usockets(windows): keep a closed poll alive until the outer tick and libuv are done with it#39643robobun wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughChangesSocket poll lifecycle
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status: ready to merge. Head Reproduced on Windows x64 with the new case in
The first push ( No libuv patch. The |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and the automated bug-hunt found no issues. Because it reworks lifetime/ownership of uv_poll_t blocks in the Windows usockets backend — memory-safety-critical native code with subtle ordering across nested uv_run — a human look is still worthwhile.
What was reviewed:
- Traced the
released/uv_closedhand-off across allus_poll_stop/us_poll_freecall sites and theus_poll_start_rcinit-failure path — each block is freed exactly once on every path. - Checked
us_poll_resize: the memcpy carriespoll_cb_depth/stoppedto the new block andpoll_cbre-readsp->dataafter dispatch, so the decrement and deferreduv_closeland on the live block; the old block's stale counter is unreachable (uv_p == NULL→ fast-path free). - Confirmed
us_internal_loop_data_tlayout is unchanged (comment-only edit), so the Rust mirror needs no update;us_poll_thas no Rust mirror. - The early-return in
poll_cbbefore the depth increment runs no JS, so it cannot raceus_poll_stop.
Extended reasoning...
Overview
This PR fixes a Windows-only use-after-free / double-free in the usockets libuv backend when a socket handler re-enters the event loop (e.g. expect(promise).resolves, auto-install). It touches packages/bun-usockets/src/eventing/libuv.c (poll lifecycle: poll_cb, us_poll_stop, us_poll_free, us_poll_start_rc, close_cb_free_poll, us_loop_run/us_loop_pump), adds four state fields to us_poll_t in libuv.h, updates a comment in loop_data.h, and adds a subprocess regression test to test/js/bun/net/socket.test.ts.
The mechanism: (1) count tick_depth on the libuv backend so us_internal_loop_post defers freeing closed sockets past nested ticks (POSIX parity); (2) defer uv_close on a poll until the outermost poll_cb frame for that handle unwinds, so libuv's outer uv__fast_poll_process_poll_req never sees a handle whose endgame already ran; (3) replace the old data = NULL handshake between us_poll_stop and close_cb_free_poll with two explicit bits (released, uv_closed) so whichever runs second frees both blocks — the old scheme leaked when the callback ran first.
Security risks
None user-facing. This is internal event-loop plumbing; no untrusted input parsing, no auth/crypto. The risk class is memory safety (UAF, double-free, leak) in a hot path that every Windows socket goes through.
Level of scrutiny
High. Per the repo's review guidance, native memory safety is the most-blocked category. This change rewrites the ownership contract between two heap blocks (us_poll_t and its uv_poll_t) across three release points (us_poll_stop, us_poll_free, close_cb_free_poll) and a re-entrant uv_run. The reasoning in the PR description is thorough and the code reads correctly to me, but the failure mode of a mistake here is heap corruption on every Windows user's machine, and one defensive branch (the uv_poll_init_socket half-init failure path) was reasoned about rather than exercised. That warrants a maintainer's eyes.
Other factors
- I traced every
us_poll_stop/us_poll_free/us_poll_start_rccaller inloop.c,socket.c,context.c, andudp.c; each follows create → start → stop → free, matching the new one-waystoppedcontract. Theus_poll_start_rc"already registered" branch andus_poll_changeafter stop are defensive with no current caller (as the PR notes). us_poll_resizeinteraction checks out: memcpy copies the new fields to the replacement block,p->datais repointed, andpoll_cbre-reads it after dispatch; the old block'suv_p = NULLsends its laterus_poll_freedown the fast path so its stalepoll_cb_depthis never consulted.loop_data.hchange is comment-only; the Rust mirror insrc/uws_sys/InternalLoopData.rsneeds no update. No Rust mirror ofus_poll_texists.- Test follows harness conventions (subprocess,
tempDir,bunEnv, drains both pipes concurrently, asserts a combined object). The 20 msBun.sleepinside the fixture is the deliberate mechanism (drive nested ticks), not a wait-for-condition, and is commented as such.
|
Updated 7:07 PM PT - Aug 21st, 2026
❌ @robobun, your commit 174085f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 39643That installs a local version of the PR into your bun-39643 --bun |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it redesigns the two-block ownership handoff for every Windows socket close (a memory-safety-critical path where getting it wrong trades one UAF for another) and introduces a five-bit state machine (stopped/close_fd/uv_closed/released/poll_cb_depth), a human look would still be worthwhile — particularly to sign off on this shape versus the earlier libuv-patch approach from #33018.
Checked: us_poll_resize copies poll_cb_depth via memcpy before p->data is repointed, so the epilogue's re-read lands on the right counter; us_socket_detach (stop without close_fd) keeps the fd open through the deferred uv_close; the us_poll_start_rc init-failure path composes with the caller's us_poll_free via released; the four bsd_close_socket(us_poll_fd(...)) sites that follow a stop are all converted, and the remaining one in loop.c:862 is epoll-gated.
Extended reasoning...
Overview
This PR fixes Windows crashes (BUN-43AH/442Y/442Z, deinitialization.test.ts) caused by nested uv_run freeing a socket the outer dispatch still holds, and by uv_close running under an outer uv__fast_poll_process_poll_req frame that then queues the endgame twice. The fix has three parts: (1) us_loop_run/us_loop_pump now count tick_depth so us_internal_loop_post defers the closed-socket free to the outermost tick, matching POSIX; (2) a per-poll poll_cb_depth counter defers uv_close to the outermost poll_cb frame's epilogue, so libuv sees a supported close-from-inside-callback instead of a close from a nested run; (3) the old data = NULL handshake between us_poll_stop/us_poll_free/close_cb_free_poll is replaced by explicit released/uv_closed bits so whichever runs second frees both blocks (the old scheme leaked when the callback ran first). A new us_internal_poll_close_fd helper defers closesocket past the deferred uv_close so AppContainer strict-handle-checks don't kill the process on the cancel ioctl. Call sites in context.c, socket.c, udp.c are updated; epoll_kqueue.c gets a pass-through implementation.
Security risks
None identified. This is internal event-loop lifecycle management with no user-controlled input reaching the changed paths.
Level of scrutiny
High. This is the most-blocked category in Bun's review guide (native memory safety): every acquisition/release pairing on the Windows socket-close path is being reorganized, and the failure mode of a mistake is exactly the UAF/double-free class being fixed. The state machine has five interacting bits plus a depth counter, and correctness depends on invariants that span us_poll_stop → us_internal_poll_close_fd → poll_cb epilogue → close_cb_free_poll → us_poll_free, with us_poll_resize cutting across. The PR description is exceptionally thorough (symbolized fail-before, mechanism for each Sentry shape, ordering rationale, defensive-branch inventory) and the author ran the relevant Windows suites, but the changed code cannot be exercised on this Linux environment and the design choice (per-poll depth counter + deferred close vs. #33018's libuv patch) is one a maintainer should ratify.
Other factors
I traced the us_poll_resize interaction: memcpy copies poll_cb_depth after the outer frame's increment, and the epilogue re-reads p->data to land on the relocated block, so nested dispatch + adopt + close balances correctly. us_socket_detach correctly does not set close_fd, so the fd survives the deferred uv_close for handoff. The one remaining us_poll_stop not followed by us_internal_poll_close_fd at loop.c:862 is under #ifdef LIBUS_USE_EPOLL. The us_poll_start_rc early-return for an already-registered handle and the us_poll_change guard on stopped are defensive (no current caller per the PR notes) but harmless. The new socket.test.ts case is a spawned crash repro with a snapshot on {stdout, exitCode}; the appcontainer.test.ts addition correctly notes the exit code is the real assertion. The PR notes that the uv_poll_init_socket failure path was reasoned about but not run (needs a fault-injection build) — that path now routes through us_poll_stop, which is a behavior change worth a maintainer's eye.
|
Data point from the Symbolized CI crash. Build 101365 (Windows x64, The crash comes right after the How the fixture gets there. The test body resumes inside the HMR client socket's Reduction without the dev server. The import { expect, test } from "bun:test";
test("peer closes the socket during a nested tick inside its own poll callback", async () => {
const server = Bun.serve({
port: 0,
fetch(req, server) {
if (server.upgrade(req)) return;
return new Response("no");
},
websocket: { message() {} },
});
const { promise: done, resolve: finish } = Promise.withResolvers<void>();
const ws = new WebSocket(`ws://localhost:${server.port}/`);
ws.onopen = () => {
const closed = new Promise<number>(resolve => (ws.onclose = e => resolve(e.code))).then(code =>
Bun.sleep(100).then(() => code),
);
setTimeout(() => server.stop(true), 10);
// Not awaited on purpose: runs a nested event loop tick inside this poll callback.
expect(closed).resolves.toBe(1006);
console.log("nested tick returned, readyState =", ws.readyState);
finish();
};
await done;
console.log("onopen returned");
// Like the fixture's next case: allocate a batch of fresh connecting sockets.
await using server2 = Bun.serve({
port: 0,
fetch(req, server) {
if (server.upgrade(req)) return;
return new Response("no");
},
websocket: { message() {} },
});
const opens: Promise<void>[] = [];
const sockets: WebSocket[] = [];
for (let i = 0; i < 8; i++) {
const { promise, resolve, reject } = Promise.withResolvers<void>();
const s = new WebSocket(`ws://localhost:${server2.port}/`);
s.onopen = () => resolve();
s.onclose = () => reject(new Error(`socket ${i} closed before open`));
sockets.push(s);
opens.push(promise);
}
await Promise.all(opens);
console.log("all 8 follow-up sockets opened");
const closes = sockets.map(s => new Promise<void>(r => (s.onclose = () => r())));
for (const s of sockets) s.close();
await Promise.all(closes);
});Results on one Windows x64 machine:
This reduction closes the socket from a second |
|
For reference from #39488: the test/bake/deinitialization.test.ts Windows x64 segfaults on that branch (builds 101180 and 101478, both at the start of the websocket=8 case) symbolise to heap corruption in the client us_socket_t size class surfacing under us_internal_drain_pending_dns_resolve, i.e. the family this PR fixes. I briefly carried the tick_depth-only subset there and backed it out in favour of this landing whole. |
|
Thanks, that shape fits. The second Current state: |
|
Release build evidence for the A/B on one machine (Windows Server 2019 x64, CPU without AVX, so the build is the baseline variant like the CI lane). Both binaries are local
The PR body lists a debug build for this test. The numbers above cover the release build, which is the build that hangs or crashes on the CI lanes. |
|
Verified this branch ( Repro: a One note on where |
|
State after the merge of main ( The notes in the description now also record one limit. An addon that calls |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it redesigns the Windows poll lifecycle state machine (five new lifecycle bits plus poll_cb_depth, deferred uv_close/fd-close ordering, and a new released/uv_closed free handshake) across every socket close path, a human look from someone with libuv/usockets ownership would still be worthwhile.
What was reviewed:
- The
released/uv_closedhandshake inclose_cb_free_poll/us_poll_free— each block is freed exactly once regardless of which runs first; the olddata = NULLleak (#37105) is covered. poll_cbre-readingp->dataafter dispatch so a mid-dispatchus_poll_resizecarriespoll_cb_depth/stoppedto the new block, and the retired block'suv_p = NULLshort-circuitsus_poll_stop/us_poll_free.us_internal_poll_close_fdon epoll/kqueue is a no-op wrapper overbsd_close_socket, and the kqueue branch ofclose_raw(which skipsus_poll_stop) still closes immediately there.tick_depthnow bracketsus_loop_run/us_loop_pump, socheck_cb→us_internal_loop_post'stick_depth <= 1gate holds for nesteduv_run; no Rust-side layout mirror ofus_poll_texists to fall out of sync.
Extended reasoning...
Overview
This PR fixes a family of Windows-only use-after-free / double-free crashes in bun-usockets' libuv backend that surface when a socket handler drives a nested uv_run (e.g. expect(promise).resolves, auto-install). It touches eventing/libuv.c (the core lifecycle rewrite), internal/eventing/libuv.h (five new bitfields plus poll_cb_depth on us_poll_t), epoll_kqueue.c/internal.h (a pass-through us_internal_poll_close_fd), loop_data.h (comment only), and rewires four close sites in context.c/socket.c/udp.c from bsd_close_socket(us_poll_fd(p)) to the new helper. Two tests are added: a subprocess fixture in socket.test.ts that reproduced the BUN-43AH segfault on main, and an AppContainer step guarding the strict-handle-check ordering.
Security risks
None identified. This is internal event-loop lifecycle plumbing with no user-controlled input reaching the new state; the changes narrow, rather than widen, the window in which freed memory can be touched.
Level of scrutiny
High. This is native memory-safety code that changes when uv_close, the socket closesocket(), and the two heap frees run relative to poll-callback frames and nested uv_runs. The design introduces a small state machine (stopped/close_fd/uv_closed/released/poll_cb_depth) whose invariants must hold across adoption/resize, the uv_poll_init_socket failure path, us_socket_detach, and UDP. The PR body documents each invariant and the CI iteration that caught the fd-before-uv_close ordering (AppContainer 0xC0000008), and the A/B numbers on deinitialization.test.ts are compelling — but the design choices (defer uv_close to the outermost poll_cb frame rather than patching libuv; count tick_depth at us_loop_run/us_loop_pump rather than in every libuv callback) are exactly the kind a maintainer should ratify.
Other factors
- The bug-hunting pass found nothing. I traced the free protocol in both orders, the resize path (memcpy carries the counter; the retired block's
uv_p = NULLmakesus_poll_stop/us_poll_freeno-ops on it), the kqueueclose_rawbranch (still closes immediately since epoll/kqueue'sus_internal_poll_close_fdis justbsd_close_socket), andus_socket_detach(defersuv_close, leavesclose_fd = 0, so the fd survives as intended). us_internal_loop_postatloop.c:446already gates the closed-list free ontick_depth <= 1; this PR wires the libuv entry points into that counter, matchingus_loop_run_bun_tickon POSIX. I confirmed there is no Rust#[repr(C)]mirror ofus_poll_tthat the added fields would desync.- The PR notes an acknowledged limit (native addons calling
uv_rundirectly are not counted bytick_depth) and defers it; that is a design call worth a maintainer's nod. - Build 101247 was green on all lanes at
f91d44edb8; the diff is unchanged since. alii's comment corroborates this fixes the #39488 Windows failures.
|
Cross reference: #39910 (opened 2026-08-21, CI green) fixes the same bug. It also counts I built both heads (
The full comparison is in #39910 (comment). It lists the hunks and tests in this PR that #39910 does not have: the I am not closing either PR. One of the two should land and the other should close. If #39910 lands first, the close order hunks and the appcontainer step from here should follow it as a rebased follow-up, or AppContainer processes regress from main. |
…libuv are done with it The libuv backend never counted tick_depth, so a nested tick (a handler that waits for a promise) freed the socket that the outer dispatch was still reading. The nested uv_run also ran the endgame of that socket's uv_poll_t under libuv's outer uv__fast_poll_process_poll_req frame, which then queued the endgame again, and close_cb_free_poll freed both blocks twice. us_loop_run and us_loop_pump now count tick_depth like the POSIX backend. While a poll_cb frame for a poll is on the stack, us_poll_stop only disarms the handle; the outermost frame issues the uv_close on its way back into libuv and then closes the socket, which the close paths hand over through us_internal_poll_close_fd (uv_close cancels the in-flight request with an ioctl on the socket, and a process with strict handle checks, such as an AppContainer, dies if the socket is already closed). us_poll_free and close_cb_free_poll record which of them ran first and the second one frees both blocks, which also covers a close callback that runs before us_poll_free, a us_poll_free without us_poll_stop, and us_poll_start_rc or us_poll_change on a handle libuv still references.
063e4c0 to
174085f
Compare
There was a problem hiding this comment.
I reviewed this PR and the automated bug hunt found no issues. Given the scope — a redesign of the libuv poll lifecycle (deferred uv_close via poll_cb_depth, the released/uv_closed two-block hand-off, deferred fd close for the AppContainer ordering) — a human look is still warranted, and there's a competing design in #39910 to decide between.
Checked: the four bsd_close_socket → us_internal_poll_close_fd call sites all follow us_poll_stop (or its kqueue equivalent), so the epoll/kqueue path is a straight rename. us_poll_resize copies the new bitfields and poll_cb_depth into the replacement block, and poll_cb re-reads p->data after dispatch, so a mid-dispatch adopt keeps the counter and deferred-close state consistent. The h->type != UV_POLL guard in us_poll_stop/us_poll_free relies on us_create_poll's new us_calloc zeroing, which is in place.
Extended reasoning...
Overview
This PR reworks the Windows/libuv poll lifecycle in packages/bun-usockets to fix a family of use-after-free crashes (BUN-43AH, BUN-442H/Y/Z, BUN-4NC8, BUN-4NP3, and four TLS variants) that occur when a socket handler synchronously waits for a promise, which re-enters uv_run while the outer uv__fast_poll_process_poll_req frame still holds the handle. The fix has three interlocking pieces: (1) tick_depth is now counted around uv_run in us_loop_run/us_loop_pump so us_internal_loop_post defers freeing the closed list to the outermost tick (POSIX parity); (2) us_poll_stop only disarms the handle while poll_cb_depth > 0, and the outermost poll_cb frame issues uv_close on its way back into libuv, which is the reentrancy shape libuv supports; (3) us_poll_free and close_cb_free_poll coordinate via released/uv_closed bits so whichever runs second frees both blocks. A new us_internal_poll_close_fd helper defers the socket close until after the deferred uv_close is issued, preserving the order libuv's cancel ioctl needs (AppContainers with strict handle checks die otherwise — caught by CI on the first push). Four call sites in context.c/socket.c/udp.c are rewired through the helper; on epoll/kqueue it is a straight bsd_close_socket wrapper.
Security risks
None identified. This is event-loop lifecycle and memory-ownership code, not input parsing or auth/crypto. The change closes UAF/double-free bugs rather than introducing new attack surface.
Level of scrutiny
High. This is native C in the event loop, the "memory safety (the most-blocked category)" section of the review rules. It redesigns two-block ownership across an asynchronous libuv close callback, adds five state fields to us_poll_t, and changes when uv_close and closesocket run relative to nested uv_run frames. The PR description is exceptionally thorough (symbolized fail-before, A/B on release builds, Sentry group mapping, the AppContainer regression it caught and fixed mid-PR), and CI is green on the merged head. But the design has an acknowledged limit (native addons calling uv_run directly), one defensive path was "reasoned about, not run" (the fault-injection failure branch), and #39910 offers a smaller competing design that this PR's author has A/B-tested and shown to regress the AppContainer case — that trade-off is a design decision a maintainer should make.
Other factors
- No prior human review on the thread; the only non-bot comment (from alii) is a cross-reference confirming this fixes their branch's crashes too.
- Tests: a new subprocess-spawning case in
socket.test.tsthat reproduces the crash on unfixed builds (verified in the PR notes), and a newcloseInHandlerstep inappcontainer.test.tsthat guards the socket-close ordering. Both are Windows-relevant and cannot run on this reviewer's platform. - The
us_poll_tstruct grows by ~5 bytes of bitfields plus anunsigned int;us_create_pollinitializes all five new fields explicitly, andus_poll_resize'smemcpycarries them to the replacement block. - I traced
us_poll_resizeagainst the newpoll_cbepilogue: the epilogue re-readswp = p->dataafter dispatch, and resize setsp->uv_p = NULLon the retired block while pointinguv_p->dataat the new one, so the deferreduv_closeandclose_fdland on the live block. The retired block's ownus_poll_free(fromus_internal_free_closed_sockets) takes the!hfast path. - The
h->type == UV_POLLre-start guard inus_poll_start_rcand thep->stoppedguard inus_poll_changeare noted as having no current caller; they are defensive and look correct against the state machine described in the header comments.
|
For whoever merges: #39910 took the same two steps ( Build 103029 on the rebased head |
3 participants
Problem
us_socket_is_closedorus_internal_socket_follow_adoptedat0xFFFFFFFFFFFFFFFF(BUN-43AH, BUN-4NC8),us_internal_socket_close_rawon a freed socket (BUN-442H, BUN-442Y, BUN-4NP3), a garbagepoll_cbcalled fromuv__fast_poll_process_poll_req(BUN-442Z), mimalloc free-list crashes, and thetest/bake/deinitialization.test.tsfailures on the Windows lanes. The 1.4.0 release (34cbb9a40) reports this family frombun teston Windows (as of 2026-08-21: BUN-442H 12 events, BUN-4NC8 6, BUN-442Y 5, BUN-4NP3 2, BUN-442Z 1) and the TLS forms BUN-4MNY, BUN-4NFZ, BUN-4NJG and BUN-4NKV, one each.loop.c:450frees closed sockets only attick_depth <= 1. The libuv backend never counts it, so a handler that waits for a promise (a nesteduv_run) frees the socket the outer dispatch still reads (loop.c:746).uv_poll_tunder libuv's outeruv__fast_poll_process_poll_reqframe. That frame then queues the endgame again, andclose_cb_free_pollruns twice.uv_runis documented as not reentrant, so the misuse is ours.Fix
us_loop_runandus_loop_pumpcounttick_depth, as the POSIX backend does.poll_cbframe is on the stack (poll_cb_depth),us_poll_stoponly disarms the handle. The outermost frame callsuv_closeon its way back into libuv, which libuv supports, and then closes the socket (us_internal_poll_close_fd), so libuv sees the same order as before. libuv is unchanged.us_poll_freeandclose_cb_free_pollrecord who ran first (released,uv_closed). The second one frees both blocks. The olddata = NULLhandshake leaked when the callback ran first (usockets(win): free the poll when us_poll_free runs after the uv close callback #37105). A stopped poll is never re-armed or re-initialized.test/js/bun/net/socket.test.ts. On a Windows debug build of main its child segfaults at0xFFFFFFFFFFFFFFFF(the BUN-43AH signature), and the release canary crashes too. It passes with the fix (Windows x64 and arm64 debug, Linux ASAN).appcontainer.test.tsgets a step for the socket order. Notes have the rest.Background
us_socket_t(it starts with theus_poll_t) and a libuvuv_poll_t. libuv's in-flight AFD requests live inside theuv_poll_t, so it has to live until the close callback. A closed socket waits onloop->data.closed_headuntilus_internal_loop_postfrees it, andloop.creads it after its handlers return.uv_closecancels the requests. When the last one completes, libuv queues the endgame, which unlinks the handle and runs the close callback.uv__fast_poll_process_poll_reqchecks for it right afterpoll_cbreturns.wait_for_promise(expect().resolves, auto-install) runsuv_runinside the handler.Notes
Symbolized fail-before (Windows x64 debug build of main
a35696478d, a standalone copy of the new test's fixture. The test itself fails on that build with the same fault address):The freed socket is filled with mimalloc's debug poison, so
flags.adoptedreads as set andprevis followed. Theopen()variant crashes the same way atloop.c:556. The same fixture without the nested wait passes on that build. The release canary (1.4.0-canary.1+32e87032b) passes the first case and crashes in the second, at0xFFFFFFFFFFFFFFFFin one run and0x1AF0000002Ain another, which is the heap corruption from the double free.How the three Sentry shapes follow. In release,
mi_freeoverwrites the first word of the freeduv_poll_t, which isdata. The rest stays intact, so the outer frame seesevents == 0,CLOSING, and no requests in flight, and queues the endgame again. The secondclose_cb_free_pollfreesh->data, now the free-list link to another freed block, andhitself again. Later allocations alias (a garbagepoll_cb, BUN-442Z, orus_poll_start_rcfaulting at 0 indeinitialization.test.ts), or the freed socket is reused and the outer dispatch runs the error close on it (BUN-442Y), orfollow_adoptedreads a reused block (BUN-43AH).Socket order, found by CI. The first push closed the socket in
close_rawbefore the deferreduv_close.appcontainer.test.tsfailed on the Windows 11 arm64 lane with exit0xC0000008(STATUS_INVALID_HANDLE), and failed 10 of 10 runs on an arm64 machine with that build against 5 of 5 passes with main'slibuv.con the same machine.GetProcessMitigationPolicy(ProcessStrictHandleCheckPolicy)inside the container reports0x3, so a call on a closed handle raises instead of failing, anduv__poll_closecancels the in-flight request with an ioctl on the socket. A probe with four steps (listener only, serve + fetch, terminate fromdata(), terminate from a timer with the peer closed by its own dispatch) died in every step that closes a socket from a dispatch. Withus_internal_poll_close_fdthe four steps and the test pass (10 of 10 runs on arm64, and on x64). The test now also closes a socket from its owndata()handler, which is the shortest path to this order. Outside a nested tick, the later socket close is not observable from JS: the loop does not run again before the handler's dispatch ends. Inside one, a handler that closes its socket and then waits for the peer to notice now waits until the handler returns. That wait was already unreliable on Windows (the peer's completion is often in the outer run's batch) and then corrupted the heap.Why the hand-off is needed together with
tick_depth. Withtick_depthalone, every socket closed during a nested tick with no live frame finishes closing in the inner run, and the deferredus_poll_freethen met a handle whose close callback had already run: the old code handed itdataand leaked both blocks. #37105 fixes that case on its own with a marker indata. This PR covers it with the two bits.Defensive branches with no current caller:
us_poll_freeon a poll that was never stopped,us_poll_start_rcon a registered poll (mask change, orUV_EBADFonce stopped),us_poll_changeafter stop. Every current caller creates, starts, and later stops a poll exactly once. Theuv_poll_init_socketfailure path now goes throughus_poll_stop.uv_poll_stopon a half-initialized handle only clearsevents(checked againstuv__poll_set). That path was reasoned about, not run: it needs a--socketFaultInjection=onbuild, which rebuilds every Rust crate.Suites run on the Windows x64 debug build with the fix, all green:
test/js/bun/net/socket.test.ts(86 pass),tcp-server,socket-retention,socket-syscall-fault,test/js/bun/udp/udp_socket.test.ts,test/js/bun/websocket/websocket-server.test.ts,test/js/node/net/node-net-server.test.ts,test/js/node/tls/node-tls-server.test.ts,test/js/node/http/node-http.test.ts,test/js/bun/http/serve.test.ts(294 pass),test/bake/deinitialization.test.ts3 of 3 (the unfixed release canary hangs for 60 s and is killed on the same machine).test/js/web/fetch/fetch.test.tshad 14 timeouts of(with gc)body tests on this slow debug box. The two socket tests among them pass in isolation. Per the analysis in #33018,deinitialization.test.tscloses the socket from a nestedpoll_cbon the same handle, so it also coverspoll_cb_depth > 1.Sentry groups on the 1.4.0 release (
34cbb9a40, all Windows): BUN-4NC8 isfollow_adopted(internal.h:357) underloop.c:746, the frames of the fail-before above. BUN-442H isclose_rawatsocket.c:292, the low-prio unlink writing throughs->prevof a reused block, reached from the eof or error close at the end of the dispatch (BUN-442Y and BUN-4NP3 are the same call: the unlink atcontext.c:235and:240writing through the reused block'sprevandnext, andus_internal_disable_sweep_timerthrough itsgroup). BUN-4MNY isssl_retry_parked_write(openssl.c:2103,s->groupread as NULL) in the tail ofus_internal_ssl_on_dataafterus_dispatch_datareturned: the TLS form of the same read of a freed socket. BUN-4NFZ is the other exit of that tail:us_internal_ssl_close(openssl.c:1927, agains->groupNULL) from thessl_closecall atopenssl.c:2324after the data callback. BUN-4NJG and BUN-4NKV are the keylog and session flushes in the same tail (openssl.c:2430to:351, and:2429to:426):s->sslread from the reused block, soSSL_get_ex_datareturns garbage. Both flushes are guarded by!s->ssl || is_closed, which only a reused block gets past. The four TLS groups are the four helpers of that tail, in whichever order a given reused block fails. With the closed list left alone until the outermost tick,ssl_gone,is_closedands->sslread the real state there and those tails return.Limit.
tick_depthcounts the ticks Bun itself runs (us_loop_run,us_loop_pump). A native addon that callsuv_run()on this loop from inside a handler is not counted. The socket whose own callback is on the stack is still safe there, because itsuv_closeand frees are tied topoll_cb_depth, but a socket that another frame holds (an accepted socket closed from its ownopen()while the accept loop still reads it, or a block retired by adoption) is not. Counting the frames of the callbacks libuv runs for us (poll_cb,prepare_cb,timer_cb,async_cb) would cover that entry as well. None of the crashes above needed it, so it is left for a follow-up rather than re-verifying this PR for it.Release-build numbers for
test/bake/deinitialization.test.tsare in the comments below: on one Windows Server 2019 machine the fixture fails 8 of 8 runs on main (6 hangs, 2 segfaults) and passes 20 of 20 with this diff.Related. #33018 was an earlier attempt at this bug. It patched libuv's post-callback check instead of moving the
uv_close. #38024 has the same bug class in the c-ares poll and still carries that patch. Deferring itsuv_closepast the outermost callback frame would remove the need for it there too. The test fixture waits on a timer on purpose: a nested run cannot wait for an event of the peer socket, because that completion may sit in the outer run's IOCP batch.no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/windows/appcontainer.test.ts, test/js/bun/net/socket.test.ts