worker(windows): free the uWS loop wrapper at thread exit

[robobun]

Problem

On Windows, every Worker thread that exits leaks its uWS loop wrapper: the heap-allocated us_loop_t, its LoopData (two 16 KiB cork buffers), the 512 KiB recv_buf, the 16 KiB send_buf, and the uv_prepare_t/uv_check_t/sweep-timer/wakeup-async allocations. Roughly 560 KiB per worker.

LEAK: RSS grew 88.79 MiB over 200 worker cycles

Cause

WindowsLoop::get() calls uws_get_loop_with_native(uv::Loop::get()), passing the per-thread uv_loop_t as a non-null hint. uWS::Loop::get(existingNativeLoop) leaves cleanMe = false whenever a hint is supplied. bun_uws::on_thread_exit() calls clearLoopAtThreadExit(), which gated on cleanMe, so on Windows it returned without freeing. bun_sys::windows::libuv::Loop::shutdown() closes the uv_loop_t but never touches the us_loop_t wrapper that owns the buffers.

On POSIX no hint is passed (uws_get_loop() takes no argument), cleanMe is set, and on_thread_exit() already freed correctly.

Fix

clearLoopAtThreadExit() no longer gates on cleanMe: the "don't free a borrowed native loop" concern it was guarding is already handled one layer down by us_loop_free (which branches on loop->is_default and skips uv_run/uv_loop_delete for a borrowed uv_loop_t while still releasing the wrapper and its buffers). cleanMe continues to govern only the ~LoopCleaner TLS-destructor path. On POSIX behaviour is unchanged (loop non-null iff cleanMe was true there); on Windows on_thread_exit() now frees.

WebWorker on Windows calls on_thread_exit() immediately before Loop::shutdown():

• us_loop_free queues uv_close(..., close_cb_free) for uv_pre/uv_check/the sweep timer/the wakeup async; Loop::shutdown()'s uv_run flushes those callbacks so their storage is freed too. Doing it after shutdown() would uv_close() already-closed handles.
• vm.destroy() after this point does not dereference the freed wrapper: RareData::drop's us_socket_group_deinit only touches group->loop when group->linked is still set, and every group was emptied (and so unlinked via us_internal_group_maybe_unlink) by close_all_socket_groups + the JSC-finalizer listener close + drain_closed_sockets in steps 2/3 of shutdown().

The POSIX on_thread_exit() call stays where it was and is gated to #[cfg(not(windows))].

Separately, set_enable_keeping_event_loop_alive on Windows computed (*vm).uws_loop() (which is WindowsLoop::get()) only to discard it (increment_*_ref uses the by-value uv_timer/uv_idle in RuntimeState on Windows and does let _ = uws_loop). After Loop::shutdown() both thread-locals are null, so the pending-setImmediate drain in event_loop.deinit() would lazily re-create the wrapper + uv_loop_t and leak them. That call now passes null on Windows.

#35060 will be able to call on_thread_exit() on its Windows overflow threads instead of adding the separate freeLoopWrapperAtThreadExit() helper; whichever lands second has a trivial merge in Loop.h.

Verification

New test/js/web/workers/worker-windows-loop-leak.test.ts spins up and terminates 250 workers on Windows in two variants (minimal body, and one with a pending setImmediate) and checks that post-warmup RSS growth stays under 15 MiB (25 MiB on debug/ASAN). On main:

• release, minimal: grows 88.79 MiB (fails)
• debug, minimal: grows 38.49 MiB (fails)
• debug, pending setImmediate: grows >100 MiB (fails)

With this change all three plateau under ~5 MiB past the warmup and pass.

worker.test.ts, worker-terminate-lifetime.test.ts, and worker_threads.test.ts pass on Windows debug with the fix. Workers that run Bun.serve + fetch, exit via process.exit(), or exit naturally all terminate cleanly. rust:check-all passes on all 10 targets.

[coderabbitai]

Walkthrough

Changes

Windows worker loop cleanup

Layer / File(s)	Summary
Loop wrapper cleanup bridge packages/bun-uws/src/Loop.h, src/uws_sys/Loop.rs	Thread-exit cleanup now frees an existing uWS loop wrapper, with updated FFI and ordering documentation.
Windows worker teardown src/jsc/web_worker.rs, src/runtime/timer/timer_object_internals.rs	Windows worker shutdown performs uWS cleanup before libuv shutdown, while Windows timer reference updates pass a null loop pointer.
Windows loop leak regression test test/js/web/workers/worker-windows-loop-leak.test.ts	A Windows-only subprocess test measures RSS growth across repeated worker lifecycles in two scenarios.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately describes the main Windows worker cleanup change.
Description check	✅ Passed	It covers the bug, cause, fix, and verification, though it uses custom headings instead of the template sections.

---

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

[robobun]

^{Updated 4:18 AM PT - Jul 22nd, 2026}

✅ @robobun, your commit 162d7e2b283fa1859fa7fabd36805d44a37846b1 passed in Build #77622! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 35090

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

bun-35090 --bun

[github-actions]

This PR may be a duplicate of:

1. bundler: run concurrent Bun.build() calls on overflow threads #35060 - Both PRs independently add the same freeLoopWrapperAtThreadExit() helper in Loop.h and corresponding Rust FFI bindings to fix the uWS loop wrapper leak on thread exit

🤖 Generated with Claude Code

[robobun]

Not a duplicate. #35060 adds freeLoopWrapperAtThreadExit() for its bundler overflow threads; this PR adds the same helper to fix the Windows Worker leak (~560 KiB per worker). The helper addition is identical so whichever merges second has a trivial merge in Loop.h / libuwsockets.cpp / Loop.rs; the web_worker.rs change and the test here are not in #35060.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/web/workers/worker-windows-loop-leak.test.ts`:
- Around line 24-29: Update the cycle function so both awaited worker-event
promises reject when the Worker emits an error, rather than waiting for the
timeout. Add error listeners alongside the existing message and close listeners,
and ensure the worker is terminated during cleanup whether the cycle succeeds or
fails.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 04bff7db-ff68-4055-9056-f0c4e33081c7

📥 Commits

Reviewing files that changed from the base of the PR and between 3c77c5a and 041d94e.

📒 Files selected for processing (6)

• packages/bun-uws/src/Loop.h
• src/jsc/web_worker.rs
• src/uws/lib.rs
• src/uws_sys/Loop.rs
• src/uws_sys/libuwsockets.cpp
• test/js/web/workers/worker-windows-loop-leak.test.ts

[claude]

I didn't find any bugs, but this reorders memory-safety-critical Windows worker teardown (the us_loop_t wrapper is now freed before vm.destroy() runs), so it's worth a human look and a green Windows CI run before landing.

What was reviewed:

• us_loop_free (libuv.c) queues uv_close on uv_pre/uv_check/sweep-timer/wakeup-async then us_free(loop); Loop::shutdown()'s close_walk_cb guards uv_is_closing and its uv_run(Default) flushes the queued close_cb_frees — ordering claim holds.
• us_socket_group_deinit only dereferences group->loop when group->linked; us_internal_group_maybe_unlink clears linked when empty — the RareData-drop-is-safe claim depends on every group being emptied by steps 2/3, which the existing shutdown sequence does.
• freeLoopWrapperAtThreadExit() nulls getLazyLoop().loop via free() and clears cleanMe, so ~LoopCleaner cannot double-free.

Extended reasoning...

Overview

Fixes a Windows-only ~560 KiB/worker leak: WindowsLoop::get() passes a non-null uv hint so cleanMe stays false and clearLoopAtThreadExit() no-ops. The PR adds uWS::Loop::freeLoopWrapperAtThreadExit() (frees regardless of cleanMe), a C-ABI shim, a Rust FFI binding + re-export, calls it in WebWorker::shutdown() on Windows immediately before bun_sys::windows::libuv::Loop::shutdown(), gates the pre-existing on_thread_exit() to #[cfg(not(windows))], and adds a Windows-only RSS leak regression test.

Security risks

None. No user-controlled input, no auth/crypto/permissions surface. Pure resource-teardown ordering.

Level of scrutiny

High. Per REVIEW.md, native memory safety is the most-blocked category. This changes teardown ordering so that us_free(loop) runs before vm.destroy() (whose RareData::drop calls us_socket_group_deinit on embedded groups). Correctness depends on every group having linked == 0 by then — I traced us_socket_group_deinit (context.c:70) and confirmed it only derefs group->loop when linked is set, and us_internal_group_maybe_unlink clears it once all list heads/counts are zero. The PR's comment attributes emptiness to close_all_socket_groups + JSC-finalizer listener close + drain_closed_sockets earlier in shutdown(). That reasoning is sound for the paths I can see, but if any future (or edge-case current) path leaves a group non-empty here, this converts a leak into a UAF — worth a maintainer's eye.

I also verified the second ordering claim: us_loop_free (libuv.c:297-319) queues uv_close(..., close_cb_free) on uv_pre/uv_check and (via us_internal_loop_data_free) closes sweep_timer/wakeup_async, then frees the wrapper. Loop::shutdown() (libuv.rs:432) hits EBUSY on uv_loop_close, walks with close_walk_cb (which checks uv_is_closing so no double-close), and uv_run(Default) flushes the queued close_cb_frees. So calling the free before shutdown() is correct and necessary.

Other factors

• CI was still building at review time; Windows-debug worker suites should be confirmed green.
• Shares the freeLoopWrapperAtThreadExit() helper with #35060 (trivial merge whichever lands second).
• The prior stderr-assertion nit was answered reasonably (Windows-only, bunEnv quiets debug logs, matches sibling tests) and is not blocking.
• The leak test follows the neighbouring message-port-context-destroy-leak.test.ts pattern, branches its threshold on isASAN || isDebug, and the PR reports fails-on-main / passes-with-fix on both release and debug.

[claude]

I didn't find any bugs, but this touches worker-thread teardown ordering across the uWS/libuv FFI boundary and modifies vendored Loop.h semantics for all callers, so it's worth a human look.

What was reviewed:

• clearLoopAtThreadExit gate change: free() nulls getLazyLoop().loop and cleanMe is cleared first, so ~LoopCleaner cannot double-free; POSIX behavior unchanged (loop non-null ⇔ cleanMe was true).
• Windows ordering in shutdown(): the (*loop_).internal_loop_data.jsc_vm = null write precedes on_thread_exit(), and us_loop_free (libuv.c:297) queues uv_close callbacks that Loop::shutdown()'s uv_run flushes; vm.destroy() after the free reaches RareData::drop only on already-unlinked groups.
• timer_object_internals.rs null-on-Windows: verified increment_timer_ref/increment_immediate_ref both let _ = uws_loop under cfg(windows) (mod.rs:1126/1168), so passing null is sound and prevents the lazy re-create leak the second test targets.

Extended reasoning...

Overview

Fixes a ~560 KiB/worker leak on Windows where the uWS loop wrapper (us_loop_t + LoopData + 512 KiB recv_buf + send/cork buffers + uv handle allocs) was never freed because uWS::Loop::get(hint) leaves cleanMe = false when a native-loop hint is passed (which WindowsLoop::get() always does), and the existing clearLoopAtThreadExit() gated on cleanMe. Five files touched: the vendored Loop.h (drops the cleanMe gate), web_worker.rs (calls on_thread_exit() before Loop::shutdown() on Windows, gates the existing POSIX call), timer_object_internals.rs (avoids lazily re-creating the wrapper via uws_loop() during the post-shutdown immediate drain), Loop.rs (doc-comment update), and a new Windows-only RSS regression test.

Note the PR description is slightly stale relative to the head commit: 162d7e2 simplified from adding a separate freeLoopWrapperAtThreadExit() to just dropping the gate in the existing clearLoopAtThreadExit().

Security risks

None identified. This is resource-cleanup ordering; no untrusted input, auth, or crypto paths.

Level of scrutiny

High. Per REVIEW.md this is the most-blocked category (native memory safety, cross-thread lifetime, vendored dep). The correctness argument spans four ordering constraints: (1) free the wrapper before Loop::shutdown() so uv_close callbacks flush rather than target already-closed handles, (2) the jsc_vm = null write and drain_closed_sockets precede the free, (3) RareData::drop in vm.destroy() after the free only touches groups already unlinked by close_all_socket_groups + finalizer close + drain_closed_sockets, (4) the pending-immediate drain in event_loop.deinit() must not call WindowsLoop::get() and re-leak. Each is documented in-code and I traced the supporting facts (us_loop_free in libuv.c:297-320, increment_*_ref cfg-gating in timer/mod.rs:1094-1169, uws_loop() → WindowsLoop::get() on non-unix in VirtualMachine.rs:1009-1012), but the interaction with us_socket_group_deinit / group->linked in RareData::drop and whether every path into vm.destroy() on Windows is now safe from lazy re-create (dns.rs and subprocess.rs also call increment_timer_ref with a live uws_loop(), though those run before Loop::shutdown()) is the kind of thing a maintainer with the full teardown model in their head should sign off on.

Other factors

• The Loop.h change is a semantic change to a shared entry point rather than a Windows-only additive helper — behavior on POSIX is unchanged in practice (loop non-null iff cleanMe was true there), but it's a vendored file the sibling PR #35060 also touches.
• Test looks solid: Windows-gated, subprocess-isolated, RSS threshold branched on isDebug/isASAN well below the unfixed 38-88 MiB, warmup+run split for mimalloc reclaim noise, second case covers the setImmediate re-leak. My earlier stderr nit was resolved (matches sibling tests + bunEnv).
• PR reports rust:check-all and the worker test suite pass on Windows debug.