dns: keep the c-ares uv_poll_t alive while its poll callback is on the stack (Windows)

[robobun]

Problem

• Windows only. A dns.promises.Resolver (or dns.Resolver) query whose promise reaction synchronously spins the event loop crashes the release build after about ten queries: panic(main thread): Segmentation fault at address 0x0 (also seen as address 0x600000030). Levers that spin the loop from a reaction include Bun.build() with a plugin whose setup() returns a pending promise, and expect().resolves in bun:test.
• Each c-ares socket is driven by a uv_poll_t embedded in a heap UvDnsPoll (src/runtime/dns_jsc/dns.rs). c-ares closes a UDP socket as soon as its answer has been processed, which happens inside Channel::process in on_dns_poll_uv (dns.rs:4726 on main), so on_dns_socket_state issues the uv_close (dns.rs:4830) from inside the handle's own poll callback.
• on_dns_poll_uv then drains microtasks (EventLoopEnterGuard drop). A reaction that re-enters the loop runs a nested uv_run; its endgame phase invoked on_close_uv, which freed the UvDnsPoll immediately (dns.rs:4771).
• libuv's uv__fast_poll_process_poll_req frame for that handle is still suspended underneath on_dns_poll_uv and reads handle->events, submitted_events_* and flags once the callback returns (vendor src/win/poll.c, the block after poll_cb). On the stale bytes it re-queues the endgame of the already closed handle, on_close_uv runs a second time, and the struct is freed twice; mimalloc then hands the block out twice and a later round crashes.
• Same shape as the uSockets bug in Fix Windows heap corruption when a socket's data callback closes it and re-enters the event loop #33018, whose review (alii) pointed at this copy. The POSIX path polls c-ares sockets with FilePoll, whose dispatch does not touch the poll after the callback, and is not affected.

Fix

• UvDnsPoll counts the on_dns_poll_uv frames currently active for it (poll_cb_depth). on_close_uv frees the struct only when that is zero; otherwise it just marks the handle closed.
• When the outermost frame finishes (after its microtask drain, so after any nested uv_run) and sees closed, it hands the struct to the event loop task queue (ManagedTask::new_owned, the same primitive cares_jsc.rs already uses; it also frees the struct if the VM tears down first). Bun drains that queue from its own tick, never from inside the libuv callback that enqueued the task, so by the time the task runs the frame that enqueued it has returned and libuv's tail behind it has finished reading the handle.
• Why this is the right boundary: after on_dns_socket_state removes the map entry, the only things still referring to a UvDnsPoll are the suspended on_dns_poll_uv frames for it and libuv's tail under each of them, which is exactly what the counter tracks. libuv does not invoke poll_cb for a closing handle, so the count can only go down once uv_close was issued, and the close callback reaching a handle with a zero count means no frame holds it, which is today's behavior unchanged. Re-entrant dispatch of the same handle (depth 2) works the same way: inner frames only decrement, the outermost one defers.
• patches/libuv/win-poll-no-reendgame-after-close.patch (and its entry in deps/libuv.ts) is a byte-identical copy of the patch in Fix Windows heap corruption when a socket's data callback closes it and re-enters the event loop #33018: once the struct stays alive, unpatched libuv reads real flags in the tail and re-queues the endgame of the CLOSED handle, invoking close_cb twice (double free in release; in debug uv__poll_endgame asserts, trace below). The patch guards the re-queue on !(flags & UV_HANDLE_CLOSED), which uv__poll_endgame already asserts. Whichever of the two PRs lands second drops the duplicate on rebase.
• The common path is unchanged: with no re-entry, the close callback arrives after the poll callback returned, the count is zero, and on_close_uv frees immediately as before (trace below shows 20/20 queries taking that branch).
• Test: test/js/node/dns/node-dns.test.js, "dns.Resolver: a query's promise reaction may re-enter the event loop". A child answers 20 resolve4() queries from a local UDP responder and re-enters the loop from each reaction via Bun.build(); it also checks the re-entry really was synchronous (HTMLRewriter.transform(), which Fix Windows heap corruption when a socket's data callback closes it and re-enters the event loop #33018 uses, no longer spins the loop on main). Distinct names per query keep c-ares from answering later rounds from its own cache. Windows only (skipIf(!isWindows)).
• Verified on Windows x64:
  • USE_SYSTEM_BUN=1 bun test ... -t "promise reaction may re-enter" (canary 1.4.0, unfixed): fail, child exit 3 with the segfault above; the repro script crashed by round 9 or 10 in 10/10 runs.
  • bun bd test test/js/node/dns/node-dns.test.js -t "promise reaction may re-enter" on this branch: pass. Whole file: 133 pass, 0 fail (canary: 132 pass, only the new test failing).
  • test/js/node/dns/ + test/js/bun/dns/: 222 pass, 0 fail on rerun (one dns.getServers failure in the first run; it compares against node -e dns.getServers() and passed alone and in the full-file reruns on the same build).
  • 10 node test-dns-* / test-worker-dns-terminate-during-query tests that use local servers: all exit 0.
  • Since the libuv patch affects every uv_poll_t: socket.test.ts 76 pass, tcp-server, udp_socket, socket-retention, worker-terminate-lifetime 228 pass, 0 fail.
• Fail-before is visible on release builds only, which is what the Windows CI lanes run: in a debug build mimalloc fills freed memory with 0xDF, which makes libuv's stale read take its "both request slots busy" no-op branch, so the unfixed debug build reads freed memory silently. The instrumented traces below show the free inside the frame on the unfixed build regardless.

Background

• A libuv handle is closed with uv_close(handle, close_cb); libuv invokes close_cb later, from the endgame phase of a uv_run iteration, and the embedding struct may only be freed from close_cb or later. For a uv_poll_t the endgame is reached once its outstanding AFD poll requests have completed; bun's existing rearm patch means one is always outstanding during poll_cb, so uv_close from inside poll_cb cancels it and the endgame comes from whichever uv_run iteration dequeues that cancellation.
• uv__fast_poll_process_poll_req is the libuv function that calls poll_cb; after the callback it checks whether to resubmit a request or, for a closing handle, queue the endgame. libuv assumes uv_run is not re-entered from a callback, so it never expects the endgame to have run in between.
• bun does re-enter it: EventLoop::wait_for_promise (behind Bun.build() option parsing, bun:test's .resolves/.rejects, and others) calls auto_tick, which on Windows is us_loop_run/us_loop_pump, i.e. a nested uv_run.
• EventLoopEnterGuard (enter_event_loop_scope) is how native callbacks drain microtasks when they return; that drain is where the promise reactions run, still inside the libuv callback.
• ManagedTask is the event loop's one-off heap task; a task enqueued during a libuv callback runs when bun drains its task queue after uv_run returns, or is released (here: freed) if the VM tears down before that.

Instrumented traces (Windows x64 debug builds, one query shown; eprintln added locally, not in this diff)

Unfixed (main). close_cb frees the struct between the reaction's start and end, i.e. while poll_cb for the same handle is still on the stack; the same sequence repeated for all 20 queries:

[uv] poll_cb enter poll=0x222807e0540 fd=840 status=0 events=1
[uv] uv_close poll=0x222807e0540 fd=840
[js] round 0: before re-entry
[uv] close_cb poll=0x222807e0540
[js] round 0: after re-entry
[uv] poll_cb exit poll=0x222807e0540

This branch, with re-entry (20/20 deferred, 0 freed early, 20 deferred frees ran):

[uv] poll_cb enter poll=0x1ef007e0540 fd=832 depth=0
[uv] uv_close poll=0x1ef007e0540 fd=832
[js] round 0: before re-entry
[uv] close_cb poll=0x1ef007e0540 depth=1
[uv] close_cb: poll_cb frame active, deferring free poll=0x1ef007e0540
[js] round 0: after re-entry
[uv] enqueue deferred destroy poll=0x1ef007e0540
[uv] poll_cb exit poll=0x1ef007e0540
[uv] deferred destroy task ran poll=0x1ef007e0540

This branch, without re-entry (20/20 freed directly in close_cb, no task enqueued):

[uv] poll_cb enter poll=0x1dc007e0540 fd=828 depth=0
[uv] uv_close poll=0x1dc007e0540 fd=828
[uv] poll_cb exit poll=0x1dc007e0540
[uv] close_cb poll=0x1dc007e0540 depth=0
[uv] close_cb: freeing now poll=0x1dc007e0540

This branch's dns.rs change with the libuv patch removed from deps/libuv.ts: the now-live handle makes libuv's post-callback check re-queue the endgame, and the first query aborts in libuv:

[uv] enqueue deferred destroy poll=0x2be007e0540
[uv] poll_cb exit poll=0x2be007e0540
Assertion failed: !(handle->flags & UV_HANDLE_CLOSED), file ..\..\vendor\libuv\src\win\poll.c, line 623

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e70e163b-ccb9-4d35-8892-71126d272111

📥 Commits

Reviewing files that changed from the base of the PR and between bdb7382 and 6d3c2c3.

📒 Files selected for processing (4)

• patches/libuv/win-poll-no-reendgame-after-close.patch
• scripts/build/deps/libuv.ts
• src/runtime/dns_jsc/dns.rs
• test/js/node/dns/node-dns.test.js

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 11:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 6d3c2c3 has 1 failures in Build #94023 (All Failures):

• test/cli/run/require-cache.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 38024

That installs a local version of the PR into your bun-38024 executable, so you can run:

bun-38024 --bun

[robobun]

Status: fix and test verified on Windows x64 locally and on both Windows CI lanes; review in progress.

• Reproduced on the unfixed canary (1.4.0, Windows x64) with the new test's child script: panic(main thread): Segmentation fault by the 9th or 10th query in 10/10 runs; the test itself fails with child exit code 3 under USE_SYSTEM_BUN=1.
• On this branch (bun bd debug build on the same machine) the test passes, and the rest of node-dns.test.js, the test/js/{node,bun}/dns directories, ten node test-dns-* tests with local servers, and the socket/udp/worker suites that exercise uv_poll_t are green; details and the instrumented traces are in the PR description.
• CI (build 94023): the test is Windows-only, and node-dns.test.js ran and passed on every windows 2019 x64 and windows 11 aarch64 test shard (release builds). The one red job is test/cli/run/require-cache.test.ts timing out on the Linux x64 ASAN lane, which this Windows-only diff does not touch; it has been reported separately.
• The libuv patch is the same file as in Fix Windows heap corruption when a socket's data callback closes it and re-enters the event loop #33018; whichever PR lands second drops it on rebase.

[github-actions]

This PR may be a duplicate of:

1. Fix Windows heap corruption when a socket's data callback closes it and re-enters the event loop #33018 - Adds the byte-identical patches/libuv/win-poll-no-reendgame-after-close.patch and the same scripts/build/deps/libuv.ts registration to fix the same libuv defect (endgame re-queued after a nested uv_run already closed the handle), differing only in which call site it repairs (uSockets poll vs. the c-ares DNS poll).

🤖 Generated with Claude Code

[robobun]

Not a duplicate of #33018, but deliberately overlapping with it. #33018 repairs the uSockets call site (us_poll_free / close_cb_free_poll); this PR repairs the c-ares call site in dns.rs, which #33018's description and review list as a separate follow-up. The two call sites have independent Rust/C changes and independent tests.

The libuv patch file and its deps/libuv.ts registration are the one shared piece: both fixes need it, and each PR has to build and pass on its own, so both carry it. It is byte-identical in the two branches, so whichever lands second loses that part on rebase with no conflict.

[claude]

I reviewed this PR and the automated bug-hunt found no issues. Because it's a native memory-safety fix that reasons about nested uv_run re-entry and adds a libuv vendor patch affecting every Windows uv_poll_t, a human look would still be worthwhile.

What was reviewed:

• poll_cb_depth/closed bookkeeping in on_dns_poll_uv/on_close_uv — the depth counter brackets the entire scope including the _exit microtask drain, and only the outermost frame enqueues the deferred destroy.
• The libuv patch's UV_HANDLE_CLOSED guard — matches the assertion uv__poll_endgame already makes; applied to both fast and slow poll paths.
• ManagedTask::new_owned deferral — confirmed the task queue drains outside the libuv callback and frees on VM teardown.
• The new test — hermetic (local UDP responder, port: 0), asserts the re-entry actually happened synchronously, uses distinct names to avoid the c-ares cache.

Extended reasoning...

Overview

This PR fixes a Windows-only use-after-free / double-free in the c-ares DNS resolver. Each c-ares socket is backed by a heap-allocated UvDnsPoll containing a uv_poll_t. When c-ares closes a socket from inside its own poll callback and JS then re-enters the event loop (nested uv_run) from the microtask drain, libuv's endgame phase runs on_close_uv and frees the struct while uv__fast_poll_process_poll_req's frame for that handle is still suspended underneath — libuv then reads the freed handle after the callback returns and re-queues its endgame, double-freeing it.

The fix has two parts: (1) UvDnsPoll gains a poll_cb_depth counter and a closed flag so on_close_uv defers the free to a ManagedTask when a poll-callback frame is still active; (2) a new libuv patch adds a !(flags & UV_HANDLE_CLOSED) guard to the post-poll_cb endgame check in both the fast and slow Windows poll paths so that once the struct stays alive, libuv doesn't re-queue an endgame that already ran. A Windows-only regression test spawns a child that answers 20 resolve4() queries from a local UDP server and re-enters the loop from each promise reaction via Bun.build() with a pending setup().

Security risks

None identified. This is a crash/UAF fix; no new attack surface, no untrusted-input parsing changes. The libuv patch only narrows a condition (adds a guard that uv__poll_endgame already asserts).

Level of scrutiny

High. This is squarely in REVIEW.md's most-blocked category — native memory safety. It reasons about pointer lifetime across nested uv_run re-entry (which libuv's contract does not support), adds a vendor patch that touches every Windows uv_poll_t (not just DNS), and the failure mode is release-only (debug mimalloc's 0xDF fill masks it). The reasoning in the PR description is exceptionally detailed and includes instrumented traces for all four scenarios (unfixed, fixed-with-reentry, fixed-without-reentry, fixed-without-libuv-patch), which builds substantial confidence — but that same depth is what makes it inappropriate for auto-approval.

Other factors

• I traced the new control flow: poll_cb_depth is incremented before any work and decremented after the inner scope (which drops _exit → drains microtasks → may nest uv_run), so the counter is non-zero throughout any nested endgame. Only the outermost frame (depth == 0 && closed) enqueues the deferred destroy, so re-entrant depth-2 dispatch cannot double-enqueue. on_close_uv sets closed at most once (debug_assert!(!closed), and the libuv patch prevents the second invocation).
• vm is captured before the inner scope, so it remains valid for destroy_after_poll_cb_returns even after _deref may free the resolver — vm is the VirtualMachine, not the resolver.
• The libuv patch is described as byte-identical to #33018's; the deps/libuv.ts comment correctly notes it is not upstreamable (nested uv_run is outside libuv's contract).
• The test is well-constructed: hermetic local UDP responder, distinct hostnames per round to force network hits, and it self-checks that Bun.build() actually spun the loop synchronously (throws if reentries didn't increment before Bun.build() returned), so the test cannot silently degrade into a no-op if that behavior changes.
• No prior reviews on this PR; CodeRabbit was rate-limited. Given the vendor patch and the subtlety of the lifetime argument, a maintainer familiar with the Windows event loop should sign off.