From 252a3f511b73cab1972c1102b05511e1fc82185e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:19:57 +0000 Subject: [PATCH 01/14] spawnSync: release polls and keep-alives on the loop they counted on Bun.spawnSync points vm.event_loop_handle at its isolated uws loop while it blocks. A GC finalizer that runs in that window tears down polls and keep-alives that count on the thread's loop, and every release resolved the loop through the current handle, so the isolated loop's num_polls and active counters drifted. The next spawnSync then started with a negative count, reached zero while its own polls were live, and us_loop_run_bun_tick returned without polling until the timeout. FilePoll, KeepAlive, and the two timer ref counts now record one bit when they take a counter: whether the current loop was the isolated spawnSync loop. A release resolves the loop from that bit: the isolated loop (still the current loop for as long as such an owner lives) or the thread's loop. FilePoll registration and release take the EventLoopCtx instead of a resolved loop so the poll picks the loop itself. A debug assert in SpawnSyncEventLoop::prepare checks that the isolated loop's counters are zero before each spawn. BUN_INTERNAL_SPAWN_SYNC_GC (debug builds) forces a GC inside spawnSync so the new test can exercise finalizers under the isolated loop. PosixStreamingWriterParent::loop_ lost its last caller and is removed. --- src/bun_core/env_var.rs | 2 + src/event_loop/SpawnSyncEventLoop.rs | 7 ++ src/io/ParentDeathWatchdog.rs | 9 +- src/io/PipeReader.rs | 6 +- src/io/PipeWriter.rs | 30 ++----- src/io/keep_alive.rs | 12 ++- src/io/lib.rs | 86 ++++++++----------- src/io/posix_event_loop.rs | 77 ++++++++++------- src/runtime/api/bun/Terminal.rs | 3 - src/runtime/api/bun/js_bun_spawn_bindings.rs | 7 ++ src/runtime/dns_jsc/dns.rs | 12 ++- src/runtime/dns_jsc/dns_sd.rs | 4 +- src/runtime/node/memory_pressure.rs | 6 +- src/runtime/socket/WindowsNamedPipe.rs | 1 - src/runtime/timer/mod.rs | 53 ++++++++++-- src/runtime/webcore/FileSink.rs | 1 - src/spawn/process.rs | 20 +---- .../spawnsync-isolated-event-loop.test.ts | 59 ++++++++++++- 18 files changed, 233 insertions(+), 162 deletions(-) diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index 5295b888775d..d9e5d82f3bfb 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -274,6 +274,8 @@ pub mod feature_flag { new_feature_flag!(pub BUN_INTERNAL_BUNX_INSTALL, "BUN_INTERNAL_BUNX_INSTALL", {}); // Debug-only fault injection for test/js/bun/spawn/spawn-pipe-start-error.test.ts. new_feature_flag!(pub BUN_INTERNAL_FAIL_PIPE_READER_START, "BUN_INTERNAL_FAIL_PIPE_READER_START", {}); + // Debug-only fault injection for test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts. + new_feature_flag!(pub BUN_INTERNAL_SPAWN_SYNC_GC, "BUN_INTERNAL_SPAWN_SYNC_GC", {}); // Test-only: bypass the stdin isatty gate in `bun update --interactive` so // tests can drive the multi-select by writing keystrokes to a pipe. new_feature_flag!(pub BUN_INTERNAL_INTERACTIVE_ASSUME_TTY, "BUN_INTERNAL_INTERACTIVE_ASSUME_TTY", {}); diff --git a/src/event_loop/SpawnSyncEventLoop.rs b/src/event_loop/SpawnSyncEventLoop.rs index e3406117c593..808cc2733e8c 100644 --- a/src/event_loop/SpawnSyncEventLoop.rs +++ b/src/event_loop/SpawnSyncEventLoop.rs @@ -284,6 +284,13 @@ impl Drop for SpawnSyncEventLoop { impl SpawnSyncEventLoop { /// Configure the event loop for a specific VM context pub fn prepare(&mut self, vm: *mut () /* SAFETY: erased *mut VirtualMachine */) { + #[cfg(unix)] + debug_assert!( + self.uws_loop().num_polls == 0 && self.uws_loop().active == 0, + "spawnSync isolated loop has num_polls={} active={} before a spawn", + self.uws_loop().num_polls, + self.uws_loop().active, + ); __bun_spawn_sync_event_loop_set_vm(self.event_loop, vm); self.did_timeout.set(false); self.vm = vm; diff --git a/src/io/ParentDeathWatchdog.rs b/src/io/ParentDeathWatchdog.rs index fe05cc8c709a..ba93867e02da 100644 --- a/src/io/ParentDeathWatchdog.rs +++ b/src/io/ParentDeathWatchdog.rs @@ -326,13 +326,8 @@ pub fn install_on_event_loop(handle: EventLoopCtx) { Default::default(), Owner::new(poll_tag::PARENT_DEATH_WATCHDOG, instance_ptr.cast()), ); - // SAFETY: `poll` was just allocated by `FilePoll::init`; sole `&mut` - // borrow; `register` does not re-derive the loop. - match unsafe { &mut *poll }.register( - handle.loop_mut(), - crate::file_poll::Pollable::Process, - true, - ) { + // SAFETY: `poll` was just allocated by `FilePoll::init`; sole `&mut` borrow. + match unsafe { &mut *poll }.register(handle, crate::file_poll::Pollable::Process, true) { bun_sys::Result::Ok(()) => { // Do not keep the event loop alive on this poll's behalf — the // watchdog must never prevent Bun from exiting when there is no diff --git a/src/io/PipeReader.rs b/src/io/PipeReader.rs index 038dcc8254ae..3453c3747f56 100644 --- a/src/io/PipeReader.rs +++ b/src/io/PipeReader.rs @@ -110,6 +110,7 @@ impl BufferedReaderVTable { self.link().event_loop() } + #[cfg(windows)] fn loop_(&self) -> *mut Loop { self.link().loop_ptr() } @@ -352,7 +353,7 @@ impl PosixBufferedReader { // Unregister the FilePoll if it's registered if let PollOrFd::Poll(poll) = &mut self.handle { if poll.is_registered() { - let _ = poll.unregister(self.vtable.loop_().cast(), false); + let _ = poll.unregister(self.vtable.event_loop(), false); } } } @@ -513,7 +514,6 @@ impl PosixBufferedReader { // normalize self.handle to Poll before taking the single &mut borrow, // so no raw-pointer escape is needed. let ev = self.vtable.event_loop(); - let lp = self.vtable.loop_(); let owner_ptr = std::ptr::from_mut(self).cast::(); if let PollOrFd::Fd(fd) = self.handle { @@ -537,7 +537,7 @@ impl PosixBufferedReader { poll.enable_keeping_process_alive(ev); } - match poll.register_with_fd(lp.cast(), FilePollKind::Readable, poll.fd()) { + match poll.register_with_fd(ev, FilePollKind::Readable, poll.fd()) { sys::Result::Err(err) => Err(err), sys::Result::Ok(()) => Ok(()), } diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index d11dddf016c0..75880fef3ea3 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -425,9 +425,7 @@ impl PosixBufferedWriter { pub fn register_poll(&mut self) { let Some(poll) = self.get_poll() else { return }; - // Use the event loop from the parent, not the global one - let loop_ = self.parent_event_loop().loop_(); - match poll.register_with_fd(loop_, FilePollKind::Writable, poll.fd()) { + match poll.register_with_fd(self.parent_event_loop(), FilePollKind::Writable, poll.fd()) { sys::Result::Err(err) => { // Same report as a failed write (the streaming writer does the // same): parents expect every error to be followed by `on_close`. @@ -534,9 +532,7 @@ impl PosixBufferedWriter { p } }; - let loop_ = self.parent_event_loop().loop_(); - - match poll.register_with_fd(loop_, FilePollKind::Writable, fd) { + match poll.register_with_fd(self.parent_event_loop(), FilePollKind::Writable, fd) { sys::Result::Err(err) => { return sys::Result::Err(err); } @@ -581,9 +577,6 @@ pub trait PosixStreamingWriterParent { /// # Safety /// `this` must point to a live `Self`. unsafe fn event_loop(this: *mut Self) -> EventLoopHandle; - /// # Safety - /// `this` must point to a live `Self`. - unsafe fn loop_(this: *mut Self) -> *mut bun_uws_sys::Loop; } pub struct PosixStreamingWriter { @@ -772,8 +765,8 @@ impl PosixStreamingWriter { fn register_poll(&mut self) { let Some(poll) = self.get_poll() else { return }; // SAFETY: parent BACKREF set via set_parent; outlives this writer. - let loop_ = unsafe { Parent::loop_(self.parent()) }.cast(); - match poll.register_with_fd(loop_, FilePollKind::Writable, poll.fd()) { + let event_loop = unsafe { Parent::event_loop(self.parent()) }; + match poll.register_with_fd(event_loop, FilePollKind::Writable, poll.fd()) { sys::Result::Err(err) => { // PORT_NOTES_PLAN R-2: `&mut self` carries LLVM `noalias`, but // `Parent::on_error` (e.g. `FileSink::on_error`) re-enters via @@ -1042,12 +1035,12 @@ impl PosixStreamingWriter { } // SAFETY: parent BACKREF set via set_parent; outlives this writer. - let loop_ = unsafe { Parent::event_loop(self.parent()) }; + let event_loop = unsafe { Parent::event_loop(self.parent()) }; let poll = match self.get_poll() { Some(p) => p, None => { let p = FilePollRef::init( - loop_, + event_loop, fd, Owner::new(Parent::POLL_OWNER_TAG, std::ptr::from_mut(self).cast()), ); @@ -1056,7 +1049,7 @@ impl PosixStreamingWriter { } }; - match poll.register_with_fd(loop_.loop_(), FilePollKind::Writable, fd) { + match poll.register_with_fd(event_loop, FilePollKind::Writable, fd) { sys::Result::Err(err) => { return sys::Result::Err(err); } @@ -2587,7 +2580,6 @@ pub mod __parent_macro { pub use ::bun_sys::Error as SysError; #[cfg(windows)] pub use ::bun_sys::windows::libuv::Loop as UvLoop; - pub use ::bun_uws_sys::Loop as UwsLoop; } /// Stamp `PosixStreamingWriterParent` + `WindowsWriterParent` + @@ -2609,7 +2601,6 @@ macro_rules! impl_streaming_writer_parent { on_ready = $on_ready:ident, on_close = $on_close:ident, event_loop = |$el_this:ident| $el:expr, - uws_loop = |$uws_this:ident| $uws:expr, uv_loop = |$uv_this:ident| $uv:expr, ref_ = |$ref_this:ident| $ref_:expr, deref = |$deref_this:ident| $deref:expr, @@ -2649,13 +2640,6 @@ macro_rules! impl_streaming_writer_parent { #[allow(unused_unsafe)] unsafe { $el } } - #[inline] - unsafe fn loop_(this: *mut Self) -> *mut $crate::pipe_writer::__parent_macro::UwsLoop { - // SAFETY: see on_write. Shared-only read. - let $uws_this = this; - #[allow(unused_unsafe)] - unsafe { $uws } - } } #[cfg(windows)] diff --git a/src/io/keep_alive.rs b/src/io/keep_alive.rs index b45fbb8a007b..0bb55aa2328b 100644 --- a/src/io/keep_alive.rs +++ b/src/io/keep_alive.rs @@ -13,6 +13,8 @@ use crate::posix_event_loop::js_vm_ctx; #[derive(Default)] pub struct KeepAlive { status: Status, + /// `ref_` counted on `Bun.spawnSync`'s isolated loop, not the thread's loop. + spawn_sync_loop: bool, } #[derive(Copy, Clone, PartialEq, Eq, Default)] @@ -45,10 +47,11 @@ impl KeepAlive { return; } self.status = Status::Inactive; + let loop_ = event_loop_ctx.loop_for(self.spawn_sync_loop); #[cfg(not(windows))] - event_loop_ctx.loop_unref(); + loop_.unref(); #[cfg(windows)] - event_loop_ctx.loop_sub_active(1); + loop_.sub_active(1); } /// Prevent a poll from keeping the process alive on the next tick. @@ -56,6 +59,10 @@ impl KeepAlive { if self.status != Status::Active { return; } + debug_assert!( + !self.spawn_sync_loop, + "deferred unref of a spawnSync loop ref" + ); self.status = Status::Inactive; // vm.pending_unref_counter +|= 1; #[cfg(not(windows))] @@ -70,6 +77,7 @@ impl KeepAlive { return; } self.status = Status::Active; + self.spawn_sync_loop = event_loop_ctx.is_spawn_sync_loop(); event_loop_ctx.loop_ref(); } diff --git a/src/io/lib.rs b/src/io/lib.rs index c9954cd44b2f..2adecdd9316d 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -331,33 +331,18 @@ bun_dispatch::link_interface! { } impl EventLoopCtx { - /// SAFETY: caller must not hold another live `&mut` to the same loop - /// across this borrow (resolver-style accessor; the loop is per-thread). - #[inline] - pub unsafe fn platform_event_loop(&self) -> &'static mut bun_uws_sys::Loop { - // Route through the single nonnull-asref accessor below; the `unsafe` - // on this fn's signature is the caller-side aliasing contract — the - // body itself needs no extra `unsafe`. - self.loop_mut() - } - // ── safe leaf wrappers (nonnull-asref) ────────────────────────────── // The platform loop / poll store are per-thread, set-once back-pointers - // (`BackRef`-shaped). [`platform_event_loop`] cannot be a safe fn because - // handing out `&mut Loop` from `&self` would let a caller alias two - // copies; these wrappers instead perform one counter adjustment and drop - // the borrow before returning, so no `&mut` escapes. Collapses N - // identical `ctx.platform_event_loop().op()` call sites into the single - // deref inside [`loop_mut`]. + // (`BackRef`-shaped). Handing out `&mut Loop` from `&self` would let a + // caller alias two copies; these wrappers instead perform one counter + // adjustment and drop the borrow before returning, so no `&mut` escapes. // - // `loop_mut` is the single nonnull-asref accessor: `pub(crate)`, + // `loop_mut` is the single nonnull-asref accessor: crate-private, // `&self → &mut` (so it must NOT be called twice with overlapping live // results). Every in-crate caller is a leaf op — counter bump, // `FilePoll::activate`/`deactivate`, `unregister` — that consumes the // borrow before returning and never re-enters `EventLoopCtx`, so no two - // `&mut Loop` ever coexist. Widened from impl-private to crate-private so - // `posix_event_loop`/`windows_event_loop` route their N identical - // `ctx.platform_event_loop()` derefs through this single accessor. + // `&mut Loop` ever coexist. #[inline] fn loop_mut(&self) -> &'static mut bun_uws_sys::Loop { // SAFETY: per-thread set-once pointer (the uws loop singleton); the @@ -387,14 +372,29 @@ impl EventLoopCtx { // SAFETY: per-thread scratch owned by the VM/Mini loop, which outlives every read. unsafe { (*self.pipe_read_scratch()).claim() } } + /// True while `Bun.spawnSync` has its isolated loop installed as the current loop. #[inline] - pub(crate) fn loop_ref(&self) { - self.loop_mut().ref_(); + pub(crate) fn is_spawn_sync_loop(&self) -> bool { + self.platform_event_loop_ptr() != bun_uws_sys::Loop::get() } + /// The loop a counter was taken on: the isolated `spawnSync` loop when its + /// owner recorded `spawn_sync_loop` from [`Self::is_spawn_sync_loop`], else + /// the thread's loop, even while `spawnSync` has swapped the current loop. #[inline] - #[cfg(not(windows))] - pub(crate) fn loop_unref(&self) { - self.loop_mut().unref(); + fn loop_for(&self, spawn_sync_loop: bool) -> &'static mut bun_uws_sys::Loop { + if spawn_sync_loop { + debug_assert!( + self.is_spawn_sync_loop(), + "a spawnSync loop counter is released after spawnSync returned" + ); + return self.loop_mut(); + } + // SAFETY: the thread's loop; same contract as `loop_mut`. + unsafe { &mut *bun_uws_sys::Loop::get() } + } + #[inline] + pub(crate) fn loop_ref(&self) { + self.loop_mut().ref_(); } #[inline] #[cfg(windows)] @@ -402,10 +402,12 @@ impl EventLoopCtx { self.loop_mut().dec(); } #[inline] + #[cfg(windows)] pub(crate) fn loop_add_active(&self, n: u32) { self.loop_mut().add_active(n); } #[inline] + #[cfg(windows)] pub(crate) fn loop_sub_active(&self, n: u32) { self.loop_mut().sub_active(n); } @@ -1831,41 +1833,25 @@ impl FilePollRef { pub(crate) fn deinit_force_unregister(self) { self.inner().deinit_force_unregister(); } - /// Single nonnull-asref accessor for the process-global uWS loop pointer. - /// - /// Type invariant (encapsulated `unsafe`): every caller of - /// [`unregister`](Self::unregister) / [`register_with_fd`](Self::register_with_fd) - /// passes `Loop::get()` (the per-thread uWS loop singleton), which is - /// non-null after init and lives for the program. The event loop is - /// single-threaded so the returned `&mut` is the sole live borrow at the - /// point of use. Collapses the two identical `&mut *loop_` deref blocks in - /// those wrappers into one. - #[inline(always)] - fn uws_loop_mut<'a>(loop_: *mut bun_uws_sys::Loop) -> &'a mut bun_uws_sys::Loop { - debug_assert!(!loop_.is_null()); - // SAFETY: type invariant — see doc comment above. - unsafe { &mut *loop_ } - } #[inline] - pub(crate) fn unregister(self, loop_: *mut bun_uws_sys::Loop, force: bool) -> sys::Result<()> { - let loop_ = Self::uws_loop_mut(loop_); + pub(crate) fn unregister(self, ev: EventLoopHandle, force: bool) -> sys::Result<()> { #[cfg(not(windows))] { - self.inner().unregister(loop_, force) + self.inner().unregister(ev, force) } #[cfg(windows)] { let _ = force; // The windows `FilePoll::unregister` always returns true — the // bool is vestigial, so there is no error path to surface here. - let _ = self.inner().unregister(loop_); + let _ = self.inner().unregister(ev.loop_mut()); Ok(()) } } #[inline] pub(crate) fn register_with_fd( self, - loop_: *mut bun_uws_sys::Loop, + ev: EventLoopHandle, kind: FilePollKind, fd: Fd, ) -> sys::Result<()> { @@ -1875,16 +1861,12 @@ impl FilePollRef { }; #[cfg(not(windows))] { - self.inner().register_with_fd( - Self::uws_loop_mut(loop_), - flag, - OneShotFlag::Dispatch, - fd, - ) + self.inner() + .register_with_fd(ev, flag, OneShotFlag::Dispatch, fd) } #[cfg(windows)] { - let _ = (loop_, flag, fd); + let _ = (ev, flag, fd); unreachable!("FilePoll fd registration is POSIX-only"); } } diff --git a/src/io/posix_event_loop.rs b/src/io/posix_event_loop.rs index 1d9cfc2782be..861626672330 100644 --- a/src/io/posix_event_loop.rs +++ b/src/io/posix_event_loop.rs @@ -331,7 +331,7 @@ impl FilePoll { // Note: these handlers take no loop parameter: holding a // protected `&mut Loop` across `on_update` would alias the fresh `&mut Loop` // that downstream `__bun_run_file_poll` handlers conjure via - // `EventLoopCtx::platform_event_loop()` when they re-enter the loop + // `EventLoopCtx::loop_mut()` when they re-enter the loop // (`register_with_fd`/`unregister`/`deinit`). #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub(crate) fn on_kqueue_event(&mut self, kqueue_event: &KQueueEvent) { @@ -383,10 +383,7 @@ impl FilePoll { } fn deinit_possibly_defer(&mut self, vm: EventLoopCtx, force_unregister: bool) { - // `loop_mut()` is the crate-private nonnull-asref accessor (single - // deref in `EventLoopCtx`); the `&mut Loop` is consumed by `unregister` - // and dropped before any `&mut Store` is materialised. - let _ = self.unregister(vm.loop_mut(), force_unregister); + let _ = self.unregister(vm, force_unregister); self.owner.clear(); let was_ever_registered = self.flags.contains(Flags::WasEverRegistered); @@ -445,11 +442,19 @@ impl FilePoll { || self.flags.contains(Flags::PollProcess)) } + /// The loop this poll is registered with, not the loop `vm` currently points at. + #[inline] + fn loop_mut(&self, vm: EventLoopCtx) -> &'static mut Loop { + vm.loop_for(self.flags.contains(Flags::SpawnSyncLoop)) + } + /// This decrements the active counter if it was previously incremented /// "active" controls whether or not the event loop should potentially idle pub fn disable_keeping_process_alive(&mut self, event_loop_ctx: EventLoopCtx) { - event_loop_ctx - .loop_sub_active(self.flags.contains(Flags::HasIncrementedActiveCount) as u32); + loop_sub_active( + self.loop_mut(event_loop_ctx), + self.flags.contains(Flags::HasIncrementedActiveCount) as u32, + ); self.flags.remove(Flags::KeepsEventLoopAlive); self.flags.remove(Flags::HasIncrementedActiveCount); @@ -460,15 +465,18 @@ impl FilePoll { return; } - event_loop_ctx - .loop_add_active((!self.flags.contains(Flags::HasIncrementedActiveCount)) as u32); + loop_add_active( + self.loop_mut(event_loop_ctx), + (!self.flags.contains(Flags::HasIncrementedActiveCount)) as u32, + ); self.flags.insert(Flags::KeepsEventLoopAlive); self.flags.insert(Flags::HasIncrementedActiveCount); } /// Only intended to be used from EventLoop.Pollable - fn deactivate(&mut self, loop_: &mut Loop) { + fn deactivate(&mut self, vm: EventLoopCtx) { + let loop_ = self.loop_mut(vm); if self.flags.contains(Flags::HasIncrementedPollCount) { loop_.dec(); } @@ -483,7 +491,8 @@ impl FilePoll { } /// Only intended to be used from EventLoop.Pollable - fn activate(&mut self, loop_: &mut Loop) { + fn activate(&mut self, vm: EventLoopCtx) { + let loop_ = self.loop_mut(vm); self.flags.remove(Flags::Closed); if !self.flags.contains(Flags::HasIncrementedPollCount) { @@ -509,7 +518,10 @@ impl FilePoll { /// non-macOS-debug builds and then read it in the `syslog!` below. Building /// the whole struct by value fixes both. #[inline] - fn new_value(vm: EventLoopCtx, fd: Fd, flags: FlagsSet, owner: Owner) -> FilePoll { + fn new_value(vm: EventLoopCtx, fd: Fd, mut flags: FlagsSet, owner: Owner) -> FilePoll { + if vm.is_spawn_sync_loop() { + flags.insert(Flags::SpawnSyncLoop); + } FilePoll { fd, flags, @@ -540,9 +552,9 @@ impl FilePoll { poll } - pub fn register(&mut self, loop_: &mut Loop, flag: Flags, one_shot: bool) -> sys::Result<()> { + pub fn register(&mut self, vm: EventLoopCtx, flag: Flags, one_shot: bool) -> sys::Result<()> { self.register_with_fd( - loop_, + vm, flag, if one_shot { OneShotFlag::OneShot @@ -555,7 +567,7 @@ impl FilePoll { pub fn register_with_fd( &mut self, - loop_: &mut Loop, + vm: EventLoopCtx, flag: Flags, one_shot: OneShotFlag, fd: Fd, @@ -566,7 +578,7 @@ impl FilePoll { target_os = "macos", target_os = "freebsd" ))] - return self.register_with_fd_impl(loop_, flag, one_shot, fd); + return self.register_with_fd_impl(vm, flag, one_shot, fd); #[cfg(not(any( target_os = "linux", target_os = "android", @@ -574,7 +586,7 @@ impl FilePoll { target_os = "freebsd" )))] { - let _ = (loop_, flag, one_shot, fd); + let _ = (vm, flag, one_shot, fd); sys::Result::Ok(()) } } @@ -587,12 +599,12 @@ impl FilePoll { ))] fn register_with_fd_impl( &mut self, - loop_: &mut Loop, + vm: EventLoopCtx, flag: Flags, one_shot: OneShotFlag, fd: Fd, ) -> sys::Result<()> { - let watcher_fd = loop_.fd; + let watcher_fd = self.loop_mut(vm).fd; syslog!( "register: FilePoll(0x{:x}, generation_number={}) {} ({})", @@ -653,7 +665,7 @@ impl FilePoll { let ctl = unsafe { linux::epoll_ctl(watcher_fd, op, fd.native(), &raw mut event) }; self.flags.insert(Flags::WasEverRegistered); if let Some(errno) = errno_sys(ctl, sys::Tag::epoll_ctl) { - self.deactivate(loop_); + self.deactivate(vm); return errno; } } @@ -764,7 +776,7 @@ impl FilePoll { let errno = sys::get_errno(rc); if errno != sys::E::SUCCESS { - self.deactivate(loop_); + self.deactivate(vm); return sys::Result::Err(sys::Error::from_code(errno, sys::Tag::kqueue)); } } @@ -828,12 +840,12 @@ impl FilePoll { self.flags.insert(Flags::WasEverRegistered); if let Some(err) = errno_sys(rc, sys::Tag::kevent) { - self.deactivate(loop_); + self.deactivate(vm); return err; } } - self.activate(loop_); + self.activate(vm); self.flags.insert(match flag { Flags::Readable => Flags::PollReadable, Flags::Process => { @@ -856,13 +868,13 @@ impl FilePoll { sys::Result::Ok(()) } - pub fn unregister(&mut self, loop_: &mut Loop, force_unregister: bool) -> sys::Result<()> { - self.unregister_with_fd(loop_, self.fd, force_unregister) + pub fn unregister(&mut self, vm: EventLoopCtx, force_unregister: bool) -> sys::Result<()> { + self.unregister_with_fd(vm, self.fd, force_unregister) } pub(crate) fn unregister_with_fd( &mut self, - loop_: &mut Loop, + vm: EventLoopCtx, fd: Fd, force_unregister: bool, ) -> sys::Result<()> { @@ -874,7 +886,7 @@ impl FilePoll { target_os = "macos", target_os = "freebsd" ))] - let result = self.unregister_with_fd_impl(loop_, fd, force_unregister); + let result = self.unregister_with_fd_impl(vm, fd, force_unregister); #[cfg(not(any( target_os = "linux", target_os = "android", @@ -885,7 +897,7 @@ impl FilePoll { let _ = (fd, force_unregister); sys::Result::Ok(()) }; - self.deactivate(loop_); + self.deactivate(vm); result } @@ -897,7 +909,7 @@ impl FilePoll { ))] fn unregister_with_fd_impl( &mut self, - loop_: &mut Loop, + vm: EventLoopCtx, fd: Fd, force_unregister: bool, ) -> sys::Result<()> { @@ -914,7 +926,7 @@ impl FilePoll { } debug_assert!(fd != INVALID_FD); - let watcher_fd = loop_.fd; + let watcher_fd = self.loop_mut(vm).fd; let both_directions = self.flags.contains(Flags::PollReadable) && self.flags.contains(Flags::PollWritable); let flag: Flags = 'brk: { @@ -1209,6 +1221,9 @@ pub enum Flags { IgnoreUpdates, Socket, + + /// Registered on `Bun.spawnSync`'s isolated loop, not the thread's loop. + SpawnSyncLoop, } pub type FlagsSet = enumset::EnumSet; @@ -1487,7 +1502,7 @@ unsafe extern "C" fn Bun__internal_dispatch_ready_poll( // SAFETY: `loop_` is the live uws loop. Do *not* materialize `&mut *loop_` // here — `on_update` (via `__bun_run_file_poll`) re-enters the loop and conjures - // a fresh `&mut Loop` through `EventLoopCtx::platform_event_loop()`; a + // a fresh `&mut Loop` through `EventLoopCtx::loop_mut()`; a // protected `&mut Loop` spanning that call would be SB-UB. Take a short-lived // `&*loop_` only to copy the POD event onto the stack (the `BackRef`-style // accessor returns by value), then drop the borrow before dispatching so the diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index 55ecef14a13e..589cb3963f9d 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -2040,9 +2040,6 @@ impl bun_io::pipe_writer::PosixStreamingWriterParent for Terminal { .event_loop_handle .as_event_loop_ctx() } - unsafe fn loop_(this: *mut Self) -> *mut bun_uws_sys::Loop { - Self::from_parent_ptr(this).event_loop_handle.r#loop() - } } #[cfg(windows)] diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 7a89599daf03..5fbd90592447 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -1996,6 +1996,13 @@ fn spawn_maybe_sync( return Ok(JSValue::ZERO); } + // Debug-only fault injection for test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts: + // run finalizers while the isolated loop is still installed. + #[cfg(debug_assertions)] + if bun_core::env_var::feature_flag::BUN_INTERNAL_SPAWN_SYNC_GC.get() == Some(true) { + let _ = global_this.vm().run_gc(true); + } + subprocess.update_has_pending_activity(); let signal_code = SubprocessT::get_signal_code(subprocess, global_this); diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index def668763d7c..d9597143a876 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -4807,8 +4807,6 @@ impl Resolver { Async::posix_event_loop::poll_tag::DNS_RESOLVER, self.as_ctx_ptr().cast::<()>(), ); - // SAFETY: `event_loop_handle` is set once VM is initialized; live for VM lifetime. - let loop_ = unsafe { &mut *self.vm().event_loop_handle.unwrap() }; // SAFETY: single-JS-thread; the `&mut PollsMap` borrow does not span // any re-entrant call (`FilePoll::register` is a syscall wrapper). let polls = unsafe { self.polls.get_mut() }; @@ -4838,12 +4836,12 @@ impl Resolver { // direction armed would busy-loop on level-triggered writable // once the socket connects. Full resync is the simplest // correct path and c-ares DNS fds are short-lived. - let _ = poll.unregister(loop_, false); + let _ = poll.unregister(ctx, false); if readable { - let _ = poll.register(loop_, Async::PollKind::Readable, false); + let _ = poll.register(ctx, Async::PollKind::Readable, false); } if writable { - let _ = poll.register(loop_, Async::PollKind::Writable, false); + let _ = poll.register(ctx, Async::PollKind::Writable, false); } } else { // Only adding directions (or no change). register() issues a @@ -4851,10 +4849,10 @@ impl Resolver { // on kqueue EV_ADD creates a separate (ident, filter) knote // without disturbing the existing one. if readable && !have_readable { - let _ = poll.register(loop_, Async::PollKind::Readable, false); + let _ = poll.register(ctx, Async::PollKind::Readable, false); } if writable && !have_writable { - let _ = poll.register(loop_, Async::PollKind::Writable, false); + let _ = poll.register(ctx, Async::PollKind::Writable, false); } } } diff --git a/src/runtime/dns_jsc/dns_sd.rs b/src/runtime/dns_jsc/dns_sd.rs index 12157ed64e14..c4e1585dc0c3 100644 --- a/src/runtime/dns_jsc/dns_sd.rs +++ b/src/runtime/dns_jsc/dns_sd.rs @@ -377,10 +377,8 @@ impl SharedConnection { ); // SAFETY: `FilePoll::init` returned a live pool slot; exclusive here. let poll = unsafe { &mut *poll_ptr }; - // SAFETY: the event loop outlives every lookup made on it. - let loop_ = unsafe { ctx.platform_event_loop() }; let rc = poll.register_with_fd( - loop_, + ctx, Async::PollKind::Readable, Async::posix_event_loop::OneShotFlag::None, fd, diff --git a/src/runtime/node/memory_pressure.rs b/src/runtime/node/memory_pressure.rs index 040fc1e0cf16..bf6cce71c470 100644 --- a/src/runtime/node/memory_pressure.rs +++ b/src/runtime/node/memory_pressure.rs @@ -199,10 +199,8 @@ mod posix { NonNull::<()>::dangling().as_ptr(), ), ); - // SAFETY: `poll` is the fresh hive slot; `platform_event_loop` is the live uws loop. - let result = unsafe { - (*poll).register(ctx.platform_event_loop(), Flags::MemoryPressure, false) - }; + // SAFETY: `poll` is the fresh hive slot. + let result = unsafe { (*poll).register(ctx, Flags::MemoryPressure, false) }; if result.is_err() { // SAFETY: fresh hive slot never handed out. deinit_poll(unsafe { &mut *poll }); diff --git a/src/runtime/socket/WindowsNamedPipe.rs b/src/runtime/socket/WindowsNamedPipe.rs index 043fd6dc9b2c..f48b5214ef22 100644 --- a/src/runtime/socket/WindowsNamedPipe.rs +++ b/src/runtime/socket/WindowsNamedPipe.rs @@ -1165,7 +1165,6 @@ bun_io::impl_streaming_writer_parent! { on_ready = on_writable, on_close = on_close, event_loop = |this| (*this).event_loop_handle.as_event_loop_ctx(), - uws_loop = |this| (*this).vm.uws_loop(), uv_loop = |this| (*this).vm.uv_loop(), ref_ = |this| (&*this).r#ref(), deref = |this| (&*this).deref(), diff --git a/src/runtime/timer/mod.rs b/src/runtime/timer/mod.rs index fd878ef5cd26..73363c89d46a 100644 --- a/src/runtime/timer/mod.rs +++ b/src/runtime/timer/mod.rs @@ -613,6 +613,9 @@ pub(crate) struct All { pub(crate) thread_id: std::thread::ThreadId, pub(crate) timers: TimerHeap, pub(crate) active_timer_count: i32, + /// `active_timer_count` went positive on `Bun.spawnSync`'s isolated loop. + #[cfg(not(windows))] + timer_refd_spawn_sync_loop: bool, #[cfg(windows)] pub(crate) uv_timer: bun_sys::windows::libuv::Timer, /// Whether we have emitted a warning for passing a negative timeout duration @@ -623,6 +626,9 @@ pub(crate) struct All { /// TimerObjectInternals.epoch. Masked to 25 bits on increment. pub(crate) epoch: u32, pub(crate) immediate_ref_count: i32, + /// `immediate_ref_count` went positive on `Bun.spawnSync`'s isolated loop. + #[cfg(not(windows))] + immediate_refd_spawn_sync_loop: bool, #[cfg(windows)] pub(crate) uv_idle: bun_sys::windows::libuv::uv_idle_t, pub(crate) event_loop_delay: EventLoopDelayMonitor, @@ -632,6 +638,29 @@ pub(crate) struct All { pub(crate) wtf_timers: Guarded, } +/// Whether `uws_loop`, the VM's current loop, is `Bun.spawnSync`'s isolated loop. +#[cfg(not(windows))] +fn is_spawn_sync_loop(uws_loop: *mut bun_uws_sys::Loop) -> bool { + uws_loop != bun_uws_sys::Loop::get() +} + +/// The loop a ref was taken on: the current loop `uws_loop` when the ref was +/// taken on the isolated `spawnSync` loop, else the thread's loop. +#[cfg(not(windows))] +fn refd_loop( + spawn_sync_loop: bool, + uws_loop: *mut bun_uws_sys::Loop, +) -> &'static mut bun_uws_sys::Loop { + let loop_ = if spawn_sync_loop { + uws_loop + } else { + bun_uws_sys::Loop::get() + }; + // SAFETY: the caller passes the VM's live current loop, and the thread's + // loop lives as long as the VM. + unsafe { &mut *loop_ } +} + impl All { pub(crate) fn init() -> Self { Self { @@ -639,12 +668,16 @@ impl All { thread_id: std::thread::current().id(), timers: TimerHeap::default(), active_timer_count: 0, + #[cfg(not(windows))] + timer_refd_spawn_sync_loop: false, #[cfg(windows)] uv_timer: bun_core::ffi::zeroed(), warned_negative_number: false, warned_not_number: false, epoch: 0, immediate_ref_count: 0, + #[cfg(not(windows))] + immediate_refd_spawn_sync_loop: false, #[cfg(windows)] uv_idle: bun_core::ffi::zeroed(), event_loop_delay: EventLoopDelayMonitor::default(), @@ -1135,8 +1168,11 @@ impl All { self.immediate_ref_count = new; if old <= 0 && new > 0 { #[cfg(not(windows))] - // SAFETY: caller passes the VM's live uws loop - unsafe { &mut *uws_loop }.ref_(); + { + self.immediate_refd_spawn_sync_loop = is_spawn_sync_loop(uws_loop); + // SAFETY: caller passes the VM's live uws loop + unsafe { &mut *uws_loop }.ref_(); + } #[cfg(windows)] { // Lazy-init the idle handle and start @@ -1153,8 +1189,7 @@ impl All { } } else if old > 0 && new <= 0 { #[cfg(not(windows))] - // SAFETY: caller passes the VM's live uws loop - unsafe { &mut *uws_loop }.unref(); + refd_loop(self.immediate_refd_spawn_sync_loop, uws_loop).unref(); #[cfg(windows)] if !self.uv_idle.data.is_null() { self.uv_idle.stop(); @@ -1186,8 +1221,11 @@ impl All { self.active_timer_count = new; if old <= 0 && new > 0 { #[cfg(not(windows))] - // SAFETY: caller passes the VM's live uws loop - unsafe { &mut *uws_loop }.ref_(); + { + self.timer_refd_spawn_sync_loop = is_spawn_sync_loop(uws_loop); + // SAFETY: caller passes the VM's live uws loop + unsafe { &mut *uws_loop }.ref_(); + } // `uv_timer.ref()` is intentionally unconditional (no `data != // null` guard). Invariant: every path that reaches a positive // `active_timer_count` first inserts a timer, and `insert` @@ -1197,8 +1235,7 @@ impl All { self.uv_timer.ref_(); } else if old > 0 && new <= 0 { #[cfg(not(windows))] - // SAFETY: caller passes the VM's live uws loop - unsafe { &mut *uws_loop }.unref(); + refd_loop(self.timer_refd_spawn_sync_loop, uws_loop).unref(); #[cfg(windows)] self.uv_timer.unref(); } diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 96553c72be14..5687f6d07732 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -151,7 +151,6 @@ bun_io::impl_streaming_writer_parent! { on_ready = on_ready, on_close = on_close, event_loop = |this| (*this).io_evtloop(), - uws_loop = |this| (*this).event_loop_handle.r#loop(), uv_loop = |this| (*this).event_loop_handle.uv_loop(), ref_ = |this| (&*this).ref_(), deref = |this| FileSink::deref(this), diff --git a/src/spawn/process.rs b/src/spawn/process.rs index a4c871b29326..c03c200383e2 100644 --- a/src/spawn/process.rs +++ b/src/spawn/process.rs @@ -465,15 +465,9 @@ impl Process { // Borrow scoped to the call. unsafe { (*poll).enable_keeping_process_alive(ctx) }; - // SAFETY: poll is live and `platform_event_loop` returns the live - // uws loop; both `&mut`s are scoped to the `register` call. - match unsafe { - (*poll).register( - &mut *self.event_loop.platform_event_loop(), - bun_io::PollKind::Process, - PROCESS_POLL_ONE_SHOT, - ) - } { + // SAFETY: poll is live; borrow scoped to the `register` call. + match unsafe { (*poll).register(ctx, bun_io::PollKind::Process, PROCESS_POLL_ONE_SHOT) } + { Ok(()) => { self.ref_(); Ok(()) @@ -503,13 +497,7 @@ impl Process { } if let Some(fd) = self.poller.fd_poll_mut() { - // SAFETY: `platform_event_loop` returns the live uws loop; borrow - // scoped to the `register` call. - let maybe = fd.register( - unsafe { &mut *self.event_loop.platform_event_loop() }, - bun_io::PollKind::Process, - PROCESS_POLL_ONE_SHOT, - ); + let maybe = fd.register(ctx, bun_io::PollKind::Process, PROCESS_POLL_ONE_SHOT); if maybe.is_ok() { self.ref_(); } diff --git a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts index 70619fddfcb0..96ce02e1f725 100644 --- a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts +++ b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isDebug, isWindows } from "harness"; import { join } from "node:path"; describe.concurrent("spawnSync isolated event loop", () => { @@ -188,4 +188,61 @@ describe.concurrent("spawnSync isolated event loop", () => { expect(stdout).not.toContain("FAIL"); expect(exitCode).toBe(0); }); + + // A GC that runs while spawnSync blocks finalizes objects whose polls live on + // the main loop. If those polls are released against the isolated loop, its + // poll count reaches zero while the next spawnSync still has live polls, and + // that spawnSync spins without polling until its timeout. + // BUN_INTERNAL_SPAWN_SYNC_GC (debug builds) forces such a GC. + test.skipIf(!isDebug || isWindows)( + "finalizers that run inside spawnSync do not stall the next spawnSync", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + // Three writers on stderr (a pipe here), each with a poll registered + // on the main loop, with nothing left that references them. + function leakWriters() { + for (let i = 0; i < 3; i++) { + const writer = Bun.stderr.writer(); + writer.write(""); + writer.flush(); + } + } + leakWriters(); + + // The GC at the end of this call finalizes the three writers. + Bun.spawnSync({ cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore" }); + + // pidfd + stdout + stderr: three polls, the same count the three + // writers released. + const result = Bun.spawnSync({ + cmd: ["echo", "second"], + stdout: "pipe", + stderr: "pipe", + timeout: 5000, + }); + console.log( + JSON.stringify({ + stdout: result.stdout.toString(), + exitedDueToTimeout: result.exitedDueToTimeout, + exitCode: result.exitCode, + }), + ); + `, + ], + env: { ...bunEnv, BUN_INTERNAL_SPAWN_SYNC_GC: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + + // stderr is drained, not asserted: the child's JSON and exit code carry the signal. + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout.trim()).toBe(JSON.stringify({ stdout: "second\n", exitedDueToTimeout: false, exitCode: 0 })); + expect(exitCode).toBe(0); + }, + ); }); From f22388722aa9edaff5f94557d966de02b2aa9fea Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 23 Aug 2026 05:45:00 +0000 Subject: [PATCH 02/14] Run the spawnSync finalizer test on release builds The test drove the GC inside spawnSync through a debug-only hook, so no CI lane ran it. BUN_JSC_slowPathAllocsBetweenGCs=5 makes JSC run a full synchronous GC every few slow-path allocations, and the allocations that build the first spawnSync's result are enough to run the dropped writers' finalizers while the isolated loop is installed. The writers stay reachable until right before that spawnSync so no earlier GC frees them. On the unfixed release build the second spawnSync spins into its timeout; with the fix it returns at once. The BUN_INTERNAL_SPAWN_SYNC_GC flag and its hook are removed. --- src/bun_core/env_var.rs | 2 - src/runtime/api/bun/js_bun_spawn_bindings.rs | 7 -- .../spawnsync-isolated-event-loop.test.ts | 97 +++++++++---------- 3 files changed, 46 insertions(+), 60 deletions(-) diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index d9e5d82f3bfb..5295b888775d 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -274,8 +274,6 @@ pub mod feature_flag { new_feature_flag!(pub BUN_INTERNAL_BUNX_INSTALL, "BUN_INTERNAL_BUNX_INSTALL", {}); // Debug-only fault injection for test/js/bun/spawn/spawn-pipe-start-error.test.ts. new_feature_flag!(pub BUN_INTERNAL_FAIL_PIPE_READER_START, "BUN_INTERNAL_FAIL_PIPE_READER_START", {}); - // Debug-only fault injection for test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts. - new_feature_flag!(pub BUN_INTERNAL_SPAWN_SYNC_GC, "BUN_INTERNAL_SPAWN_SYNC_GC", {}); // Test-only: bypass the stdin isatty gate in `bun update --interactive` so // tests can drive the multi-select by writing keystrokes to a pipe. new_feature_flag!(pub BUN_INTERNAL_INTERACTIVE_ASSUME_TTY, "BUN_INTERNAL_INTERACTIVE_ASSUME_TTY", {}); diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 5fbd90592447..7a89599daf03 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -1996,13 +1996,6 @@ fn spawn_maybe_sync( return Ok(JSValue::ZERO); } - // Debug-only fault injection for test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts: - // run finalizers while the isolated loop is still installed. - #[cfg(debug_assertions)] - if bun_core::env_var::feature_flag::BUN_INTERNAL_SPAWN_SYNC_GC.get() == Some(true) { - let _ = global_this.vm().run_gc(true); - } - subprocess.update_has_pending_activity(); let signal_code = SubprocessT::get_signal_code(subprocess, global_this); diff --git a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts index 96ce02e1f725..82d159e7ee94 100644 --- a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts +++ b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isDebug, isWindows } from "harness"; +import { bunEnv, bunExe, isWindows } from "harness"; import { join } from "node:path"; describe.concurrent("spawnSync isolated event loop", () => { @@ -193,56 +193,51 @@ describe.concurrent("spawnSync isolated event loop", () => { // the main loop. If those polls are released against the isolated loop, its // poll count reaches zero while the next spawnSync still has live polls, and // that spawnSync spins without polling until its timeout. - // BUN_INTERNAL_SPAWN_SYNC_GC (debug builds) forces such a GC. - test.skipIf(!isDebug || isWindows)( - "finalizers that run inside spawnSync do not stall the next spawnSync", - async () => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` - // Three writers on stderr (a pipe here), each with a poll registered - // on the main loop, with nothing left that references them. - function leakWriters() { - for (let i = 0; i < 3; i++) { - const writer = Bun.stderr.writer(); - writer.write(""); - writer.flush(); - } - } - leakWriters(); + // BUN_JSC_slowPathAllocsBetweenGCs runs a full synchronous GC every few + // slow-path allocations, so the allocations that build the first spawnSync's + // result run the writers' finalizers while the isolated loop is installed. + test.skipIf(isWindows)("finalizers that run inside spawnSync do not stall the next spawnSync", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + // Three writers on stderr (a pipe here), each with a poll registered + // on the main loop. They stay reachable until right before the first + // spawnSync so that no earlier GC frees them. + let writers = []; + for (let i = 0; i < 3; i++) { + const writer = Bun.stderr.writer(); + writer.write(""); + writer.flush(); + writers.push(writer); + } + const first = { cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore" }; + // pidfd + stdout + stderr: three polls, the same count the three + // writers release. + const second = { cmd: ["echo", "second"], stdout: "pipe", stderr: "pipe", timeout: 2000 }; + writers = null; + Bun.spawnSync(first); + + const result = Bun.spawnSync(second); + console.log( + JSON.stringify({ + stdout: result.stdout.toString(), + exitedDueToTimeout: result.exitedDueToTimeout, + exitCode: result.exitCode, + }), + ); + `, + ], + env: { ...bunEnv, BUN_JSC_slowPathAllocsBetweenGCs: "5" }, + stdout: "pipe", + stderr: "pipe", + }); - // The GC at the end of this call finalizes the three writers. - Bun.spawnSync({ cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore" }); + // stderr is drained, not asserted: the child's JSON and exit code carry the signal. + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - // pidfd + stdout + stderr: three polls, the same count the three - // writers released. - const result = Bun.spawnSync({ - cmd: ["echo", "second"], - stdout: "pipe", - stderr: "pipe", - timeout: 5000, - }); - console.log( - JSON.stringify({ - stdout: result.stdout.toString(), - exitedDueToTimeout: result.exitedDueToTimeout, - exitCode: result.exitCode, - }), - ); - `, - ], - env: { ...bunEnv, BUN_INTERNAL_SPAWN_SYNC_GC: "1" }, - stdout: "pipe", - stderr: "pipe", - }); - - // stderr is drained, not asserted: the child's JSON and exit code carry the signal. - const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - - expect(stdout.trim()).toBe(JSON.stringify({ stdout: "second\n", exitedDueToTimeout: false, exitCode: 0 })); - expect(exitCode).toBe(0); - }, - ); + expect(stdout.trim()).toBe(JSON.stringify({ stdout: "second\n", exitedDueToTimeout: false, exitCode: 0 })); + expect(exitCode).toBe(0); + }); }); From c9802f737a609f7f3db26d3f3427f0e89a00c22b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 26 Aug 2026 06:54:17 +0000 Subject: [PATCH 03/14] Make the spawnSync finalizer test independent of the allocation phase The forced GC has to land after spawnSync installs the isolated loop. A first spawnSync call creates lazy structures and Bun.spawnSync itself, so it can trigger the GC before the swap and free the writers on the main loop. Warm spawnSync up while the writers are still reachable, then drop them: the next call allocates nothing before the swap. The probe child now sleeps 300ms so the parent has to poll for its exit. With `echo` the child could exit before the polls were registered, and the unfixed build then returned without spinning. On the unfixed release, canary, and main debug builds the probe spins in 105 of 105 runs across shifted allocation phases; on the fixed build it returns in 35 of 35. --- .../spawnsync-isolated-event-loop.test.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts index 82d159e7ee94..2274f414f0b6 100644 --- a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts +++ b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts @@ -194,8 +194,8 @@ describe.concurrent("spawnSync isolated event loop", () => { // poll count reaches zero while the next spawnSync still has live polls, and // that spawnSync spins without polling until its timeout. // BUN_JSC_slowPathAllocsBetweenGCs runs a full synchronous GC every few - // slow-path allocations, so the allocations that build the first spawnSync's - // result run the writers' finalizers while the isolated loop is installed. + // slow-path allocations. A warmed-up spawnSync allocates nothing before it + // installs the isolated loop, so the GC that frees the writers runs inside it. test.skipIf(isWindows)("finalizers that run inside spawnSync do not stall the next spawnSync", async () => { await using proc = Bun.spawn({ cmd: [ @@ -203,8 +203,7 @@ describe.concurrent("spawnSync isolated event loop", () => { "-e", ` // Three writers on stderr (a pipe here), each with a poll registered - // on the main loop. They stay reachable until right before the first - // spawnSync so that no earlier GC frees them. + // on the main loop. let writers = []; for (let i = 0; i < 3; i++) { const writer = Bun.stderr.writer(); @@ -212,11 +211,19 @@ describe.concurrent("spawnSync isolated event loop", () => { writer.flush(); writers.push(writer); } - const first = { cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore" }; + const first = { cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore", timeout: 2000 }; // pidfd + stdout + stderr: three polls, the same count the three - // writers release. - const second = { cmd: ["echo", "second"], stdout: "pipe", stderr: "pipe", timeout: 2000 }; + // writers release. The child outlives the parent's poll registration, + // so the parent has to poll for its exit. + const second = { cmd: ["sh", "-c", "sleep 0.3; echo second"], stdout: "pipe", stderr: "pipe", timeout: 2000 }; + + // Warm up while the writers are still reachable: lazy structures and + // Bun.spawnSync itself are created here, on the main loop. + Bun.spawnSync(first); writers = null; + // The first allocations after the drop are this call's result, built + // while the isolated loop is installed: the GC they trigger frees the + // writers there. Bun.spawnSync(first); const result = Bun.spawnSync(second); From 9a35ffba9faccca37ee30af2582a44c7a6d67233 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:27:14 +0000 Subject: [PATCH 04/14] FileSink::setup: leave the fd to the writer when start() fails on POSIX After a failed writer.start(), the POSIX writer still holds the fd in its poll and closes it on Drop. setup() closed the same fd first, so the deferred close in the Closer found it closed already. Windows keeps the close in setup(): Source::open leaves no source there. Add a spawnSync test: a Bun.file(2).writer() finalized inside spawnSync must unregister its dup of fd 2 from the main epoll, so the next writer on the same fd number does not fail with EEXIST. --- src/runtime/webcore/FileSink.rs | 7 ++- test/js/bun/spawn/spawnSync.test.ts | 67 ++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 5687f6d07732..090980680760 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -6,7 +6,9 @@ use bun_io::pipe_writer::BaseWindowsPipeWriter as _; use bun_io::{self, WriteResult, WriteStatus}; use bun_jsc::JsCell; use bun_ptr::RefPtr; -use bun_sys::{self as sys, Fd, FdExt as _}; +#[cfg(windows)] +use bun_sys::FdExt as _; +use bun_sys::{self as sys, Fd}; use crate::api::bun::process::Status as SpawnStatus; use crate::webcore::jsc::{CallFrame, EventLoopHandle, JSGlobalObject, JSValue, JsResult}; @@ -719,6 +721,9 @@ impl FileSink { // SAFETY(JsCell): `start` is pure I/O setup; no JS. match self.writer.with_mut(|w| w.start(fd, self.pollable.get())) { sys::Result::Err(err) => { + // POSIX: the writer's poll holds `fd` and closes it with the writer + // (see `Terminal::init`). Windows: `start` left no source, so the fd is ours. + #[cfg(windows)] fd.close(); return sys::Result::Err(err); } diff --git a/test/js/bun/spawn/spawnSync.test.ts b/test/js/bun/spawn/spawnSync.test.ts index fdfdb3b6cf20..7692a70b453d 100644 --- a/test/js/bun/spawn/spawnSync.test.ts +++ b/test/js/bun/spawn/spawnSync.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, bunRun, isLinux, isMusl, isPosix, isWindows } from "harness"; +import { describe, expect, it, test } from "bun:test"; +import { bunEnv, bunExe, bunRun, isLinux, isMusl, isPosix, isWindows, tempDir } from "harness"; import { totalmem } from "os"; import { join } from "path"; describe("spawnSync", () => { @@ -207,3 +207,66 @@ describe("uid/gid", () => { expect(thrown?.code).toBe("EPERM"); }); }); + +// spawnSync points the VM's loop handle at its private loop for the whole +// call. A FilePoll torn down inside that window (here: the GC finalizing a +// garbage `Bun.file(2).writer()` while spawnSync builds its result) must +// still unregister from the loop it was registered with. Before the fix the +// EPOLL_CTL_DEL went to the private loop, so the main loop kept a stale +// entry for the dup'd fd number. The next `Bun.file(2).writer()` dups to the +// same number, EPOLL_CTL_ADD fails with EEXIST, and the fd is closed twice: +// once by FileSink::setup and once by the writer's Drop through the Closer. +test.skipIf(!isLinux)("a writer finalized during spawnSync does not break the next writer on the same fd", async () => { + // A file, not `-e`: under `-e` the leaked writers stay reachable and are never finalized. + using dir = tempDir("spawnsync-writer-finalized", { + "fixture.js": ` + import { fstatSync, readdirSync } from "node:fs"; + const stderrInode = fstatSync(2).ino; + // Every writer holds a dup() of fd 2; those share stderr's inode. + const stderrDups = () => + readdirSync("/proc/self/fd") + .map(Number) + .filter(fd => { + if (fd === 2) return false; + try { + return fstatSync(fd).ino === stderrInode; + } catch { + return false; + } + }); + function leakWriter() { + const w = Bun.file(2).writer(); + w.write(""); + } + for (let i = 0; i < 4; i++) leakWriter(); + const dups = stderrDups().length; + // The collector finishes inside spawnSync: the first JS allocation after + // the wait (the result object) is where the mutator runs the sweep. + Bun.gc(false); + const r = Bun.spawnSync({ cmd: ["sleep", "0.2"] }); + // The finalized writers close their fds on the work pool; wait for that so + // dup(2) below hands out one of the same numbers. + const deadline = Date.now() + 5000; + while (Date.now() < deadline && stderrDups().length > 0) Bun.sleepSync(5); + const remaining = stderrDups().length; + const w = Bun.file(2).writer(); + w.write("second writer ok\\n"); + w.flush(); + console.log(JSON.stringify({ exitCode: r.exitCode, dups, remaining })); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + // A pipe is pollable, so the writers register their dup of fd 2 with epoll. + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ exitCode: 0, dups: 4, remaining: 0 }) + "\n", + stderr: "second writer ok\n", + exitCode: 0, + }); +}); From 6dba38c9c60891955c85335994062a44325fa1bd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:34:07 +0000 Subject: [PATCH 05/14] FileSink::setup: one-line note on the cfg(windows) close --- src/runtime/webcore/FileSink.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 090980680760..003df94efc0f 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -721,8 +721,7 @@ impl FileSink { // SAFETY(JsCell): `start` is pure I/O setup; no JS. match self.writer.with_mut(|w| w.start(fd, self.pollable.get())) { sys::Result::Err(err) => { - // POSIX: the writer's poll holds `fd` and closes it with the writer - // (see `Terminal::init`). Windows: `start` left no source, so the fd is ours. + // The POSIX writer keeps `fd` in its poll after a failed `start` and closes it on Drop. #[cfg(windows)] fd.close(); return sys::Result::Err(err); From 6727e1ab5b34f7fccb4a2b84d67aa61aaedeaa07 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:12:18 +0000 Subject: [PATCH 06/14] timer: take the EventLoopCtx for the loop ref instead of a raw loop pointer increment_timer_ref and increment_immediate_ref carried their own copy of the spawnSync loop predicate and release path over a *mut Loop. They now record and release the ref through EventLoopCtx::is_spawn_sync_loop, loop_ref and the new loop_unref_for, the same way FilePoll and KeepAlive do, which also gives the timer the release-after-return debug assert. --- src/io/keep_alive.rs | 5 +- src/io/lib.rs | 11 +++- src/runtime/api/bun/subprocess.rs | 3 +- src/runtime/dns_jsc/dns.rs | 9 ++-- src/runtime/timer/mod.rs | 57 ++++----------------- src/runtime/timer/timer_object_internals.rs | 10 ++-- 6 files changed, 29 insertions(+), 66 deletions(-) diff --git a/src/io/keep_alive.rs b/src/io/keep_alive.rs index 0bb55aa2328b..2b109fc86759 100644 --- a/src/io/keep_alive.rs +++ b/src/io/keep_alive.rs @@ -47,11 +47,10 @@ impl KeepAlive { return; } self.status = Status::Inactive; - let loop_ = event_loop_ctx.loop_for(self.spawn_sync_loop); #[cfg(not(windows))] - loop_.unref(); + event_loop_ctx.loop_unref_for(self.spawn_sync_loop); #[cfg(windows)] - loop_.sub_active(1); + event_loop_ctx.loop_sub_active(1); } /// Prevent a poll from keeping the process alive on the next tick. diff --git a/src/io/lib.rs b/src/io/lib.rs index 2adecdd9316d..066e656a2c14 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -374,13 +374,14 @@ impl EventLoopCtx { } /// True while `Bun.spawnSync` has its isolated loop installed as the current loop. #[inline] - pub(crate) fn is_spawn_sync_loop(&self) -> bool { + pub fn is_spawn_sync_loop(&self) -> bool { self.platform_event_loop_ptr() != bun_uws_sys::Loop::get() } /// The loop a counter was taken on: the isolated `spawnSync` loop when its /// owner recorded `spawn_sync_loop` from [`Self::is_spawn_sync_loop`], else /// the thread's loop, even while `spawnSync` has swapped the current loop. #[inline] + #[cfg(not(windows))] fn loop_for(&self, spawn_sync_loop: bool) -> &'static mut bun_uws_sys::Loop { if spawn_sync_loop { debug_assert!( @@ -393,9 +394,15 @@ impl EventLoopCtx { unsafe { &mut *bun_uws_sys::Loop::get() } } #[inline] - pub(crate) fn loop_ref(&self) { + pub fn loop_ref(&self) { self.loop_mut().ref_(); } + /// Releases a [`Self::loop_ref`] taken while [`Self::is_spawn_sync_loop`] was `spawn_sync_loop`. + #[inline] + #[cfg(not(windows))] + pub fn loop_unref_for(&self, spawn_sync_loop: bool) { + self.loop_for(spawn_sync_loop).unref(); + } #[inline] #[cfg(windows)] pub(crate) fn loop_dec(&self) { diff --git a/src/runtime/api/bun/subprocess.rs b/src/runtime/api/bun/subprocess.rs index 1d8686eb6ea9..bb40847c39aa 100644 --- a/src/runtime/api/bun/subprocess.rs +++ b/src/runtime/api/bun/subprocess.rs @@ -649,9 +649,8 @@ impl Subprocess<'_> { return; } self.event_loop_timer_refd.set(refd); - let uws_loop = self.global_this().bun_vm().uws_loop(); let delta: i32 = if refd { 1 } else { -1 }; - Self::timer_all().increment_timer_ref(delta, uws_loop); + Self::timer_all().increment_timer_ref(delta, bun_io::js_vm_ctx()); } #[inline] diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index d9597143a876..00502603cfe9 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -3951,7 +3951,6 @@ impl Resolver { sec: now.sec, nsec: now.nsec, }; - let uws_loop = vm.uws_loop(); // R-2: `&self` carries no `noalias`, and every field touched below is // UnsafeCell-backed, so the re-entrant `ares_process_fd` callbacks // (`request_completed`, `drain_pending_*`) may freely re-derive @@ -3962,7 +3961,7 @@ impl Resolver { // resolve via the high-tier `RuntimeState` hook. let state = crate::jsc_hooks::runtime_state(); // SAFETY: `state` is the boxed per-thread `RuntimeState`; single-threaded JS heap. - unsafe { (*state).timer.increment_timer_ref(-1, uws_loop) }; + unsafe { (*state).timer.increment_timer_ref(-1, js_event_loop_ctx()) }; // SAFETY: `deref_this` is the heap allocation from `init`; releases // `add_timer`'s ref. May be the final release; nothing touches // `*self` after this point. @@ -4054,11 +4053,10 @@ impl Resolver { nsec: next.nsec, } }); - let uws_loop = self.vm().uws_loop(); let state = crate::jsc_hooks::runtime_state(); // SAFETY: `state` is the boxed per-thread `RuntimeState`; single-threaded JS heap. unsafe { - (*state).timer.increment_timer_ref(1, uws_loop); + (*state).timer.increment_timer_ref(1, js_event_loop_ctx()); // whole-struct provenance: `from_field_ptr!` recovers the container on fire (*state).timer.insert( core::ptr::addr_of!(self.event_loop_timer) @@ -4083,9 +4081,8 @@ impl Resolver { // global-resolver permanent pin), so this // `deref` cannot reach 0 while `&self` is live. unsafe { - let uws_loop = (*this).vm().uws_loop(); let state = crate::jsc_hooks::runtime_state(); - (*state).timer.increment_timer_ref(-1, uws_loop); + (*state).timer.increment_timer_ref(-1, js_event_loop_ctx()); Self::deref(this); } } diff --git a/src/runtime/timer/mod.rs b/src/runtime/timer/mod.rs index 73363c89d46a..a982f8ac00f6 100644 --- a/src/runtime/timer/mod.rs +++ b/src/runtime/timer/mod.rs @@ -638,29 +638,6 @@ pub(crate) struct All { pub(crate) wtf_timers: Guarded, } -/// Whether `uws_loop`, the VM's current loop, is `Bun.spawnSync`'s isolated loop. -#[cfg(not(windows))] -fn is_spawn_sync_loop(uws_loop: *mut bun_uws_sys::Loop) -> bool { - uws_loop != bun_uws_sys::Loop::get() -} - -/// The loop a ref was taken on: the current loop `uws_loop` when the ref was -/// taken on the isolated `spawnSync` loop, else the thread's loop. -#[cfg(not(windows))] -fn refd_loop( - spawn_sync_loop: bool, - uws_loop: *mut bun_uws_sys::Loop, -) -> &'static mut bun_uws_sys::Loop { - let loop_ = if spawn_sync_loop { - uws_loop - } else { - bun_uws_sys::Loop::get() - }; - // SAFETY: the caller passes the VM's live current loop, and the thread's - // loop lives as long as the VM. - unsafe { &mut *loop_ } -} - impl All { pub(crate) fn init() -> Self { Self { @@ -1156,22 +1133,15 @@ impl All { } } - /// # Safety - /// `uws_loop` must point to the calling VM's live uws loop. - // `uws_loop` is an FFI handle held as `*mut` by every caller; contract is - // documented in `# Safety` above. Cannot be `&mut` without breaking the - // out-of-file call sites that hold raw pointers. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub(crate) fn increment_immediate_ref(&mut self, delta: i32, uws_loop: *mut bun_uws_sys::Loop) { + pub(crate) fn increment_immediate_ref(&mut self, delta: i32, ctx: bun_io::EventLoopCtx) { let old = self.immediate_ref_count; let new = old + delta; self.immediate_ref_count = new; if old <= 0 && new > 0 { #[cfg(not(windows))] { - self.immediate_refd_spawn_sync_loop = is_spawn_sync_loop(uws_loop); - // SAFETY: caller passes the VM's live uws loop - unsafe { &mut *uws_loop }.ref_(); + self.immediate_refd_spawn_sync_loop = ctx.is_spawn_sync_loop(); + ctx.loop_ref(); } #[cfg(windows)] { @@ -1189,14 +1159,14 @@ impl All { } } else if old > 0 && new <= 0 { #[cfg(not(windows))] - refd_loop(self.immediate_refd_spawn_sync_loop, uws_loop).unref(); + ctx.loop_unref_for(self.immediate_refd_spawn_sync_loop); #[cfg(windows)] if !self.uv_idle.data.is_null() { self.uv_idle.stop(); } } #[cfg(windows)] - let _ = uws_loop; + let _ = ctx; } /// Empty `uv_idle` callback. Its presence alone @@ -1208,13 +1178,7 @@ impl All { // prevent libuv from polling forever } - /// # Safety - /// `uws_loop` must point to the calling VM's live uws loop. - // `uws_loop` is an FFI handle held as `*mut` by every caller; contract is - // documented in `# Safety` above. Cannot be `&mut` without breaking the - // out-of-file call sites that hold raw pointers. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub(crate) fn increment_timer_ref(&mut self, delta: i32, uws_loop: *mut bun_uws_sys::Loop) { + pub(crate) fn increment_timer_ref(&mut self, delta: i32, ctx: bun_io::EventLoopCtx) { let old = self.active_timer_count; let new = old + delta; debug_assert!(new >= 0); @@ -1222,9 +1186,8 @@ impl All { if old <= 0 && new > 0 { #[cfg(not(windows))] { - self.timer_refd_spawn_sync_loop = is_spawn_sync_loop(uws_loop); - // SAFETY: caller passes the VM's live uws loop - unsafe { &mut *uws_loop }.ref_(); + self.timer_refd_spawn_sync_loop = ctx.is_spawn_sync_loop(); + ctx.loop_ref(); } // `uv_timer.ref()` is intentionally unconditional (no `data != // null` guard). Invariant: every path that reaches a positive @@ -1235,12 +1198,12 @@ impl All { self.uv_timer.ref_(); } else if old > 0 && new <= 0 { #[cfg(not(windows))] - refd_loop(self.timer_refd_spawn_sync_loop, uws_loop).unref(); + ctx.loop_unref_for(self.timer_refd_spawn_sync_loop); #[cfg(windows)] self.uv_timer.unref(); } #[cfg(windows)] - let _ = uws_loop; + let _ = ctx; } /// VM teardown, after `cancel_all_timeout_objects`: unlink every timer still diff --git a/src/runtime/timer/timer_object_internals.rs b/src/runtime/timer/timer_object_internals.rs index 039f7b610e36..3acf77e40cf8 100644 --- a/src/runtime/timer/timer_object_internals.rs +++ b/src/runtime/timer/timer_object_internals.rs @@ -189,20 +189,18 @@ impl TimerObjectInternals { let state = crate::jsc_hooks::runtime_state(); debug_assert!(!state.is_null(), "RuntimeState not installed"); - // SAFETY: `vm` is the live per-thread VM (hook contract); field read only. - let uws_loop = unsafe { (*vm).uws_loop() }; + // SAFETY: `vm` is the live per-thread VM (hook contract). + let ctx = unsafe { VirtualMachine::event_loop_ctx(vm) }; let delta = if enable { 1 } else { -1 }; match self.flags.get().kind() { // SAFETY: `state` points at the boxed per-thread `RuntimeState`; // single-threaded JS heap so no concurrent `&mut` to `.timer`. Kind::SetTimeout | Kind::SetInterval => unsafe { - (*state).timer.increment_timer_ref(delta, uws_loop) + (*state).timer.increment_timer_ref(delta, ctx) }, // setImmediate has slightly different event loop logic // SAFETY: as above. - Kind::SetImmediate => unsafe { - (*state).timer.increment_immediate_ref(delta, uws_loop) - }, + Kind::SetImmediate => unsafe { (*state).timer.increment_immediate_ref(delta, ctx) }, } } From a2256c526da24e1dc2708f3da524221e711fe217 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:12:18 +0000 Subject: [PATCH 07/14] test: report how many writers the probe spawnSync collected The fixture asserted only the outcome of the third spawnSync. It now also reports, through WeakRefs created before a job boundary, that all three writers were collected by the time the probe call returned. A change that keeps them alive past the call, or stops the GC from running in it, now fails the test instead of passing it on an unfixed build. --- .../spawnsync-isolated-event-loop.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts index 2274f414f0b6..1bfea7d65070 100644 --- a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts +++ b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts @@ -196,6 +196,11 @@ describe.concurrent("spawnSync isolated event loop", () => { // BUN_JSC_slowPathAllocsBetweenGCs runs a full synchronous GC every few // slow-path allocations. A warmed-up spawnSync allocates nothing before it // installs the isolated loop, so the GC that frees the writers runs inside it. + // The fixture also reports how many writers were collected by the time that + // call returned, so a change that keeps them alive past the call, or stops + // the GC from running there, fails this test instead of passing it. + // (bun:internal-for-testing would give exact counts, but loading it under + // this GC setting takes a debug build 15s.) test.skipIf(isWindows)("finalizers that run inside spawnSync do not stall the next spawnSync", async () => { await using proc = Bun.spawn({ cmd: [ @@ -205,12 +210,17 @@ describe.concurrent("spawnSync isolated event loop", () => { // Three writers on stderr (a pipe here), each with a poll registered // on the main loop. let writers = []; + const refs = []; for (let i = 0; i < 3; i++) { const writer = Bun.stderr.writer(); writer.write(""); writer.flush(); writers.push(writer); + refs.push(new WeakRef(writer)); } + // A WeakRef keeps its target alive until the end of the job that + // created it. Cross a job boundary so only \`writers\` holds them. + await Bun.sleep(0); const first = { cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore", timeout: 2000 }; // pidfd + stdout + stderr: three polls, the same count the three // writers release. The child outlives the parent's poll registration, @@ -223,12 +233,14 @@ describe.concurrent("spawnSync isolated event loop", () => { writers = null; // The first allocations after the drop are this call's result, built // while the isolated loop is installed: the GC they trigger frees the - // writers there. + // writers there. Nothing may run between the drop and the call. Bun.spawnSync(first); + const collectedDuringCall = refs.filter(ref => ref.deref() === undefined).length; const result = Bun.spawnSync(second); console.log( JSON.stringify({ + collectedDuringCall, stdout: result.stdout.toString(), exitedDueToTimeout: result.exitedDueToTimeout, exitCode: result.exitCode, @@ -244,7 +256,9 @@ describe.concurrent("spawnSync isolated event loop", () => { // stderr is drained, not asserted: the child's JSON and exit code carry the signal. const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stdout.trim()).toBe(JSON.stringify({ stdout: "second\n", exitedDueToTimeout: false, exitCode: 0 })); + expect(stdout.trim()).toBe( + JSON.stringify({ collectedDuringCall: 3, stdout: "second\n", exitedDueToTimeout: false, exitCode: 0 }), + ); expect(exitCode).toBe(0); }); }); From 4f80ad934cf159f5ee0874eda661a44564538c55 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:29:58 +0000 Subject: [PATCH 08/14] PipeWriter: start() owns the fd on every failure path When start() reuses the poll of an earlier start(), that poll still holds the earlier fd, and a failed registration left the new fd with no owner. start() now closes the fd itself in that case, so FileSink::setup can rely on the writer on every path. Add a Linux test to filesink.test.ts that provokes the registration failure without spawnSync: close a live writer's dup of fd 2 so its epoll entry stays behind, let the next writer dup onto the same number and fail with EEXIST, then check that a sentinel opened on the lowest free fd survives the failed writer's deferred close. On the unfixed build the fixture aborts with "panic: assertion failed: err.is_none()". --- src/io/PipeWriter.rs | 7 ++- src/runtime/webcore/FileSink.rs | 2 +- test/js/bun/util/filesink.test.ts | 73 +++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 75880fef3ea3..530101029e60 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -12,7 +12,7 @@ use bun_sys::windows::libuv as uv; // `close`/`set_data`/`ref_` are default trait methods; bring traits into scope // so method resolution finds them on `Pipe`/`uv_tty_t`/`fs_t`. use bun_sys::windows::libuv::UvHandle as _; -use bun_sys::{self as sys, Fd}; +use bun_sys::{self as sys, Fd, FdExt as _}; use crate::{EventLoopHandle, FilePollFlag, FilePollKind, FilePollRef, Owner, PollTag}; @@ -1051,6 +1051,11 @@ impl PosixStreamingWriter { match poll.register_with_fd(event_loop, FilePollKind::Writable, fd) { sys::Result::Err(err) => { + // A fresh poll holds `fd` and closes it with the writer. A poll kept + // from an earlier `start` still holds that fd, so `fd` is closed here. + if poll.fd() != fd { + fd.close(); + } return sys::Result::Err(err); } sys::Result::Ok(()) => {} diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 003df94efc0f..0f16220bdc9a 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -721,7 +721,7 @@ impl FileSink { // SAFETY(JsCell): `start` is pure I/O setup; no JS. match self.writer.with_mut(|w| w.start(fd, self.pollable.get())) { sys::Result::Err(err) => { - // The POSIX writer keeps `fd` in its poll after a failed `start` and closes it on Drop. + // After a failed `start` the POSIX writer has closed `fd` or holds it in its poll. #[cfg(windows)] fd.close(); return sys::Result::Err(err); diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 2409dfd91915..874ece28f3a1 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -923,3 +923,76 @@ it("start() with invalid options throws instead of silently ignoring them", asyn await writer.end(); expect(await Bun.file(join(dir, "start-invalid.txt")).text()).toBe("ok"); }); + +// `Bun.file(fd).writer()` dups the fd and registers the dup for writability. +// When that registration fails, the writer owns the dup and closes it once, +// on the work pool. FileSink::setup used to close it as well. +// Linux only: closing a dup leaves its epoll entry behind while fd 2 keeps the +// pipe open, so the next dup to land on the same number fails with EEXIST. +// On kqueue the knote goes with the fd, so there is no such failure to provoke. +it.skipIf(!isLinux)("a writer whose poll registration fails closes its fd exactly once", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { closeSync, fstatSync, openSync, readdirSync } from "node:fs"; + const stderrInode = fstatSync(2).ino; + // Dups of fd 2 above the stdio range that this script created. + let baseline = new Set(); + const stderrDups = () => + readdirSync("/proc/self/fd") + .map(Number) + .filter(fd => { + if (fd <= 2 || baseline.has(fd)) return false; + try { + return fstatSync(fd).ino === stderrInode; + } catch { + return false; + } + }); + baseline = new Set(stderrDups()); + // Stays alive for the whole run: its poll keeps the epoll entry for the + // dup, and nothing else may reach that fd number through it. + globalThis.keep = Bun.file(2).writer(); + globalThis.keep.write(""); + globalThis.keep.flush(); + const [dup] = stderrDups(); + // fd 2 keeps the pipe open, so the entry for (pipe, dup) stays in epoll. + closeSync(dup); + let code; + try { + // dup(2) lands on the same number again: EPOLL_CTL_ADD fails with EEXIST. + Bun.file(2).writer(); + } catch (e) { + code = e.code; + } + // Lands on the lowest free fd. A second close of the failed writer's dup + // would hit this descriptor. + const sentinel = openSync("/dev/null", "r"); + const sentinelInode = fstatSync(sentinel).ino; + // The failed writer closes its dup on the work pool. + const deadline = Date.now() + 5000; + while (Date.now() < deadline && stderrDups().length > 0) Bun.sleepSync(5); + const remaining = stderrDups().length; + let sentinelIntact; + try { + sentinelIntact = fstatSync(sentinel).ino === sentinelInode; + } catch { + sentinelIntact = false; + } + console.log(JSON.stringify({ code, remaining, sentinelIntact })); + `, + ], + env: bunEnv, + stdout: "pipe", + // A pipe is pollable, so the writers register their dup of fd 2 with epoll. + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ code: "EEXIST", remaining: 0, sentinelIntact: true }) + "\n", + stderr: "", + exitCode: 0, + }); +}); From f7bca3bdde2f788ef18d2990bb3676dcedf30387 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:29:58 +0000 Subject: [PATCH 09/14] test: hold the finalizer fixture's writers through a global A module binding that is only written after the await is dead at the suspension point, so the writers were already collected in the warm-up call and the WeakRef count said nothing about the probe call. The array now hangs off globalThis until the drop right before the probe, and the refs are read in a loop that does not allocate. Verified against an unfixed build with padding before the warm-up: it still spins with collectedDuringCall 3 in every variant. Also restate the spawnSync.test.ts fixture comment as the observed behaviour of -e instead of a claim about reachability. --- test/js/bun/spawn/spawnSync.test.ts | 4 ++- .../spawnsync-isolated-event-loop.test.ts | 27 +++++++++++-------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/test/js/bun/spawn/spawnSync.test.ts b/test/js/bun/spawn/spawnSync.test.ts index 7692a70b453d..c291f68a1a34 100644 --- a/test/js/bun/spawn/spawnSync.test.ts +++ b/test/js/bun/spawn/spawnSync.test.ts @@ -217,7 +217,9 @@ describe("uid/gid", () => { // same number, EPOLL_CTL_ADD fails with EEXIST, and the fd is closed twice: // once by FileSink::setup and once by the writer's Drop through the Closer. test.skipIf(!isLinux)("a writer finalized during spawnSync does not break the next writer on the same fd", async () => { - // A file, not `-e`: under `-e` the leaked writers stay reachable and are never finalized. + // A file, not `-e`: run through `-e`, the collection that `Bun.gc(false)` + // requests does not finalize the writers inside spawnSync (their dups stay + // open for the whole wait), and the test no longer reaches the bug. using dir = tempDir("spawnsync-writer-finalized", { "fixture.js": ` import { fstatSync, readdirSync } from "node:fs"; diff --git a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts index 1bfea7d65070..8c7fdcdb94b5 100644 --- a/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts +++ b/test/js/bun/spawn/spawnsync-isolated-event-loop.test.ts @@ -196,11 +196,12 @@ describe.concurrent("spawnSync isolated event loop", () => { // BUN_JSC_slowPathAllocsBetweenGCs runs a full synchronous GC every few // slow-path allocations. A warmed-up spawnSync allocates nothing before it // installs the isolated loop, so the GC that frees the writers runs inside it. - // The fixture also reports how many writers were collected by the time that - // call returned, so a change that keeps them alive past the call, or stops - // the GC from running there, fails this test instead of passing it. - // (bun:internal-for-testing would give exact counts, but loading it under - // this GC setting takes a debug build 15s.) + // The fixture also reports how many writers were collected by the time the + // probe call returned. They are held by a global until the drop right before + // that call, so a change that keeps them alive past the call, or stops the + // GC from running in it, fails this test instead of passing it. + // (bun:internal-for-testing or node:fs would give fd and sink counts, but + // loading either module under this GC setting costs a debug build 5s to 15s.) test.skipIf(isWindows)("finalizers that run inside spawnSync do not stall the next spawnSync", async () => { await using proc = Bun.spawn({ cmd: [ @@ -208,18 +209,20 @@ describe.concurrent("spawnSync isolated event loop", () => { "-e", ` // Three writers on stderr (a pipe here), each with a poll registered - // on the main loop. - let writers = []; + // on the main loop. They hang off globalThis: a binding that is only + // written after the await below is dead at the suspension, and the + // writers would already be collected in the warm-up call. + globalThis.writers = []; const refs = []; for (let i = 0; i < 3; i++) { const writer = Bun.stderr.writer(); writer.write(""); writer.flush(); - writers.push(writer); + globalThis.writers.push(writer); refs.push(new WeakRef(writer)); } // A WeakRef keeps its target alive until the end of the job that - // created it. Cross a job boundary so only \`writers\` holds them. + // created it. await Bun.sleep(0); const first = { cmd: ["echo", "first"], stdout: "pipe", stderr: "ignore", timeout: 2000 }; // pidfd + stdout + stderr: three polls, the same count the three @@ -230,12 +233,14 @@ describe.concurrent("spawnSync isolated event loop", () => { // Warm up while the writers are still reachable: lazy structures and // Bun.spawnSync itself are created here, on the main loop. Bun.spawnSync(first); - writers = null; + globalThis.writers = null; // The first allocations after the drop are this call's result, built // while the isolated loop is installed: the GC they trigger frees the // writers there. Nothing may run between the drop and the call. Bun.spawnSync(first); - const collectedDuringCall = refs.filter(ref => ref.deref() === undefined).length; + // Read the refs before anything allocates: a GC here would change the answer. + let collectedDuringCall = 0; + for (let i = 0; i < refs.length; i++) if (refs[i].deref() === undefined) collectedDuringCall++; const result = Bun.spawnSync(second); console.log( From 390b22c96b2c8a01aaf6019335e397bcdbd08277 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:32:28 +0000 Subject: [PATCH 10/14] PipeWriter: one-line note on the reused-poll close --- src/io/PipeWriter.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 530101029e60..eeb38135c314 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -1051,8 +1051,7 @@ impl PosixStreamingWriter { match poll.register_with_fd(event_loop, FilePollKind::Writable, fd) { sys::Result::Err(err) => { - // A fresh poll holds `fd` and closes it with the writer. A poll kept - // from an earlier `start` still holds that fd, so `fd` is closed here. + // A poll reused from an earlier start() keeps its own fd, so nothing else closes this one. if poll.fd() != fd { fd.close(); } From e1159c84650488bda77f9a3e401c1fad320029ea Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:53:07 +0000 Subject: [PATCH 11/14] test: keep the closed dup number occupied until the filesink fixture exits The fixture closes a live writer's dup of fd 2 behind its back. On the asan lane the process tears the VM down at exit, so that writer closes its dup number again, hits EBADF, and FdExt::close panics on the work pool ("assertion failed: err.is_none()"). When the work pool is fast the sentinel already sits on that number and nothing fails, which is why a debug build passes. If the number is free once the failed writer's deferred close is done, open /dev/null onto it so the teardown close has a descriptor. --- test/js/bun/util/filesink.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 874ece28f3a1..29e5765ce186 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -981,7 +981,16 @@ it.skipIf(!isLinux)("a writer whose poll registration fails closes its fd exactl } catch { sentinelIntact = false; } - console.log(JSON.stringify({ code, remaining, sentinelIntact })); + // \`keep\` still owns the number \`dup\` and closes it when the process tears + // down. If nothing occupies that number by now, give it a live file so the + // teardown close has a descriptor to close. + let dupOccupied = true; + try { + fstatSync(dup); + } catch { + dupOccupied = openSync("/dev/null", "r") === dup; + } + console.log(JSON.stringify({ code, remaining, sentinelIntact, dupOccupied })); `, ], env: bunEnv, @@ -991,7 +1000,7 @@ it.skipIf(!isLinux)("a writer whose poll registration fails closes its fd exactl }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect({ stdout, stderr, exitCode }).toEqual({ - stdout: JSON.stringify({ code: "EEXIST", remaining: 0, sentinelIntact: true }) + "\n", + stdout: JSON.stringify({ code: "EEXIST", remaining: 0, sentinelIntact: true, dupOccupied: true }) + "\n", stderr: "", exitCode: 0, }); From 2cd5c710be6952cb7891bde5344393125176ca8b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:47:35 +0000 Subject: [PATCH 12/14] test: the double close is gone on main, keep the EEXIST half of the comment --- test/js/bun/spawn/spawnSync.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/js/bun/spawn/spawnSync.test.ts b/test/js/bun/spawn/spawnSync.test.ts index ab8309bdeb0b..ac5040c922f0 100644 --- a/test/js/bun/spawn/spawnSync.test.ts +++ b/test/js/bun/spawn/spawnSync.test.ts @@ -232,8 +232,7 @@ describe("uid/gid", () => { // still unregister from the loop it was registered with. Before the fix the // EPOLL_CTL_DEL went to the private loop, so the main loop kept a stale // entry for the dup'd fd number. The next `Bun.file(2).writer()` dups to the -// same number, EPOLL_CTL_ADD fails with EEXIST, and the fd is closed twice: -// once by FileSink::setup and once by the writer's Drop through the Closer. +// same number and EPOLL_CTL_ADD fails with EEXIST. test.skipIf(!isLinux)("a writer finalized during spawnSync does not break the next writer on the same fd", async () => { // A file, not `-e`: run through `-e`, the collection that `Bun.gc(false)` // requests does not finalize the writers inside spawnSync (their dups stay From 87f1190cdc7e08eeef5578b59a7b2065ff6e5ce0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 30 Sep 2026 03:55:39 +0000 Subject: [PATCH 13/14] Keep the loop bookkeeping of KeepAlive out of line A release build of the previous commit had 66,560 more bytes of .text than its merge base on linux-x64. The loop lookup that `KeepAlive::ref_` and `unref` gained was inlined into each of their several hundred callers. `EventLoopCtx::loop_ref` now returns the spawnSync-loop bit itself, and it and `loop_unref_for` are `#[inline(never)]`. A caller keeps a status check and one call. `FilePoll::register_with_fd` and `unregister_with_fd` resolve the loop once and hand it to the private helpers, which take `&mut Loop` again as on main. --- src/io/keep_alive.rs | 3 +-- src/io/lib.rs | 16 +++++++++++----- src/io/posix_event_loop.rs | 29 ++++++++++++++--------------- src/runtime/timer/mod.rs | 6 ++---- 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/io/keep_alive.rs b/src/io/keep_alive.rs index 2b109fc86759..f6c7dc000381 100644 --- a/src/io/keep_alive.rs +++ b/src/io/keep_alive.rs @@ -76,8 +76,7 @@ impl KeepAlive { return; } self.status = Status::Active; - self.spawn_sync_loop = event_loop_ctx.is_spawn_sync_loop(); - event_loop_ctx.loop_ref(); + self.spawn_sync_loop = event_loop_ctx.loop_ref(); } /// Allow a poll to keep the process alive. diff --git a/src/io/lib.rs b/src/io/lib.rs index c374e28252c5..1ba087166ea5 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -374,7 +374,7 @@ impl EventLoopCtx { } /// True while `Bun.spawnSync` has its isolated loop installed as the current loop. #[inline] - pub fn is_spawn_sync_loop(&self) -> bool { + pub(crate) fn is_spawn_sync_loop(&self) -> bool { self.platform_event_loop_ptr() != bun_uws_sys::Loop::get() } /// The loop a counter was taken on: the isolated `spawnSync` loop when its @@ -393,12 +393,18 @@ impl EventLoopCtx { // SAFETY: the thread's loop; same contract as `loop_mut`. unsafe { &mut *bun_uws_sys::Loop::get() } } - #[inline] - pub fn loop_ref(&self) { + /// Refs the current loop. Returns whether that loop is `Bun.spawnSync`'s + /// isolated loop, which is the bit [`Self::loop_unref_for`] takes. + // Out of line, like `loop_unref_for`: `KeepAlive` inlines into several hundred callers. + #[inline(never)] + #[must_use] + pub fn loop_ref(&self) -> bool { + let spawn_sync_loop = self.is_spawn_sync_loop(); self.loop_mut().ref_(); + spawn_sync_loop } - /// Releases a [`Self::loop_ref`] taken while [`Self::is_spawn_sync_loop`] was `spawn_sync_loop`. - #[inline] + /// Releases a [`Self::loop_ref`] that returned `spawn_sync_loop`. + #[inline(never)] #[cfg(not(windows))] pub fn loop_unref_for(&self, spawn_sync_loop: bool) { self.loop_for(spawn_sync_loop).unref(); diff --git a/src/io/posix_event_loop.rs b/src/io/posix_event_loop.rs index 0129f3102ffb..6ce8f0eb1303 100644 --- a/src/io/posix_event_loop.rs +++ b/src/io/posix_event_loop.rs @@ -480,8 +480,7 @@ impl FilePoll { } /// Only intended to be used from EventLoop.Pollable - fn deactivate(&mut self, vm: EventLoopCtx) { - let loop_ = self.loop_mut(vm); + fn deactivate(&mut self, loop_: &mut Loop) { if self.flags.contains(Flags::HasIncrementedPollCount) { loop_.dec(); } @@ -496,8 +495,7 @@ impl FilePoll { } /// Only intended to be used from EventLoop.Pollable - fn activate(&mut self, vm: EventLoopCtx) { - let loop_ = self.loop_mut(vm); + fn activate(&mut self, loop_: &mut Loop) { self.flags.remove(Flags::Closed); if !self.flags.contains(Flags::HasIncrementedPollCount) { @@ -583,7 +581,7 @@ impl FilePoll { target_os = "macos", target_os = "freebsd" ))] - return self.register_with_fd_impl(vm, flag, one_shot, fd); + return self.register_with_fd_impl(self.loop_mut(vm), flag, one_shot, fd); #[cfg(not(any( target_os = "linux", target_os = "android", @@ -604,12 +602,12 @@ impl FilePoll { ))] fn register_with_fd_impl( &mut self, - vm: EventLoopCtx, + loop_: &mut Loop, flag: Flags, one_shot: OneShotFlag, fd: Fd, ) -> sys::Result<()> { - let watcher_fd = self.loop_mut(vm).fd; + let watcher_fd = loop_.fd; syslog!( "register: FilePoll(0x{:x}, generation_number={}) {} ({})", @@ -670,7 +668,7 @@ impl FilePoll { let ctl = unsafe { linux::epoll_ctl(watcher_fd, op, fd.native(), &raw mut event) }; self.flags.insert(Flags::WasEverRegistered); if let Some(errno) = errno_sys(ctl, sys::Tag::epoll_ctl) { - self.deactivate(vm); + self.deactivate(loop_); return errno; } } @@ -781,7 +779,7 @@ impl FilePoll { let errno = sys::get_errno(rc); if errno != sys::E::SUCCESS { - self.deactivate(vm); + self.deactivate(loop_); return sys::Result::Err(sys::Error::from_code(errno, sys::Tag::kqueue)); } } @@ -845,12 +843,12 @@ impl FilePoll { self.flags.insert(Flags::WasEverRegistered); if let Some(err) = errno_sys(rc, sys::Tag::kevent) { - self.deactivate(vm); + self.deactivate(loop_); return err; } } - self.activate(vm); + self.activate(loop_); self.flags.insert(match flag { Flags::Readable => Flags::PollReadable, Flags::Process => { @@ -883,6 +881,7 @@ impl FilePoll { fd: Fd, force_unregister: bool, ) -> sys::Result<()> { + let loop_ = self.loop_mut(vm); // Note: compute the syscall result first, then unconditionally // deactivate. Avoids a raw-pointer scopeguard. #[cfg(any( @@ -891,7 +890,7 @@ impl FilePoll { target_os = "macos", target_os = "freebsd" ))] - let result = self.unregister_with_fd_impl(vm, fd, force_unregister); + let result = self.unregister_with_fd_impl(loop_, fd, force_unregister); #[cfg(not(any( target_os = "linux", target_os = "android", @@ -902,7 +901,7 @@ impl FilePoll { let _ = (fd, force_unregister); sys::Result::Ok(()) }; - self.deactivate(vm); + self.deactivate(loop_); result } @@ -914,7 +913,7 @@ impl FilePoll { ))] fn unregister_with_fd_impl( &mut self, - vm: EventLoopCtx, + loop_: &mut Loop, fd: Fd, force_unregister: bool, ) -> sys::Result<()> { @@ -931,7 +930,7 @@ impl FilePoll { return sys::Result::Ok(()); } - let watcher_fd = self.loop_mut(vm).fd; + let watcher_fd = loop_.fd; let both_directions = disarmed_only || (self.flags.contains(Flags::PollReadable) && self.flags.contains(Flags::PollWritable)); diff --git a/src/runtime/timer/mod.rs b/src/runtime/timer/mod.rs index 3dda9fadd555..5aa336278acb 100644 --- a/src/runtime/timer/mod.rs +++ b/src/runtime/timer/mod.rs @@ -1102,8 +1102,7 @@ impl All { if old <= 0 && new > 0 { #[cfg(not(windows))] { - self.immediate_refd_spawn_sync_loop = ctx.is_spawn_sync_loop(); - ctx.loop_ref(); + self.immediate_refd_spawn_sync_loop = ctx.loop_ref(); } #[cfg(windows)] { @@ -1148,8 +1147,7 @@ impl All { if old <= 0 && new > 0 { #[cfg(not(windows))] { - self.timer_refd_spawn_sync_loop = ctx.is_spawn_sync_loop(); - ctx.loop_ref(); + self.timer_refd_spawn_sync_loop = ctx.loop_ref(); } // `uv_timer.ref()` is intentionally unconditional (no `data != // null` guard). Invariant: every path that reaches a positive From 2d3f147ea0f7afcd1bf6e61a8a140d2d7d1d4bd6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:01:10 +0000 Subject: [PATCH 14/14] Shorten the comment on EventLoopCtx::loop_ref --- src/io/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/io/lib.rs b/src/io/lib.rs index 1ba087166ea5..278b23411502 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -393,10 +393,8 @@ impl EventLoopCtx { // SAFETY: the thread's loop; same contract as `loop_mut`. unsafe { &mut *bun_uws_sys::Loop::get() } } - /// Refs the current loop. Returns whether that loop is `Bun.spawnSync`'s - /// isolated loop, which is the bit [`Self::loop_unref_for`] takes. - // Out of line, like `loop_unref_for`: `KeepAlive` inlines into several hundred callers. - #[inline(never)] + /// Refs the current loop and returns the bit for [`Self::loop_unref_for`]. + #[inline(never)] // `KeepAlive::ref_` is inlined into several hundred callers. #[must_use] pub fn loop_ref(&self) -> bool { let spawn_sync_loop = self.is_spawn_sync_loop();