diff --git a/src/codegen/bake-codegen.ts b/src/codegen/bake-codegen.ts index b60a3dba613e..4dc08e51c963 100644 --- a/src/codegen/bake-codegen.ts +++ b/src/codegen/bake-codegen.ts @@ -37,7 +37,7 @@ function css(file: string, is_development: boolean): string { stdio: ["ignore", "pipe", "pipe"], }); if (!success) throw new Error(stderr.toString("utf-8")); - return stdout.toString("utf-8"); + return stdout.toString("utf-8").trimEnd(); } async function run() { @@ -53,7 +53,11 @@ async function run() { side: JSON.stringify(side), IS_ERROR_RUNTIME: String(file === "error"), IS_BUN_DEVELOPMENT: String(!!debug), - OVERLAY_CSS: css("../runtime/bake/client/overlay.css", !!debug), + // `define` values must be JS expressions; pass the CSS as an + // explicit string literal instead of relying on the define + // parser's auto-quote recovery for raw non-JSON values. The + // consumer is `declare const OVERLAY_CSS: string`. + OVERLAY_CSS: JSON.stringify(css("../runtime/bake/client/overlay.css", !!debug)), }, minify: { syntax: !debug, diff --git a/src/runtime/node.rs b/src/runtime/node.rs index daac1fc22b02..d7a70e7531d9 100644 --- a/src/runtime/node.rs +++ b/src/runtime/node.rs @@ -43,8 +43,13 @@ pub mod crypto; // codegen (`generated_js2native.rs`) addresses this by its file-stem name. pub use crypto as node_crypto_binding; +// macOS-only: the FSEvents/CoreFoundation backend for `fs.watch`. Gating the +// whole module (rather than `#[allow(dead_code)]` on its entry points) keeps +// the other platforms from compiling CF-specific code at all. +#[cfg(target_os = "macos")] #[path = "node/fs_events.rs"] pub mod fs_events; +#[cfg(target_os = "macos")] pub use fs_events as FSEvents; // Sibling modules node_fs.rs imports by `super::` path. Stat/StatFS/time_like diff --git a/src/runtime/node/fs_events.rs b/src/runtime/node/fs_events.rs index 3632bd62ef1d..eb7a2016c3ac 100644 --- a/src/runtime/node/fs_events.rs +++ b/src/runtime/node/fs_events.rs @@ -1,3 +1,4 @@ +use core::cell::UnsafeCell; use core::ffi::{c_char, c_int, c_long, c_void}; use core::ptr::{self, NonNull}; use core::sync::atomic::{AtomicPtr, Ordering}; @@ -92,12 +93,16 @@ pub(crate) const K_FS_EVENTS_RENAMED: c_int = K_FS_EVENT_STREAM_EVENT_FLAG_ITEM_ | K_FS_EVENT_STREAM_EVENT_FLAG_ITEM_RENAMED; static FSEVENTS_DEFAULT_LOOP_MUTEX: Mutex = Mutex::new(); -// PORTING.md §Global mutable state: written under FSEVENTS_DEFAULT_LOOP_MUTEX, -// read with double-checked-locking. AtomicPtr gives safe load/store; the mutex -// serialises the init/teardown writes (Acquire/Release publishes the pointee). -static FSEVENTS_DEFAULT_LOOP: AtomicPtr = AtomicPtr::new(ptr::null_mut()); +// PORTING.md §Global mutable state: written under FSEVENTS_DEFAULT_LOOP_MUTEX +// on first `watch()`. `OnceLock` publishes the `&'static FSEventsLoop` with a +// Release store; all subsequent readers (including the CF thread closure +// captured in `init()`) see the fully-initialised struct. Never reset: the +// loop lives for the process lifetime; `close_and_wait()` only `shutdown()`s +// it (joins the CF thread and releases CF handles) so no `&mut FSEventsLoop` +// is ever formed after the CF thread starts. +static FSEVENTS_DEFAULT_LOOP: std::sync::OnceLock<&'static FSEventsLoop> = + std::sync::OnceLock::new(); -#[cfg(unix)] fn dlsym(handle: *mut c_void, symbol: &core::ffi::CStr) -> Option { const { assert!(core::mem::size_of::() == core::mem::size_of::<*mut c_void>()) }; // SAFETY: handle is a valid dlopen handle; symbol is NUL-terminated @@ -113,13 +118,6 @@ fn dlsym(handle: *mut c_void, symbol: &core::ffi::CStr) -> Option { // bytemuck/as: fn pointers are not Pod and `as` can't cast data→fn pointers. Some(unsafe { core::mem::transmute_copy::<*mut c_void, T>(&ptr) }) } -#[cfg(not(unix))] -fn dlsym(_handle: *mut c_void, _symbol: &core::ffi::CStr) -> Option { - // FSEvents is macOS-only; CoreFoundation/CoreServices loaders below are - // gated behind `target_os = "macos"`, so this body is unreachable on - // Windows but must still type-check. - None -} // Clone/Copy: bitwise OK — `handle` is a leaked dlopen handle held for the // process lifetime (never dlclosed); the rest are resolved fn pointers. @@ -132,6 +130,7 @@ pub struct CoreFoundation { CFIndex, *const c_void, ) -> CFArrayRef, + pub retain: unsafe extern "C" fn(CFTypeRef) -> CFTypeRef, pub release: unsafe extern "C" fn(CFTypeRef), pub run_loop_add_source: unsafe extern "C" fn(CFRunLoopRef, CFRunLoopSourceRef, CFStringRef), @@ -230,6 +229,8 @@ fn init_core_foundation() -> CoreFoundation { handle: fsevents_cf_handle, array_create: dlsym(fsevents_cf_handle, c"CFArrayCreate") .unwrap_or_else(|| panic!("Cannot Load CoreFoundation")), + retain: dlsym(fsevents_cf_handle, c"CFRetain") + .unwrap_or_else(|| panic!("Cannot Load CoreFoundation")), release: dlsym(fsevents_cf_handle, c"CFRelease") .unwrap_or_else(|| panic!("Cannot Load CoreFoundation")), run_loop_add_source: dlsym(fsevents_cf_handle, c"CFRunLoopAddSource") @@ -288,21 +289,69 @@ fn init_core_services() -> CoreServices { } } +/// Process-global FSEvents dispatch loop. One instance per process, leaked as +/// `&'static FSEventsLoop` on first `watch()` and shared between the JS +/// thread(s) (which call `register_watcher`/`unregister_watcher`/`shutdown`) +/// and the dedicated CoreFoundation thread (which runs `cf_thread_loop` for +/// the process lifetime and dispatches `_schedule`/`_stop`/`_events_cb`). +/// +/// All methods take `&self`; cross-thread scalars (`signal_source`, `loop_`) +/// are `AtomicPtr`, and everything else mutable sits in +/// `UnsafeCell` guarded by `mutex`. That makes +/// `&'static FSEventsLoop: Send` (via `FSEventsLoop: Sync`), so the CF thread +/// closure can capture the reference directly — no raw-pointer smuggling, +/// and no `&mut FSEventsLoop` is ever live on one thread while another +/// thread touches the struct. pub struct FSEventsLoop { - pub signal_source: CFRunLoopSourceRef, - pub mutex: Mutex, - pub loop_: CFRunLoopRef, + /// Created in `init()` *before* the CF thread spawns; read by both + /// threads; released (and nulled) in `shutdown()` *after* `thread.join()`. + signal_source: AtomicPtr, + /// Set (and `CFRetain`ed) by the CF thread once it enters `cf_thread_loop` + /// (Release), read by the JS thread in `enqueue_task_concurrent` (Acquire); + /// released (and nulled) in `shutdown()` *after* `thread.join()` so it + /// outlives the CF thread's pthread-TSD destructor. The `sem` handshake in + /// `init()` orders the first store before any JS-thread read. + loop_: AtomicPtr, + /// Guards `state`. + mutex: Mutex, sem: Semaphore, - pub thread: Option>, - pub tasks: UnboundedQueue, - pub watchers: Vec>>, - pub watcher_count: u32, - pub fsevent_stream: FSEventStreamRef, - pub paths: Option>, - pub cf_paths: CFArrayRef, - pub has_scheduled_watchers: bool, + /// Lock-free MPSC; `push`/`pop_batch` are `&self`. + tasks: UnboundedQueue, + /// JS-thread-only: written once in `init()`, taken once in `shutdown()`. + /// The CF thread never touches this field. + thread: UnsafeCell>>, + /// All remaining mutable state — accessed only while holding `mutex`. + state: UnsafeCell, +} + +struct FSEventsLoopState { + watchers: Vec>>, + watcher_count: u32, + has_scheduled_watchers: bool, + /// CF-thread-only (written in `_schedule`), but `_schedule` already holds + /// `mutex`, so lumping them in here costs nothing and keeps the invariant + /// simple: "touch `state` ⇒ hold `mutex`". + fsevent_stream: FSEventStreamRef, + paths: Option>, + cf_paths: CFArrayRef, } +// SAFETY: +// - `signal_source` / `loop_` are `AtomicPtr` — data-race-free by construction. +// - `mutex`, `sem`, `tasks` are `Sync`. +// - `state` is only accessed while holding `mutex` (every access site takes +// the guard first; this is the same discipline `PathWatcherManager` uses). +// - `thread` is JS-thread-only: written once in `init()` (through +// `UnsafeCell`, *after* the CF thread has been spawned with a capture of +// `&'static self`), and read once in `shutdown()` from the JS thread. The +// CF thread never touches this field, so the unsynchronized write is not +// a data race. +unsafe impl Sync for FSEventsLoop {} +// SAFETY: the CF thread takes ownership of nothing; `Send` is required only so +// `&'static FSEventsLoop: Send` (which follows from `Sync`). Included for +// completeness — no `FSEventsLoop` value is ever moved across threads. +unsafe impl Send for FSEventsLoop {} + pub struct Task { pub ctx: *mut (), pub callback: fn(*mut ()), @@ -316,12 +365,17 @@ impl Task { callback(ctx); } - pub fn new(ctx: &mut T, callback: fn(&mut T)) -> Task { + /// Takes `&'static T` / `fn(&T)`: the only tasks ever enqueued are + /// `FSEventsLoop::_schedule` / `_stop`, both `&self` on the + /// process-lifetime loop. Storing a `&mut`-derived pointer here would + /// be invalidated by the next reborrow at the call site under Stacked + /// Borrows; a shared-`&'static`-derived pointer is stable forever. + pub fn new(ctx: &'static T, callback: fn(&T)) -> Task { Task { - // SAFETY: fn(&mut T) and fn(*mut ()) have identical single-pointer ABI; - // ctx is always a valid &mut T at call time (see run()). - callback: unsafe { bun_ptr::cast_fn_ptr::(callback) }, - ctx: std::ptr::from_mut::(ctx).cast::<()>(), + // SAFETY: fn(&T) and fn(*mut ()) have identical single-pointer ABI; + // ctx is always a valid &T at call time (see run()). + callback: unsafe { bun_ptr::cast_fn_ptr::(callback) }, + ctx: core::ptr::from_ref::(ctx).cast_mut().cast::<()>(), } } } @@ -357,35 +411,57 @@ impl ConcurrentTask { } impl FSEventsLoop { - pub fn cf_thread_loop(&mut self) { + /// Body of the dedicated CoreFoundation thread. `&'static self` because + /// the loop is a leaked process-lifetime singleton and the CF thread is + /// joined in `shutdown()` before exit, so the reference is never dangling + /// while used. + fn cf_thread_loop(&'static self) { bun_core::Output::Source::configure_named_thread(zstr!("CFThreadLoop")); let cf = CoreFoundation::get(); + let signal_source = self.signal_source.load(Ordering::Relaxed); // SAFETY: CF fn pointers loaded via dlsym; signal_source is valid unsafe { - self.loop_ = (cf.run_loop_get_current)(); - - (cf.run_loop_add_source)(self.loop_, self.signal_source, *cf.run_loop_default_mode); + // `CFRunLoopGetCurrent()` follows the Get-rule — we don't own a + // reference. When this thread exits, the pthread TSD destructor + // releases the thread's run loop, so a JS-thread + // `enqueue_task_concurrent()` racing thread exit (between + // `CFRunLoopSourceSignal` and `CFRunLoopWakeUp` on the `_stop` + // enqueue) would pass a freed pointer to `CFRunLoopWakeUp` and + // fault at `CFRuntimeBase._rc` (+0xC). Retain here; `shutdown()` + // releases after `thread.join()` so the run loop outlives every + // JS-thread reader. `CFRunLoopWakeUp` on a stopped-but-alive loop + // is a documented no-op. + let loop_ = (cf.retain)((cf.run_loop_get_current)()); + // Release pairs with the Acquire in `enqueue_task_concurrent`; + // additionally ordered-before any JS-thread read by `sem.post()` + // below → `sem.wait()` in `init()`. + self.loop_.store(loop_, Ordering::Release); + + (cf.run_loop_add_source)(loop_, signal_source, *cf.run_loop_default_mode); self.sem.post(); (cf.run_loop_run)(); - (cf.run_loop_remove_source)(self.loop_, self.signal_source, *cf.run_loop_default_mode); + (cf.run_loop_remove_source)(loop_, signal_source, *cf.run_loop_default_mode); } - - self.loop_ = ptr::null_mut(); + // Leave `self.loop_` set — `shutdown()` releases it after `join()`. + // Nulling it here would reintroduce the race this retain closes + // (JS thread could load null between signal and wake). } - // Runs in CF thread, executed after `enqueueTaskConcurrent()`. Body + // Runs in CF thread, executed after `enqueue_task_concurrent()`. Body // discharges its own preconditions; safe `extern "C" fn` coerces to the // `CFRunLoopSourceContext.perform` fn-pointer slot. extern "C" fn cf_loop_callback(arg: *mut c_void) { if arg.is_null() { return; } - // SAFETY: arg was set to `this: *mut FSEventsLoop` in init() - let this = unsafe { bun_ptr::callback_ctx::(arg) }; + // SAFETY: arg is the `&'static FSEventsLoop` set as `ctx.info` in + // `init()`. Recover as shared — only `tasks.pop_batch()` (lock-free, + // `&self`) is called on it. + let this: &FSEventsLoop = unsafe { &*arg.cast::() }; let concurrent = this.tasks.pop_batch(); let count = concurrent.count; @@ -409,27 +485,42 @@ impl FSEventsLoop { } } - pub fn init() -> Result<*mut FSEventsLoop, bun_core::Error> { - let this = bun_core::heap::into_raw(Box::new(FSEventsLoop { - signal_source: ptr::null_mut(), + pub fn init() -> Result<&'static FSEventsLoop, bun_core::Error> { + // Process-lifetime singleton: leak the allocation and immediately + // reborrow as the canonical shared `&'static` that *everything* — + // the CF thread closure, `FSEVENTS_DEFAULT_LOOP`, watchers, and the + // `ctx.info` pointers handed to CoreFoundation — derives from. One + // borrow ⇒ one Stacked-Borrows tag ⇒ no access through any of + // those aliases can invalidate another. Setup writes below go + // through `AtomicPtr`/`UnsafeCell`, which is exactly what those + // fields are for. + let this: &'static FSEventsLoop = &*bun_core::heap::release(Box::new(FSEventsLoop { + signal_source: AtomicPtr::new(ptr::null_mut()), + loop_: AtomicPtr::new(ptr::null_mut()), mutex: Mutex::new(), - loop_: ptr::null_mut(), sem: Semaphore::default(), - thread: None, tasks: UnboundedQueue::default(), - watchers: Vec::new(), - watcher_count: 0, - fsevent_stream: ptr::null_mut(), - paths: None, - cf_paths: ptr::null_mut(), - has_scheduled_watchers: false, + thread: UnsafeCell::new(None), + state: UnsafeCell::new(FSEventsLoopState { + watchers: Vec::new(), + watcher_count: 0, + has_scheduled_watchers: false, + fsevent_stream: ptr::null_mut(), + paths: None, + cf_paths: ptr::null_mut(), + }), })); let cf = CoreFoundation::get(); let mut ctx = CFRunLoopSourceContext { version: 0, - info: this.cast::(), + // `cf_loop_callback` recovers this as `&FSEventsLoop`. Derived + // from the same shared `&'static` as everything else, so CF's + // later deref cannot invalidate any other alias. + info: core::ptr::from_ref::(this) + .cast_mut() + .cast::(), retain: None, release: None, copy_description: None, @@ -444,33 +535,39 @@ impl FSEventsLoop { let signal_source = unsafe { (cf.run_loop_source_create)(ptr::null_mut(), 0, &raw mut ctx) }; if signal_source.is_null() { + // `this` leaks — CFRunLoopSourceCreate only fails under OOM, at + // which point one struct is noise. (`FSEVENTS_DEFAULT_LOOP` stays + // `None`, so `watch()` retries and leaks again on the next call; + // not bounded, but not reachable outside allocator failure.) return Err(bun_core::err!("FailedToCreateCoreFoudationSourceLoop")); } - - // SAFETY: this is a valid freshly-boxed pointer + this.signal_source.store(signal_source, Ordering::Relaxed); + + // `FSEventsLoop: Sync` ⇒ `&'static FSEventsLoop: Send`, so the + // spawn closure captures `this` directly — no raw-pointer smuggling. + // `Builder` propagates pthread_create failure to JS instead of + // panicking (matches `Linux::init`/`Kqueue::init`). + let handle = std::thread::Builder::new() + .name("CFThreadLoop".into()) + .spawn(move || this.cf_thread_loop()) + .map_err(|_| { + // `this` and `signal_source` leak — thread-spawn failure + // means the process is OOM; one struct is noise. Same + // retry-leak shape as the CF-source failure above. + bun_core::err!("FailedToSpawnFSEventsThread") + })?; + // SAFETY: `thread` is JS-thread-only; the CF thread captured `this` + // above but never accesses this field. unsafe { - (*this).signal_source = signal_source; - // The raw `this` pointer is moved - // into the closure; the FSEventsLoop is heap-allocated and outlives - // the thread (joined in Drop). - let this_addr = this as usize; - (*this).thread = Some( - std::thread::Builder::new() - .name("CFThreadLoop".into()) - .spawn(move || { - // SAFETY: see above — `this` is a valid heap allocation for the thread's lifetime. - (*(this_addr as *mut FSEventsLoop)).cf_thread_loop() - }) - .expect("failed to spawn thread"), - ); - - // sync threads - (*this).sem.wait(); + *this.thread.get() = Some(handle); } + + // sync threads + this.sem.wait(); Ok(this) } - fn enqueue_task_concurrent(&mut self, task: Task) { + fn enqueue_task_concurrent(&self, task: Task) { let cf = CoreFoundation::get(); let concurrent = bun_core::heap::into_raw(Box::new(ConcurrentTask { task: Task { @@ -484,8 +581,20 @@ impl FSEventsLoop { unsafe { ConcurrentTask::from(&mut *concurrent, task, true); self.tasks.push(NonNull::new_unchecked(concurrent)); - (cf.run_loop_source_signal)(self.signal_source); - (cf.run_loop_wake_up)(self.loop_); + } + // Acquire pairs with the CF thread's Release in `cf_thread_loop`; + // additionally ordered-after that store by the `sem` handshake in + // `init()`, so every enqueue sees a non-null run loop. `cf_thread_loop` + // retains the CFRunLoop, so even if the CF thread fully exits between + // the signal and the wake below (processing `_stop` off the `push` + // alone), `loop_` stays alive — `CFRunLoopWakeUp` on a stopped loop + // is a no-op. `shutdown()` releases it after `thread.join()`. + let signal_source = self.signal_source.load(Ordering::Relaxed); + let loop_ = self.loop_.load(Ordering::Acquire); + // SAFETY: CF fn pointers loaded via dlsym; handles valid per above. + unsafe { + (cf.run_loop_source_signal)(signal_source); + (cf.run_loop_wake_up)(loop_); } } @@ -503,20 +612,24 @@ impl FSEventsLoop { let paths_ptr = event_paths as *const *const c_char; // SAFETY: event_paths is a `char **` of length num_events per FSEvents API let paths = unsafe { bun_core::ffi::slice(paths_ptr, num_events) }; - // SAFETY: info was set to self in _schedule() - let loop_ = unsafe { bun_ptr::callback_ctx::(info) }; + // SAFETY: info is the `&'static FSEventsLoop` set as `ctx.info` in + // `_schedule()`. Recover as shared. + let loop_: &FSEventsLoop = unsafe { &*info.cast::() }; // SAFETY: event_flags is an array of length num_events per FSEvents API let event_flags = unsafe { bun_core::ffi::slice(event_flags.cast_const(), num_events) }; - // Hold the mutex for the whole iteration. `unregisterWatcher` on the + // Hold the mutex for the whole iteration. `unregister_watcher` on the // main thread nulls the entry under this same mutex and then the // caller immediately frees the FSEventsWatcher (and its path buffer), // so without this lock we can read `handle.path` / call `handle.emit` - // on freed memory. Holding the lock also prevents `registerWatcher` + // on freed memory. Holding the lock also prevents `register_watcher` // from reallocating the `watchers` buffer mid-iteration. let _guard = loop_.mutex.lock_guard(); + // SAFETY: `state` is `UnsafeCell`; exclusive access is guaranteed by + // holding `mutex` (same pattern as `PathWatcherManager::watchers`). + let state = unsafe { &mut *loop_.state.get() }; - for watcher in loop_.watchers.slice() { + for watcher in state.watchers.slice() { let Some(handle) = *watcher else { continue }; // `handle` is alive while held under the mutex (see comment above); // `BackRef` invariant (pointee outlives holder) holds for this @@ -585,31 +698,31 @@ impl FSEventsLoop { } // Runs on CF Thread - pub fn _schedule(&mut self) { + fn _schedule(&self) { let _guard = self.mutex.lock_guard(); - self.has_scheduled_watchers = false; - let watcher_count = self.watcher_count; - - // Reshaped for borrowck — defer slicing self.watchers until after - // the early-exit checks so the &mut self for fsevent_stream/paths doesn't conflict. + // SAFETY: `state` is `UnsafeCell`; exclusive access is guaranteed by + // holding `mutex` (same pattern as `PathWatcherManager::watchers`). + let state = unsafe { &mut *self.state.get() }; + state.has_scheduled_watchers = false; + let watcher_count = state.watcher_count; let cf = CoreFoundation::get(); let cs = CoreServices::get(); // SAFETY: all CF/CS calls below operate on handles we own unsafe { - if !self.fsevent_stream.is_null() { - let stream = self.fsevent_stream; + if !state.fsevent_stream.is_null() { + let stream = state.fsevent_stream; // Stop emitting events (cs.fs_event_stream_stop)(stream); // Release stream (cs.fs_event_stream_invalidate)(stream); (cs.fs_event_stream_release)(stream); - self.fsevent_stream = ptr::null_mut(); + state.fsevent_stream = ptr::null_mut(); } // clean old paths - if let Some(p) = self.paths.take() { + if let Some(p) = state.paths.take() { for s in p.iter() { if !s.is_null() { (cf.release)(*s); @@ -617,9 +730,9 @@ impl FSEventsLoop { } drop(p); } - if !self.cf_paths.is_null() { - let cfp = self.cf_paths; - self.cf_paths = ptr::null_mut(); + if !state.cf_paths.is_null() { + let cfp = state.cf_paths; + state.cf_paths = ptr::null_mut(); (cf.release)(cfp); } @@ -627,12 +740,10 @@ impl FSEventsLoop { return; } - let watchers = self.watchers.slice(); - let mut paths: Box<[*mut c_void]> = vec![ptr::null_mut(); watcher_count as usize].into_boxed_slice(); let mut count: u32 = 0; - for w in watchers { + for w in state.watchers.slice() { if let Some(watcher) = *w { // SAFETY: watcher alive under mutex; its `path` borrows from the // owning PathWatcher, whose `ZBox` storage is NUL-terminated, so @@ -654,7 +765,11 @@ impl FSEventsLoop { ptr::null(), ); let mut ctx = FSEventStreamContext { - info: std::ptr::from_mut(self).cast::(), + // `_events_cb` recovers this as `&FSEventsLoop`. Same pointer + // as the `&'static` in `FSEVENTS_DEFAULT_LOOP`. + info: core::ptr::from_ref::(self) + .cast_mut() + .cast::(), ..Default::default() }; @@ -705,7 +820,8 @@ impl FSEventsLoop { (cs.fs_event_stream_schedule_with_run_loop)( r#ref, - self.loop_, + // Runs on the CF thread — this is our own run loop. + self.loop_.load(Ordering::Relaxed), *cf.run_loop_default_mode, ); if (cs.fs_event_stream_start)(r#ref) == 0 { @@ -721,104 +837,130 @@ impl FSEventsLoop { (cs.fs_event_stream_release)(r#ref); return; } - self.fsevent_stream = r#ref; - self.paths = Some(paths); - self.cf_paths = cf_paths; + state.fsevent_stream = r#ref; + state.paths = Some(paths); + state.cf_paths = cf_paths; } } - fn register_watcher(&mut self, watcher: *mut FSEventsWatcher) { - { - let _guard = self.mutex.lock_guard(); - if self.watcher_count as usize == self.watchers.len() { - self.watcher_count += 1; - self.watchers.push(NonNull::new(watcher)); - } else { - let watchers = self.watchers.slice_mut(); - for (i, w) in watchers.iter_mut().enumerate() { - let _ = i; - if w.is_none() { - *w = NonNull::new(watcher); - self.watcher_count += 1; - break; - } + fn register_watcher(&'static self, watcher: *mut FSEventsWatcher) { + let _guard = self.mutex.lock_guard(); + // SAFETY: `state` is `UnsafeCell`; exclusive access is guaranteed by + // holding `mutex` (same pattern as `PathWatcherManager::watchers`). + let state = unsafe { &mut *self.state.get() }; + if state.watcher_count as usize == state.watchers.len() { + state.watcher_count += 1; + state.watchers.push(NonNull::new(watcher)); + } else { + for w in state.watchers.slice_mut() { + if w.is_none() { + *w = NonNull::new(watcher); + state.watcher_count += 1; + break; } } + } - if !self.has_scheduled_watchers { - self.has_scheduled_watchers = true; - } else { - return; - } + if !state.has_scheduled_watchers { + state.has_scheduled_watchers = true; + } else { + return; } - // Enqueue after dropping the guard so we can take &mut self twice; - // safe to release first since enqueue only pushes to a lock-free queue and - // signals CF, and `_schedule` re-acquires the mutex on the CF thread. - let task = Task::new(self, FSEventsLoop::_schedule); - self.enqueue_task_concurrent(task); + // Holding the lock through the enqueue keeps the schedule flag and the + // queued task atomic; `enqueue_task_concurrent` is `&self`, so there is + // no borrow conflict with the guard. + self.enqueue_task_concurrent(Task::new(self, FSEventsLoop::_schedule)); } - fn unregister_watcher(&mut self, watcher: *mut FSEventsWatcher) { - { - let _guard = self.mutex.lock_guard(); - // Reshaped for borrowck — capture len before mutable iteration - let len = self.watchers.len() as usize; - let watchers = self.watchers.slice_mut(); - for i in 0..len { - if let Some(item) = watchers[i] { - if item.as_ptr() == watcher { - watchers[i] = None; - // if is the last one just pop - if i == len - 1 { - let _ = self.watchers.pop(); - } - self.watcher_count -= 1; - break; + fn unregister_watcher(&'static self, watcher: *mut FSEventsWatcher) { + let _guard = self.mutex.lock_guard(); + // SAFETY: `state` is `UnsafeCell`; exclusive access is guaranteed by + // holding `mutex` (same pattern as `PathWatcherManager::watchers`). + let state = unsafe { &mut *self.state.get() }; + let len = state.watchers.len() as usize; + for i in 0..len { + if let Some(item) = state.watchers.slice_mut()[i] { + if item.as_ptr() == watcher { + state.watchers.slice_mut()[i] = None; + // if is the last one just pop + if i == len - 1 { + let _ = state.watchers.pop(); } + state.watcher_count -= 1; + break; } } + } - // Rebuild the FSEventStream on the CF thread so it stops firing for - // the path we just removed. Without this the stream keeps delivering - // events for freed paths until another register happens to - // reschedule. `_events_cb` tolerates the interim (it sees `null` and - // skips) because both sides hold `this.mutex`. - if !self.has_scheduled_watchers { - self.has_scheduled_watchers = true; - } else { - return; - } + // Rebuild the FSEventStream on the CF thread so it stops firing for + // the path we just removed. Without this the stream keeps delivering + // events for freed paths until another register happens to + // reschedule. `_events_cb` tolerates the interim (it sees `null` and + // skips) because both sides hold `this.mutex`. + if !state.has_scheduled_watchers { + state.has_scheduled_watchers = true; + } else { + return; } - // Reshaped for borrowck — see register_watcher - let task = Task::new(self, FSEventsLoop::_schedule); - self.enqueue_task_concurrent(task); + self.enqueue_task_concurrent(Task::new(self, FSEventsLoop::_schedule)); } // Runs on CF loop to close the loop - fn _stop(&mut self) { + fn _stop(&self) { let cf = CoreFoundation::get(); - // SAFETY: self.loop_ is the CF thread's current run loop - unsafe { (cf.run_loop_stop)(self.loop_) }; + // SAFETY: runs on the CF thread — this is our own run loop. + unsafe { (cf.run_loop_stop)(self.loop_.load(Ordering::Relaxed)) }; } -} -impl Drop for FSEventsLoop { - fn drop(&mut self) { + /// Called from `close_and_wait()` at process exit. Not a `Drop` impl + /// because `&mut self` there would alias the CF thread's + /// `&'static FSEventsLoop` until `join()` returns. The allocation is + /// `&'static` (leaked in `init()`), so there is nothing to free; this + /// only joins the CF thread and releases CF handles. + /// + /// `FSEVENTS_DEFAULT_LOOP` is a `OnceLock` and cannot be cleared, so a + /// `watch()` after shutdown would reach a dead loop — but + /// `close_and_wait` runs from `Bun__onExit` after the VM has stopped + /// scheduling JS, so no new `watch()` calls are possible. + /// `close_and_wait` serializes calls under `FSEVENTS_DEFAULT_LOOP_MUTEX`, + /// and this is idempotent under that lock: `thread.take()` returns `None` + /// on a repeat call and we bail before touching CF. + fn shutdown(&'static self) { + // SAFETY: `thread` is JS-thread-only; `shutdown()` runs from + // `close_and_wait()` on the JS thread at exit under + // `FSEVENTS_DEFAULT_LOOP_MUTEX`. No other access exists after + // `init()` returns. + let Some(thread) = (unsafe { (*self.thread.get()).take() }) else { + return; // already shut down + }; // signal close and wait - // Reshaped for borrowck — build Task (stores raw ptr) before re-borrowing &mut self - let stop_task = Task::new(self, FSEventsLoop::_stop); - self.enqueue_task_concurrent(stop_task); - if let Some(thread) = self.thread.take() { - let _ = thread.join(); - } - let cf = CoreFoundation::get(); + self.enqueue_task_concurrent(Task::new(self, FSEventsLoop::_stop)); + let _ = thread.join(); + let cf = CoreFoundation::get(); + // `cf_thread_loop` retained the run loop so it outlives the CF thread + // (whose pthread TSD destructor would otherwise free it between + // `CFRunLoopSourceSignal` and `CFRunLoopWakeUp` in the `_stop` enqueue + // above). The thread has now joined; drop our reference. + let loop_ = self.loop_.swap(ptr::null_mut(), Ordering::Relaxed); + debug_assert!(!loop_.is_null()); + // SAFETY: retained in `cf_thread_loop`; sole owner after join. + unsafe { (cf.release)(loop_) }; + + let signal_source = self.signal_source.swap(ptr::null_mut(), Ordering::Relaxed); + debug_assert!(!signal_source.is_null()); // SAFETY: signal_source is a valid CF object until released here - unsafe { (cf.release)(self.signal_source) }; - self.signal_source = ptr::null_mut(); + unsafe { (cf.release)(signal_source) }; - if self.watcher_count > 0 { - while let Some(watcher) = self.watchers.pop() { + // CF thread has exited; we are the sole accessor of `state`. Take the + // mutex anyway to keep the "touch `state` ⇒ hold `mutex`" invariant + // uniform. + let _guard = self.mutex.lock_guard(); + // SAFETY: `state` is `UnsafeCell`; exclusive access is guaranteed by + // holding `mutex` (same pattern as `PathWatcherManager::watchers`). + let state = unsafe { &mut *self.state.get() }; + if state.watcher_count > 0 { + while let Some(watcher) = state.watchers.pop() { if let Some(w) = watcher { // `w` is a registered, not-yet-freed watcher; `BackRef` // invariant holds. `loop_` is a `Cell`, so the write goes @@ -827,8 +969,6 @@ impl Drop for FSEventsLoop { } } } - - // Vec storage freed by its own Drop (or explicit deinit) } } @@ -842,13 +982,11 @@ pub struct FSEventsWatcher { pub path: bun_ptr::RawSlice, pub callback: Callback, pub flush_callback: UpdateEndCallback, - // Stored as a raw pointer because the loop is - // shared with the CFRunLoop thread and mutated through `unregister_watcher` - // on drop; holding a `&'static FSEventsLoop` and casting it to `*mut` would - // be UB (write through pointer derived from shared ref). `Cell` so - // `FSEventsLoop::drop` can null it through a shared `BackRef` (the watcher - // is otherwise only read via `&self` on the CF thread under the mutex). - pub loop_: core::cell::Cell>>, + /// `Cell` so `FSEventsLoop::shutdown` can null + /// it through a shared `BackRef` (the watcher is otherwise only read via + /// `&self` on the CF thread under the mutex). The loop itself is + /// `&'static`, so no raw pointer needed. + pub loop_: core::cell::Cell>, pub recursive: bool, pub ctx: *mut c_void, } @@ -857,13 +995,8 @@ pub type Callback = fn(ctx: *mut c_void, event: Event, is_file: bool); pub(crate) type UpdateEndCallback = fn(ctx: *mut c_void); impl FSEventsWatcher { - /// # Safety - /// `loop_` must point to a valid, live `FSEventsLoop` (the heap-allocated - /// global default loop from `FSEventsLoop::init`) for the lifetime of the - /// returned watcher; mutable access to its watcher list is serialized by - /// `loop_.mutex` inside `register_watcher`. pub(crate) fn init( - loop_: NonNull, + loop_: &'static FSEventsLoop, path: &[u8], recursive: bool, callback: Callback, @@ -879,8 +1012,7 @@ impl FSEventsWatcher { ctx, }); - // SAFETY: caller contract — see `# Safety` above. - unsafe { (*loop_.as_ptr()).register_watcher(&raw mut *this) }; + loop_.register_watcher(&raw mut *this); this } @@ -896,14 +1028,7 @@ impl FSEventsWatcher { impl Drop for FSEventsWatcher { fn drop(&mut self) { if let Some(loop_) = self.loop_.get() { - // SAFETY: `loop_` is the heap-allocated global default loop (see - // FSEventsLoop::init); it outlives every watcher, and is only set to - // None here by FSEventsLoop::drop *after* draining watchers. Mutable - // access to the watcher list is serialized by `self.mutex` inside - // unregister_watcher. - unsafe { - (*loop_.as_ptr()).unregister_watcher(std::ptr::from_mut(self)); - } + loop_.unregister_watcher(std::ptr::from_mut(self)); } } } @@ -915,28 +1040,28 @@ pub fn watch( update_end: UpdateEndCallback, ctx: *mut c_void, ) -> Result, bun_core::Error> { - let loop_ = FSEVENTS_DEFAULT_LOOP.load(Ordering::Acquire); - if let Some(loop_) = NonNull::new(loop_) { - // SAFETY: `loop_` is the heap-allocated global default loop published - // under `FSEVENTS_DEFAULT_LOOP_MUTEX`; valid for the program lifetime. + // Unlocked fast path — `OnceLock::get` is an Acquire load. + if let Some(&loop_) = FSEVENTS_DEFAULT_LOOP.get() { return Ok(FSEventsWatcher::init( loop_, path, recursive, callback, update_end, ctx, )); } let _guard = FSEVENTS_DEFAULT_LOOP_MUTEX.lock_guard(); - let mut loop_ = FSEVENTS_DEFAULT_LOOP.load(Ordering::Acquire); - if loop_.is_null() { - loop_ = FSEventsLoop::init()?; - FSEVENTS_DEFAULT_LOOP.store(loop_, Ordering::Release); - // First loop ever created → arrange `close_and_wait` to run from - // `Bun__onExit`, which runs it BEFORE - // `runExitCallbacks()`, so push to the pre-exit list rather than - // the generic atexit list (storage lives in bun_core; forward dep). - bun_core::Global::add_pre_exit_callback(close_and_wait_on_exit); - } - // SAFETY: `loop_` is the heap-allocated global default loop (just created or - // re-read under the mutex); valid for the program lifetime. - let loop_ = NonNull::new(loop_).expect("FSEventsLoop::init returned non-null"); + let loop_: &'static FSEventsLoop = match FSEVENTS_DEFAULT_LOOP.get() { + Some(&l) => l, + None => { + let l = FSEventsLoop::init()?; + // Holding FSEVENTS_DEFAULT_LOOP_MUTEX with `.get()` having + // returned `None` above, so this is the first publish. + let _ = FSEVENTS_DEFAULT_LOOP.set(l); + // First loop ever created → arrange `close_and_wait` to run from + // `Bun__onExit`, which runs it BEFORE `run_exit_callbacks()`, so + // push to the pre-exit list rather than the generic atexit list + // (storage lives in bun_core; forward dep). + bun_core::Global::add_pre_exit_callback(close_and_wait_on_exit); + l + } + }; Ok(FSEventsWatcher::init( loop_, path, recursive, callback, update_end, ctx, )) @@ -948,19 +1073,8 @@ extern "C" fn close_and_wait_on_exit() { } pub(crate) fn close_and_wait() { - #[cfg(not(target_os = "macos"))] - { - return; - } - - #[cfg(target_os = "macos")] - { - let loop_ = FSEVENTS_DEFAULT_LOOP.load(Ordering::Acquire); - if !loop_.is_null() { - let _guard = FSEVENTS_DEFAULT_LOOP_MUTEX.lock_guard(); - // SAFETY: loop_ was heap-allocated in FSEventsLoop::init(); reconstitute to run Drop - unsafe { drop(bun_core::heap::take(loop_)) }; - FSEVENTS_DEFAULT_LOOP.store(ptr::null_mut(), Ordering::Release); - } + if let Some(&loop_) = FSEVENTS_DEFAULT_LOOP.get() { + let _guard = FSEVENTS_DEFAULT_LOOP_MUTEX.lock_guard(); + loop_.shutdown(); } } diff --git a/src/runtime/node/path_watcher.rs b/src/runtime/node/path_watcher.rs index 9e33639dbba3..d96eec4f0679 100644 --- a/src/runtime/node/path_watcher.rs +++ b/src/runtime/node/path_watcher.rs @@ -159,26 +159,27 @@ impl PathWatcherManager { return Ok(m); } - // Process-lifetime singleton. Hand the allocation off via - // `heap::release`; it is published into - // `DEFAULT_MANAGER` below and lives until process exit — except on the - // `Platform::init` error path, which is the one place it is reclaimed. - let m: &'static mut PathWatcherManager = - bun_core::heap::release(Box::new(PathWatcherManager::default())); - if let Err(e) = Platform::init(m) { - // SAFETY: `m` came from `release(Box::new(..))` above and has not - // been published — reclaim it so the failed init isn't a leak. - unsafe { - drop(bun_core::heap::take( - std::ptr::from_mut::(m), - )) - }; - return Err(e); - } + // Fallible platform setup (inotify_init1 / kqueue) happens inside + // `Platform::init` *before* it leaks the manager, so a persistent + // OS failure (EMFILE, ENOMEM) retried on every `fs.watch()` doesn't + // accumulate leaked managers. + let m = Platform::init()?; // Holding DEFAULT_MANAGER_MUTEX with `.get()` having returned `None` // above, so this is the first publish; `set` cannot fail. - let _ = DEFAULT_MANAGER.set(&*m); - Ok(&*m) + let _ = DEFAULT_MANAGER.set(m); + Ok(m) + } + + /// Leak the process-lifetime singleton and + /// return the canonical shared `&'static`. Every downstream reference + /// (the reader thread, `DEFAULT_MANAGER`, callers) derives from this one + /// borrow — re-deriving a second `&'static` from a retained + /// `&'static mut` later would pop the first under Stacked Borrows. + /// Called from `Platform::init` *after* its fallible syscall succeeds, + /// so the common retry path never leaks. + #[cfg(not(windows))] // `WindowsStub::init` errors before allocating. + fn leak() -> &'static PathWatcherManager { + &*bun_core::heap::release(Box::new(PathWatcherManager::default())) } /// Build the dedup key into `buf`. Not null-terminated; only used as a hashmap key. @@ -288,7 +289,7 @@ impl PathWatcher { } /// Called from the platform reader thread with `manager.mutex` held. - /// `rel_path` is borrowed — `onPathUpdatePosix` dupes it before enqueuing. + /// `rel_path` is borrowed — `on_path_update_posix` dupes it before enqueuing. #[cfg(not(windows))] fn emit(&mut self, event_type: EventType, rel_path: &[u8], is_file: bool) { let timestamp = bun_core::time::milli_timestamp(); @@ -328,11 +329,11 @@ impl PathWatcher { /// Worker cannot observe a zero-handler PathWatcher still present in the dedup map. /// /// On macOS the FSEvents unregister happens *after* releasing `manager.mutex`: - /// `FSEventsWatcher.deinit()` takes the FSEvents loop mutex, and the CF thread's - /// `_events_cb` holds that mutex while calling into `onFSEvent` (which takes - /// `manager.mutex`). Holding both here would be AB/BA with the CF thread. Once - /// `fse.deinit()` returns, `_events_cb` has released the loop mutex and nulled our - /// slot, so no further callbacks will fire and `destroy()` is safe. + /// dropping the `FSEventsWatcher` takes the FSEvents loop mutex, and the CF + /// thread's `_events_cb` holds that mutex while calling into `on_fs_event` (which + /// takes `manager.mutex`). Holding both here would be AB/BA with the CF thread. + /// Once `remove_watch()` returns, `_events_cb` has released the loop mutex and + /// nulled our slot, so no further callbacks will fire and `destroy()` is safe. /// /// # Safety /// `this` must be a live `PathWatcher` produced by [`PathWatcher::new`] whose @@ -367,7 +368,7 @@ impl PathWatcher { // SAFETY: holding manager.mutex; the reader/CF threads only form their own // `&mut PathWatcher` while holding this lock, so ours is exclusive. Scope // `w` so its last use is before `unlock()` (NLL ends the borrow there) — - // on macOS the tail below must not hold a `&mut` across `fse.deinit()`. + // on macOS the tail below must not hold a `&mut` across `remove_watch()`. let w = unsafe { &mut *this }; w.handlers.swap_remove(&ctx); if w.handlers.len() > 0 { @@ -389,7 +390,7 @@ impl PathWatcher { { // Takes fsevents_loop.mutex; must not hold manager.mutex (see doc comment). // Pass the raw pointer: the CF thread (holding the FSEvents loop mutex - // that `deinit` is about to block on) may concurrently take + // that `remove_watch` is about to block on) may concurrently take // `manager.mutex`, raw-read `(*this).manager`, observe `None`, and bail // — so no `&mut PathWatcher` may be live across that call. Platform::remove_watch(manager, this); @@ -435,8 +436,8 @@ pub fn watch( // // Open with O_PATH|O_DIRECTORY first and retry without O_DIRECTORY on ENOTDIR — // that tells us file-vs-dir without a separate stat, follows symlinks, and the - // resulting fd feeds `getFdPath` for the realpath. One or two syscalls instead - // of lstat + open + (stat) in the old code. `O.PATH` is 0 on macOS (degrades to + // resulting fd feeds `get_fd_path` for the realpath. One or two syscalls instead + // of lstat + open + (stat) in the old code. `O::PATH` is 0 on macOS (degrades to // O_RDONLY, which is what F_GETPATH needs anyway). let mut resolve_buf = path::path_buffer_pool::get(); let mut is_file = false; @@ -499,11 +500,11 @@ pub fn watch( unsafe { handle_oom((*watcher).handlers.put(ctx, ChangeEvent::default())) }; handle_oom(watchers.put(key, watcher)); - // Linux/FreeBSD: `addWatch` mutates the platform dispatch maps (wd_map/entries) + // Linux/FreeBSD: `add_watch` mutates the platform dispatch maps (wd_map/entries) // which live under `manager.mutex`, so call it while still locked. // - // macOS: `addWatch` calls `FSEvents.watch()` which takes the FSEvents loop mutex. - // The CF thread holds that mutex while calling `onFSEvent`, which in turn takes + // macOS: `add_watch` calls `FSEvents::watch()` which takes the FSEvents loop mutex. + // The CF thread holds that mutex while calling `on_fs_event`, which in turn takes // `manager.mutex`. To keep lock order one-way (fsevents → manager), release ours // first. Another Worker's `watch()` finding this PathWatcher in the interim is // fine — it just appends a handler; events won't deliver until the FSEventStream @@ -521,7 +522,7 @@ pub fn watch( (*watcher).manager = None; PathWatcher::destroy(watcher); } - // `Linux.addOne` builds the error with `.path = watcher.path`, which we + // `Linux::add_one` builds the error with `.path = watcher.path`, which we // just freed; strip it like every other return in this function. return Err(err.without_path()); } @@ -541,7 +542,7 @@ pub fn watch( // handler and already returned `watcher` to its caller. Only destroy if // ours was the last handler; otherwise surface the error to the survivors // and leave `watcher.manager` set so their `detach()` takes the locked path - // (→ `unlinkWatcherLocked` no-ops, `removeWatch` no-ops on null `fsevents`, + // (→ `unlink_watcher_locked` no-ops, `remove_watch` no-ops on `None` `fsevents`, // then frees). Never free memory another thread holds. manager.mutex.lock(); manager.unlink_watcher_locked(watcher); @@ -715,27 +716,30 @@ mod inotify_masks { #[cfg(any(target_os = "linux", target_os = "android"))] impl Linux { - fn init(manager: &mut PathWatcherManager) -> sys::Result<()> { + fn init() -> sys::Result<&'static PathWatcherManager> { use bun_sys::linux::IN; + // Fallible syscall first — if this fails we haven't allocated, so + // the retry path in `PathWatcherManager::get()` doesn't leak. let rc = sys::linux::inotify_init1(IN::CLOEXEC); if rc < 0 { return Err(sys::Error::from_code_int(sys::last_errno(), Tag::watch)); } + let manager = PathWatcherManager::leak(); manager.platform_fd.set(Fd::from_native(rc)); // The manager is process-global and never torn down, so the reader thread is // a daemon — detach it instead of stashing a handle we'd never join. - let mgr_ptr = std::ptr::from_mut::(manager) as usize; - match std::thread::Builder::new().spawn(move || { - // SAFETY: manager is process-global (&'static), never freed. - Linux::thread_main(unsafe { &*(mgr_ptr as *const PathWatcherManager) }) - }) { + // `PathWatcherManager: Sync` ⇒ `&'static PathWatcherManager: Send`, so the + // closure captures the shared reference directly — no raw-pointer games. + match std::thread::Builder::new().spawn(move || Linux::thread_main(manager)) { Ok(handle) => drop(handle), // detach Err(_) => { + // `manager` leaks — thread-spawn failure means the process + // is OOM; one struct is the least of its problems. manager.platform_fd.get().close(); return Err(sys::Error::from_code(E::ENOMEM, Tag::watch)); } } - Ok(()) + Ok(manager) } /// Caller holds `manager.mutex`. @@ -784,8 +788,8 @@ impl Linux { // - a subdirectory was *renamed* within the tree: IN_MOVED_TO re-adds it, // inotify returns the same wd (it watches by inode), and the cached subpath // is now stale. Overwrite so later events under the moved dir report the - // new name. `walkAndAdd` never follows symlinks (`entry.kind == .directory`, - // not `.sym_link`), so this can't pick a longer alias via a cycle. + // new name. `walk_and_add` never follows symlinks (`EntryKind::Directory`, + // not `EntryKind::SymLink`), so this can't pick a longer alias via a cycle. for o in owners.iter_mut() { if core::ptr::eq(o.watcher, watcher) { if !strings::eql(o.subpath.as_bytes(), subpath) { @@ -968,7 +972,7 @@ impl Linux { }; // Dispatch to every owner of this wd. The recursive branch below calls - // `addOne`/`walkAndAdd`, which insert into `wd_map` via `getOrPut` and + // `add_one`/`walk_and_add`, which insert into `wd_map` via `entry()` and // may rehash — that would invalidate any pointer into the map's value // storage. Re-fetch the owners list by key each iteration rather than // caching `getPtr(wd)` across the loop. @@ -1085,7 +1089,7 @@ use bun_watcher::inotify_watcher::Event as InotifyEvent; /// macOS: delegate to `fs_events.rs`, which already runs one CFRunLoop thread with /// one FSEventStream covering every watched path. The PathWatcher itself is the -/// FSEventsWatcher's opaque ctx — `fs_events.rs` calls back via `onFSEvent` below, +/// FSEventsWatcher's opaque ctx — `fs_events.rs` calls back via `on_fs_event` below, /// and we fan out to the JS handlers. /// /// Unlike the old design, FSEvents is used for both files and directories (same as @@ -1115,12 +1119,14 @@ impl Drop for DarwinWatch { #[cfg(target_os = "macos")] impl Darwin { - fn init(_: &mut PathWatcherManager) -> sys::Result<()> { - Ok(()) + fn init() -> sys::Result<&'static PathWatcherManager> { + // No manager-level fallible setup on macOS — FSEvents owns its own + // thread via `fs_events.rs`, created lazily on first `add_watch`. + Ok(PathWatcherManager::leak()) } /// Caller does NOT hold `manager.mutex` — `FSEvents.watch()` takes the FSEvents - /// loop mutex, and the CF thread holds that while calling `onFSEvent` (which + /// loop mutex, and the CF thread holds that while calling `on_fs_event` (which /// takes `manager.mutex`). Keeping this call outside `manager.mutex` makes the /// lock order one-way: fsevents_loop.mutex → manager.mutex. fn add_watch(_: &'static PathWatcherManager, watcher: &mut PathWatcher) -> sys::Result<()> { @@ -1151,13 +1157,13 @@ impl Darwin { } } - /// Caller does NOT hold `manager.mutex` (same lock-order reasoning as `addWatch`). - /// `FSEventsWatcher.deinit()` → `unregisterWatcher()` blocks on the FSEvents loop + /// Caller does NOT hold `manager.mutex` (same lock-order reasoning as `add_watch`). + /// `FSEventsWatcher::drop` → `unregister_watcher()` blocks on the FSEvents loop /// mutex, which `_events_cb` holds for the whole dispatch; once this returns no - /// further `onFSEvent` calls will arrive for `watcher`. + /// further `on_fs_event` calls will arrive for `watcher`. /// /// Takes a raw `*mut PathWatcher`: while we block on the FSEvents loop mutex - /// inside `deinit`, the CF thread may concurrently take `manager.mutex` and + /// inside the drop, the CF thread may concurrently take `manager.mutex` and /// raw-read `(*watcher).manager` (to bail on `None`). Holding a `&mut PathWatcher` /// across that would be aliased-`&mut` UB under Stacked Borrows. fn remove_watch(_: &'static PathWatcherManager, watcher: *mut PathWatcher) { @@ -1178,10 +1184,10 @@ impl Darwin { /// `manager.mutex` across a call into FSEvents, so this is deadlock-free. /// /// `watcher` itself is kept alive by the FSEvents loop mutex: `detach()` → - /// `removeWatch()` → `fse.deinit()` → `unregisterWatcher()` blocks until - /// `_events_cb` releases it, so `destroy()` cannot run under us. The - /// `watcher.manager == null` check catches the window where detach has already - /// unlinked us but hasn't yet called `fse.deinit()`. + /// `remove_watch()` → `FSEventsWatcher::drop` → `unregister_watcher()` blocks + /// until `_events_cb` releases it, so `destroy()` cannot run under us. The + /// `watcher.manager.is_none()` check catches the window where detach has already + /// unlinked us but hasn't yet run `remove_watch()`. fn on_fs_event(ctx: *mut c_void, event: Event, is_file: bool) { // SAFETY: ctx is the *mut PathWatcher passed in add_watch above. Keep it raw // until `manager.mutex` is held and the `manager.is_none()` bail-out has run: @@ -1276,25 +1282,25 @@ impl PathWatcherManager { #[cfg(target_os = "freebsd")] impl Kqueue { - fn init(manager: &mut PathWatcherManager) -> sys::Result<()> { - let kq = match sys::kqueue() { - Ok(f) => f, - Err(e) => return Err(e), - }; + fn init() -> sys::Result<&'static PathWatcherManager> { + // Fallible syscall first — if this fails we haven't allocated, so + // the retry path in `PathWatcherManager::get()` doesn't leak. + let kq = sys::kqueue()?; + let manager = PathWatcherManager::leak(); manager.platform_fd.set(kq); // Daemon reader — the manager is process-global and never torn down. - let mgr_ptr = manager as *mut PathWatcherManager as usize; - match std::thread::Builder::new().spawn(move || { - // SAFETY: manager is process-global (&'static), never freed. - Kqueue::thread_main(unsafe { &*(mgr_ptr as *const PathWatcherManager) }) - }) { + // `PathWatcherManager: Sync` ⇒ `&'static PathWatcherManager: Send`, so the + // closure captures the shared reference directly — no raw-pointer games. + match std::thread::Builder::new().spawn(move || Kqueue::thread_main(manager)) { Ok(handle) => drop(handle), // detach Err(_) => { + // `manager` leaks — thread-spawn failure means the process + // is OOM; one struct is the least of its problems. manager.platform_fd.get().close(); return Err(sys::Error::from_code(E::ENOMEM, Tag::watch)); } } - Ok(()) + Ok(manager) } /// Caller holds `manager.mutex`. @@ -1451,7 +1457,7 @@ impl Kqueue { for kev in &events[..count as usize] { // Validate via the map — the entry may have been freed by a racing - // removeWatch between kevent() returning and us taking the lock. POSIX + // remove_watch between kevent() returning and us taking the lock. POSIX // recycles the lowest fd on open(), so the ident could also now belong // to an *unrelated* watch registered in that same window; `udata` was // set to a monotonic generation at registration and survives in the @@ -1513,7 +1519,7 @@ pub(crate) struct WindowsStub {} #[cfg(windows)] impl WindowsStub { - fn init(_: &mut PathWatcherManager) -> sys::Result<()> { + fn init() -> sys::Result<&'static PathWatcherManager> { Err(sys::Error::from_code(E::ENOTSUP, Tag::watch)) } fn add_watch(_: &'static PathWatcherManager, _: &mut PathWatcher) -> sys::Result<()> { diff --git a/test/js/node/watch/fs.watch.close-exit.test.ts b/test/js/node/watch/fs.watch.close-exit.test.ts new file mode 100644 index 000000000000..63adaa7e2e02 --- /dev/null +++ b/test/js/node/watch/fs.watch.close-exit.test.ts @@ -0,0 +1,98 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// Regression test for pointer-provenance UB in the fs.watch backends +// introduced by the Rust port. +// +// `FSEventsLoop::init()` spawned the CoreFoundation thread by laundering +// `*mut FSEventsLoop` through `usize` (`this as usize` → `addr as *mut _`) to +// satisfy `Send` on the closure. That strips provenance: the CF thread's +// writes to `self.loop_` become disconnected from the JS thread's reads. +// Compounding this, `cf_thread_loop` took `&mut self` and held it across +// `CFRunLoopRun()`, so the JS thread's `&mut FSEventsLoop` in +// `register_watcher`/`unregister_watcher`/`Drop` aliased it — two live +// `&mut` to one allocation is UB regardless of synchronization. Under +// `noalias` LLVM is free to treat the CF thread's +// `self.loop_ = CFRunLoopGetCurrent()` as invisible, so the JS thread's +// `enqueue_task_concurrent` reads a stale `NULL` and calls +// `CFRunLoopWakeUp(NULL)`, faulting inside CoreFoundation at +0xC. +// +// The same `usize` round-trip existed in the Linux inotify and FreeBSD +// kqueue reader-thread spawns; fixed together. +// +// Field report: test/js/node/async_hooks/async-context/async-context-fs-watch.js +// crashed on macOS aarch64 release with "Segmentation fault at address 0xC". +// +// This test hammers the exact sequence from that report — watch → trigger → +// close-in-callback → process.exit — across many subprocesses so the optimizer +// has plenty of chances to exploit the UB. +test.concurrent( + "fs.watch: close() + process.exit() inside the watch callback does not crash", + async () => { + using dir = tempDir("fs-watch-close-exit", { + ".keep": "", + }); + + const script = /* js */ ` + const fs = require("fs"); + const path = require("path"); + // Each subprocess gets its own file so concurrent runs don't race on + // unlink/watch of a shared path. Under 'bun -e' there is no script + // slot, so the first extra CLI arg is argv[1]. + const file = path.join(process.argv[1], "target-" + process.pid + ".txt"); + fs.writeFileSync(file, "initial"); + + const watcher = fs.watch(file, () => { + // Inside the callback: drop the watcher (→ unregister_watcher → + // enqueue_task_concurrent, which reads self.loop_), then exit + // (→ close_and_wait → shutdown → enqueue_task_concurrent again while + // the CF thread is still inside cf_thread_loop). + watcher.close(); + try { fs.unlinkSync(file); } catch {} + process.exit(0); + }); + + // Trigger the watch — repeat on an interval so every platform + // (FSEvents has a 50ms latency floor) gets a chance to deliver. + let n = 0; + const trigger = setInterval(() => fs.writeFileSync(file, "m" + n++), 20); + + // Fallback: if the event never fires, still exercise the crash site + // (close() → unregister_watcher → enqueue_task_concurrent → reads + // self.loop_; process.exit → close_and_wait → shutdown → same). We're + // asserting "does not crash", and that code path is identical whether + // close() is called from the watch callback or a timer — only the CF + // thread's concurrent position differs. Failing here instead would + // make the test flaky under load for no extra coverage. + setTimeout(() => { + clearInterval(trigger); + watcher.close(); + process.exit(0); + }, 4000); + `; + + // Run the sequence many times. On an unpatched macOS aarch64 release build + // this reproduces the 0xC segfault within a handful of iterations; on other + // platforms it still exercises the reader-thread spawn + shutdown path. + // Batch them so a failure surfaces quickly without serializing 40 spawns. + const iterations = 40; + const width = 8; + for (let base = 0; base < iterations; base += width) { + const batch = Array.from({ length: Math.min(width, iterations - base) }, async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script, String(dir)], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe(""); + expect(proc.signalCode).toBeNull(); + expect(exitCode).toBe(0); + }); + await Promise.all(batch); + } + }, + 60_000, +);