From 594e54ba305f4daca58b2e681c2743d553b71c40 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 16:29:52 +0000 Subject: [PATCH 1/5] resolver: close remaining unsynchronized DirEntry windows behind #34271 The reload()-vs-Bun.build() regression test segfaulted once on CI at address 0x25, 22ms into the subprocess (issue #37266, ubuntu aarch64). The crash did not reproduce locally in ~3000 stress runs, so this closes the remaining windows of the race class #34271 fixed, identified by code reading. RealFS::entries_at rewrites a cached DirEntry in place under entries_mutex when a resolver with a newer generation re-reads it, dropping the old data map's bucket allocation. The route-loader iteration sites snapshot under that lock, but the resolver lookup sites (finalize_result, handle_esm_resolution, probe_wildcard_extensions, load_index_with_extension, run_env_loader) probed .data after the lock was released, so a concurrent rewrite could free the buckets mid-probe. Lookups now run inside one entries_mutex critical section (DirInfo::get_entry, or an explicit guard where the listing fd is also needed); the entry pointers they yield stay valid after unlock because EntryStore never frees. run_env_loader copies the basenames out under the lock instead of letting the dotenv loader probe the live map between file reads. Entry::kind/symlink also published their lazy-stat result in the wrong order: the slow path cleared need_stat before writing cache, so a lock-free reader on a weakly ordered CPU could skip the mutex and read a stale or torn EntryCache (a torn 16-byte Interned symlink faults inside as_bytes). need_stat is now an AtomicBool cleared after the cache write, with Release/Acquire pairing. --- src/bundler/transpiler.rs | 39 +++- src/jsc/hot_reloader.rs | 5 +- src/resolver/dir_info.rs | 51 +++-- src/resolver/fs.rs | 48 +++-- src/resolver/resolver.rs | 380 ++++++++++++++++++++------------------ 5 files changed, 301 insertions(+), 222 deletions(-) diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index c6848d9bfce9..96fbc815f1fe 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -766,16 +766,26 @@ impl<'a> Transpiler<'a> { merge_tsconfig_jsx_into(tsconfig, &mut self.options.jsx); } - let Some(dir) = dir_info.get_entries(self.resolver.generation) else { + // Refresh the listing at our generation (takes and releases + // `entries_mutex`), then copy the basenames out under the + // lock: `dot_env::Loader::load` probes the listing between + // file reads, and another resolver at a newer generation + // rewrites the `DirEntry` map in place under `entries_mutex`. + if dir_info.get_entries(self.resolver.generation).is_none() { return Ok(()); + } + let dir = { + let _entries_lock = bun_resolver::fs::FileSystem::instance() + .fs + .entries_mutex + .lock_guard(); + match dir_info.get_entries_const() { + Some(entries) => DotEnvProbeKeys( + entries.data.iter().map(|(k, _)| Box::from(&**k)).collect(), + ), + None => return Ok(()), + } }; - // `get_entries` returns `*mut bun_resolver::fs::DirEntry` - // (BSSMap-owned). `dot_env::Loader::load` takes - // `impl DirEntryProbe` (bun_dotenv sits below `bun_resolver` - // in the crate graph); `bun_resolver::fs::DirEntry` impls it. - // SAFETY: BSSMap singleton owns `*dir`; single-threaded path — - // sole `&mut` for the call. - let dir: &mut bun_resolver::fs::DirEntry = unsafe { &mut *dir }; // `Env.files: Box<[Box<[u8]>]>` but `Loader::load` // wants `&[&[u8]]`. Re-borrow into a small Vec; the explicit @@ -789,7 +799,7 @@ impl<'a> Transpiler<'a> { } else { dot_env::DotEnvFileSuffix::Development }; - env.load(dir, &env_files, suffix, skip_default_env)?; + env.load(&dir, &env_files, suffix, skip_default_env)?; } DotEnvBehavior::disable => { env.load_process()?; @@ -809,6 +819,17 @@ impl<'a> Transpiler<'a> { } } +/// Basenames copied out of a cached `DirEntry` under `entries_mutex` so +/// `dot_env::Loader::load` can probe them without holding the lock across its +/// file reads (see `run_env_loader`). +struct DotEnvProbeKeys(Vec>); + +impl dot_env::DirEntryProbe for DotEnvProbeKeys { + fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool { + self.0.iter().any(|k| **k == *query_lower) + } +} + // ══════════════════════════════════════════════════════════════════════════ // `ParseResult` / `AlreadyBundled` / `ParseOptions` + `Transpiler::parse*` // — used by `ModuleLoader::transpile_source_code` (jsc_hooks.rs) and diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index a95eba4b929e..d8fc65976bbe 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -1180,7 +1180,10 @@ where // the per-entry mutex. let _entry_guard = ent.mutex.lock_guard(); ent.set_cache_fd(Fd::INVALID); - ent.need_stat.set(true); + ent.need_stat.store( + true, + core::sync::atomic::Ordering::Release, + ); } path_string = ent.abs_path; file_hash = Watcher::get_hash(path_string.as_bytes()); diff --git a/src/resolver/dir_info.rs b/src/resolver/dir_info.rs index fafb775b5bae..858c6a45de8b 100644 --- a/src/resolver/dir_info.rs +++ b/src/resolver/dir_info.rs @@ -238,34 +238,49 @@ impl DirInfo { } } - /// Shared-borrow variant of [`get_entries`](Self::get_entries) for the - /// read-only call sites (`.get()`, `.fd`, iteration). The `DirEntry` is a - /// slot in the BSSMap-backed `EntriesOptionMap` singleton (ARENA — process - /// lifetime), so a `&'static` reborrow of the `&'static mut` returned by - /// `entries_at` is sound and needs no `unsafe` here. Prefer this over - /// `get_entries` + per-site raw deref whenever the caller only reads. - pub(crate) fn get_entries_ref(&self, generation: Generation) -> Option<&'static fs::DirEntry> { + /// Shared-borrow variant of [`get_entries`](Self::get_entries) for call + /// sites that already hold `entries_mutex` (the mutex is non-recursive); + /// see [`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked). The + /// `DirEntry` is a slot in the BSSMap-backed `EntriesOptionMap` singleton + /// (ARENA — process lifetime), so a `&'static` reborrow of the + /// `&'static mut` returned by `entries_at_locked` is sound and needs no + /// `unsafe` here. The `.data` map must only be probed/iterated while the + /// lock is held; use [`get_entry`](Self::get_entry) for one-shot lookups. + pub(crate) fn get_entries_ref_locked( + &self, + generation: Generation, + ) -> Option<&'static fs::DirEntry> { let entries_ptr = fs::FileSystem::instance() .fs - .entries_at(self.entries, generation)?; + .entries_at_locked(self.entries, generation)?; match entries_ptr { fs::EntriesOption::Entries(entries) => Some(&**entries), fs::EntriesOption::Err(_) => None, } } - /// [`get_entries_ref`](Self::get_entries_ref) for call sites that already - /// hold `entries_mutex` (the mutex is non-recursive); see - /// [`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked). - pub(crate) fn get_entries_ref_locked( + /// Generation-checked lookup of one basename in this directory's cached + /// listing, performed in a single `entries_mutex` critical section. A + /// concurrent resolver with a newer generation rewrites the `DirEntry` in + /// place under that lock ([`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked) + /// drops the old `data` map's buckets), so probing the map after the lock + /// is released can walk freed buckets. The returned lookup wraps a raw + /// pointer into the process-lifetime `EntryStore`, which stays valid after + /// the lock is dropped. + pub(crate) fn get_entry( &self, generation: Generation, - ) -> Option<&'static fs::DirEntry> { - let entries_ptr = fs::FileSystem::instance() - .fs - .entries_at_locked(self.entries, generation)?; - match entries_ptr { - fs::EntriesOption::Entries(entries) => Some(&**entries), + query: &[u8], + ) -> Option> { + let rfs = &mut fs::FileSystem::instance().fs; + // `MutexGuard` stores the mutex by raw pointer, so holding it does not + // keep `rfs` borrowed (same pattern as `RealFS::entries_at`). + let _lock = rfs.entries_mutex.lock_guard(); + match rfs.entries_at_locked(self.entries, generation)? { + fs::EntriesOption::Entries(entries) => { + let entries: &'static fs::DirEntry = entries; + entries.get(query) + } fs::EntriesOption::Err(_) => None, } } diff --git a/src/resolver/fs.rs b/src/resolver/fs.rs index 5dec4abe7aca..4534c5fe9198 100644 --- a/src/resolver/fs.rs +++ b/src/resolver/fs.rs @@ -1,3 +1,4 @@ +use core::sync::atomic::{AtomicBool, Ordering}; use std::borrow::Cow; use std::io::Write as _; @@ -136,8 +137,11 @@ impl Default for EntryCache { // `Entry::symlink` while callers hold a shared // `&Entry`. `EntryCache` is `Copy`, so `Cell` gives us safe // `.get()/.set()` through `&self` — the per-entry `mutex` serializes every -// rewrite of these `Cell`s across threads (the `unsafe impl Sync for Entry` -// below opts back in under that external-locking discipline). +// rewrite of these fields across threads (the `unsafe impl Sync for Entry` +// below opts back in under that external-locking discipline). `need_stat` +// is atomic because the `kind()`/`symlink()` fast path reads it without the +// mutex: the Release store after the `cache` write paired with the Acquire +// load is what publishes `cache` to those lock-free readers. pub struct Entry { pub(crate) cache: core::cell::Cell, pub dir: &'static [u8], @@ -148,7 +152,7 @@ pub struct Entry { pub(crate) base_lowercase_: strings::StringOrTinyString, pub mutex: Mutex, - pub need_stat: core::cell::Cell, + pub need_stat: AtomicBool, pub abs_path: Interned, } @@ -226,10 +230,10 @@ impl Entry { // Generic over `R: EntryKindResolver` so this block is independent of // which `RealFS` copy `fs` points at (see file-top comment). pub unsafe fn kind(&self, fs: *mut R, store_fd: bool) -> EntryKind { - if self.need_stat.get() { + if self.need_stat.load(Ordering::Acquire) { let _guard = self.mutex.lock_guard(); - if self.need_stat.get() { - self.need_stat.set(false); + // Relaxed: every write happens under `mutex`, which we hold. + if self.need_stat.load(Ordering::Relaxed) { // This is technically incorrect, but we are choosing not to handle errors here // SAFETY: `fs` points at the process-global RealFS singleton; `resolve_kind` // only does syscalls + string interning, so the short `&mut` cannot alias. @@ -240,8 +244,15 @@ impl Entry { store_fd, ) { Ok(c) => self.cache.set(c), - Err(_) => return self.cache().kind, + Err(_) => { + self.need_stat.store(false, Ordering::Release); + return self.cache().kind; + } } + // Clear the flag only after the `cache` write: lock-free readers + // that observe `false` skip the mutex, so this Release store is + // what publishes `cache` to them. + self.need_stat.store(false, Ordering::Release); } } self.cache().kind @@ -256,10 +267,10 @@ impl Entry { fs: *mut R, store_fd: bool, ) -> &'static [u8] { - if self.need_stat.get() { + if self.need_stat.load(Ordering::Acquire) { let _guard = self.mutex.lock_guard(); - if self.need_stat.get() { - self.need_stat.set(false); + // Relaxed: every write happens under `mutex`, which we hold. + if self.need_stat.load(Ordering::Relaxed) { // This error can happen if the file was deleted between the time the directory // was scanned and the time it was read // SAFETY: see the note on `Entry::kind`. @@ -270,8 +281,13 @@ impl Entry { store_fd, ) { Ok(c) => self.cache.set(c), - Err(_) => return b"", + Err(_) => { + self.need_stat.store(false, Ordering::Release); + return b""; + } } + // See `Entry::kind`: Release-publish `cache` before the flag clear. + self.need_stat.store(false, Ordering::Release); } } self.cache().symlink.as_bytes() @@ -479,10 +495,14 @@ impl DirEntry { let _guard = existing.mutex.lock_guard(); existing.dir = self.dir; - existing.need_stat.set( - existing.need_stat.get() + // Relaxed load: writes are serialized on the per-entry + // mutex held above; Release store pairs with the Acquire + // fast-path load in `kind()`/`symlink()`. + existing.need_stat.store( + existing.need_stat.load(Ordering::Relaxed) || found_kind.is_none() || Some(existing.cache().kind) != found_kind, + Ordering::Release, ); // TODO: is this right? if Some(existing.cache().kind) != found_kind { @@ -536,7 +556,7 @@ impl DirEntry { // Call "stat" lazily for performance. The "@material-ui/icons" package // contains a directory with over 11,000 entries in it and running "stat" // for each entry was a big performance issue for that package. - addr_of_mut!((*p).need_stat).write(core::cell::Cell::new(found_kind.is_none())); + addr_of_mut!((*p).need_stat).write(AtomicBool::new(found_kind.is_none())); addr_of_mut!((*p).cache).write(core::cell::Cell::new(EntryCache { symlink: Interned::EMPTY, // if found_kind is null, we have set need_stat above, so we diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index f9670b03931e..09713c23b34f 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -1665,98 +1665,99 @@ impl<'a> Resolver<'a> { module_type_from_ext(name.ext).unwrap_or(options::ModuleType::Unknown); } - if let Some(entries) = dir.get_entries_ref(self.generation) { - if let Some(query) = entries.get(name.filename) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. - let symlink_path = - unsafe { query.entry().symlink(self.rfs_ptr(), self.store_fd) }; - if !symlink_path.is_empty() { - path.set_realpath(symlink_path); - if !result.file_fd.is_valid() { - result.file_fd = query.entry().cache().fd; - } - - if let Some(debug) = self.debug_logs.as_mut() { - debug.add_note_fmt(format_args!( - "Resolved symlink \"{}\" to \"{}\"", - bstr::BStr::new(path.text()), - bstr::BStr::new(symlink_path) - )); - } - } else if !dir.abs_real_path.is_empty() { - // When the directory is a symlink, we don't need to call getFdPath. - let parts = [dir.abs_real_path, query.entry().base()]; - let mut buf = bun_paths::PathBuffer::uninit(); - - // NOTE: `abs_buf` returns a borrow of `buf`; capture only the - // length so `buf` can be re-borrowed for null-termination below. - let out_len = self.fs_ref().abs_buf(&parts, &mut buf).len(); - - let store_fd = self.store_fd; - - if !query.entry().cache().fd.is_valid() && store_fd { - buf[out_len] = 0; - // SAFETY: buf[out_len] == 0 written above - let span = bun_core::ZStr::from_buf(&buf[..], out_len); - // I/O errors propagate so `resolveAndAutoInstall` can - // return them as `Result.Union.failure` — never - // panic on EACCES/EMFILE/ELOOP here. - let file = bun_sys::open(span, bun_sys::O::RDONLY, 0) - .map_err(Into::::into)?; - { - // Every cached-`Entry` rewrite takes the per-entry mutex. - let _entry_guard = query.entry().mutex.lock_guard(); - query.entry().set_cache_fd(file); - } - Fs::FileSystem::set_max_fd(file.native()); - } - - // NOTE: snapshot `need_to_close_files` and raw-ptr the entry so - // the closure captures only Copy values — keeps `self` and - // `query.entry` reborrowable across the guard's lifetime. - let need_close = self.fs_ref().fs.need_to_close_files(); - // ARENA — Entry lives in the BSSMap singleton; guard runs before - // the slot is reused (resolver mutex held). Capture as `BackRef` - // (Copy, Deref) so the closure stays Copy-only while the read is - // a safe `BackRef::get()` instead of a raw-ptr deref. - let entry_ref = bun_ptr::BackRef::::from( - core::ptr::NonNull::new(query.entry).expect("EntryStore slot"), - ); - scopeguard::defer! { - if need_close { - let e = entry_ref.get(); - // Every cached-`Entry` rewrite takes the per-entry mutex. - let _entry_guard = e.mutex.lock_guard(); - let fd = e.cache().fd; - if fd.is_valid() { - fd.close(); - e.set_cache_fd(FD::INVALID); - } - } - } + // Probe the listing in one `entries_mutex` critical section: a + // concurrent resolver at a newer generation rewrites this `DirEntry`'s + // map in place under that lock. The entry pointer stays valid after + // unlock (EntryStore-owned). + if let Some(query) = dir.get_entry(self.generation, name.filename) { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `symlink()` is serialized on the per-entry mutex. + let symlink_path = unsafe { query.entry().symlink(self.rfs_ptr(), self.store_fd) }; + if !symlink_path.is_empty() { + path.set_realpath(symlink_path); + if !result.file_fd.is_valid() { + result.file_fd = query.entry().cache().fd; + } - let symlink = - Fs::FilenameStore::instance().append_slice(&buf[..out_len])?; - if let Some(debug) = self.debug_logs.as_mut() { - debug.add_note_fmt(format_args!( - "Resolved symlink \"{}\" to \"{}\"", - bstr::BStr::new(symlink), - bstr::BStr::new(path.text()) - )); - } + if let Some(debug) = self.debug_logs.as_mut() { + debug.add_note_fmt(format_args!( + "Resolved symlink \"{}\" to \"{}\"", + bstr::BStr::new(path.text()), + bstr::BStr::new(symlink_path) + )); + } + } else if !dir.abs_real_path.is_empty() { + // When the directory is a symlink, we don't need to call getFdPath. + let parts = [dir.abs_real_path, query.entry().base()]; + let mut buf = bun_paths::PathBuffer::uninit(); + + // NOTE: `abs_buf` returns a borrow of `buf`; capture only the + // length so `buf` can be re-borrowed for null-termination below. + let out_len = self.fs_ref().abs_buf(&parts, &mut buf).len(); + + let store_fd = self.store_fd; + + if !query.entry().cache().fd.is_valid() && store_fd { + buf[out_len] = 0; + // SAFETY: buf[out_len] == 0 written above + let span = bun_core::ZStr::from_buf(&buf[..], out_len); + // I/O errors propagate so `resolveAndAutoInstall` can + // return them as `Result.Union.failure` — never + // panic on EACCES/EMFILE/ELOOP here. + let file = bun_sys::open(span, bun_sys::O::RDONLY, 0) + .map_err(Into::::into)?; { // Every cached-`Entry` rewrite takes the per-entry mutex. let _entry_guard = query.entry().mutex.lock_guard(); - query - .entry() - .set_cache_symlink(Interned::from_static(symlink)); + query.entry().set_cache_fd(file); } - if !result.file_fd.is_valid() && store_fd { - result.file_fd = query.entry().cache().fd; + Fs::FileSystem::set_max_fd(file.native()); + } + + // NOTE: snapshot `need_to_close_files` and raw-ptr the entry so + // the closure captures only Copy values — keeps `self` and + // `query.entry` reborrowable across the guard's lifetime. + let need_close = self.fs_ref().fs.need_to_close_files(); + // ARENA — Entry lives in the BSSMap singleton; guard runs before + // the slot is reused (resolver mutex held). Capture as `BackRef` + // (Copy, Deref) so the closure stays Copy-only while the read is + // a safe `BackRef::get()` instead of a raw-ptr deref. + let entry_ref = bun_ptr::BackRef::::from( + core::ptr::NonNull::new(query.entry).expect("EntryStore slot"), + ); + scopeguard::defer! { + if need_close { + let e = entry_ref.get(); + // Every cached-`Entry` rewrite takes the per-entry mutex. + let _entry_guard = e.mutex.lock_guard(); + let fd = e.cache().fd; + if fd.is_valid() { + fd.close(); + e.set_cache_fd(FD::INVALID); + } } + } - path.set_realpath(symlink); + let symlink = Fs::FilenameStore::instance().append_slice(&buf[..out_len])?; + if let Some(debug) = self.debug_logs.as_mut() { + debug.add_note_fmt(format_args!( + "Resolved symlink \"{}\" to \"{}\"", + bstr::BStr::new(symlink), + bstr::BStr::new(path.text()) + )); + } + { + // Every cached-`Entry` rewrite takes the per-entry mutex. + let _entry_guard = query.entry().mutex.lock_guard(); + query + .entry() + .set_cache_symlink(Interned::from_static(symlink)); } + if !result.file_fd.is_valid() && store_fd { + result.file_fd = query.entry().cache().fd; + } + + path.set_realpath(symlink); } } } @@ -3721,13 +3722,6 @@ impl<'a> Resolver<'a> { return MatchStatus::NotFound; } }; - let entries = match resolved_dir_info.get_entries_ref(self.generation) { - Some(e) => e, - None => { - esm_resolution.status = Status::ModuleNotFound; - return MatchStatus::NotFound; - } - }; let extension_order: options::ExtOrder = if kind == ast::ImportKind::At || kind == ast::ImportKind::AtConditional { self.extension_order @@ -3738,7 +3732,22 @@ impl<'a> Resolver<'a> { }; let base = bun_paths::basename(abs_esm_path); - let entry_query = match entries.get(base) { + // One `entries_mutex` critical section for the probe and the + // listing fd: a concurrent resolver at a newer generation + // rewrites the `DirEntry` in place under that lock. The entry + // pointer stays valid after unlock (EntryStore-owned). + let looked_up = { + let rfs = &mut Fs::FileSystem::instance().fs; + let _entries_lock = rfs.entries_mutex.lock_guard(); + resolved_dir_info + .get_entries_ref_locked(self.generation) + .map(|entries| (entries.get(base), entries.fd)) + }; + let Some((entry_lookup, dirname_fd)) = looked_up else { + esm_resolution.status = Status::ModuleNotFound; + return MatchStatus::NotFound; + }; + let entry_query = match entry_lookup { Some(q) => q, None => { let ends_with_star = esm_resolution.status == Status::ExactEndsWithStar; @@ -3746,8 +3755,8 @@ impl<'a> Resolver<'a> { if ends_with_star && self.probe_wildcard_extensions( - entries, resolved_dir_info, + dirname_fd, package_json, base, extension_order, @@ -3760,15 +3769,16 @@ impl<'a> Resolver<'a> { } }; - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { entry_query.entry().kind(self.rfs_ptr(), self.store_fd) } == Fs::file_system::EntryKind::Dir { let ends_with_star = esm_resolution.status == Status::ExactEndsWithStar; if ends_with_star && self.probe_wildcard_extensions( - entries, resolved_dir_info, + dirname_fd, package_json, base, extension_order, @@ -3782,39 +3792,38 @@ impl<'a> Resolver<'a> { // Try to have a friendly error message if people forget the "/index.js" suffix if ends_with_star { if let Ok(Some(dir_info_ref)) = self.dir_info_cached(abs_esm_path) { - if let Some(dir_entries) = dir_info_ref.get_entries_ref(self.generation) - { - let index = b"index"; - let buf = bufs!(load_as_file); - buf[..index.len()].copy_from_slice(index); - for ext in self.opts.ext_order_slice(extension_order).iter() { - let ext: &[u8] = ext; - let file_name = &mut buf[0..index.len() + ext.len()]; - file_name[index.len()..].copy_from_slice(ext); - let index_query = dir_entries.get(&file_name[..]); - if let Some(iq) = index_query { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. - if unsafe { iq.entry().kind(self.rfs_ptr(), self.store_fd) } - == Fs::file_system::EntryKind::File - { - if let Some(debug) = self.debug_logs.as_mut() { - let mut ms = - Vec::with_capacity(1 + file_name.len()); - ms.push(b'/'); - ms.extend_from_slice(&file_name[..]); - let parts = - [package_json.name.as_ref(), package_subpath]; - debug.add_note_fmt(format_args!( - "The import {} is missing the suffix {}", - bstr::BStr::new(ResolvePath::join( - &parts, - bun_paths::Platform::AUTO - )), - bstr::BStr::new(&ms) - )); - } - break; + let index = b"index"; + let buf = bufs!(load_as_file); + buf[..index.len()].copy_from_slice(index); + for ext in self.opts.ext_order_slice(extension_order).iter() { + let ext: &[u8] = ext; + let file_name = &mut buf[0..index.len() + ext.len()]; + file_name[index.len()..].copy_from_slice(ext); + let index_query = + dir_info_ref.get_entry(self.generation, &file_name[..]); + if let Some(iq) = index_query { + // SAFETY: rfs points at the process-global RealFS; the + // lazy-stat rewrite inside `kind()` is serialized on the + // per-entry mutex. + if unsafe { iq.entry().kind(self.rfs_ptr(), self.store_fd) } + == Fs::file_system::EntryKind::File + { + if let Some(debug) = self.debug_logs.as_mut() { + let mut ms = Vec::with_capacity(1 + file_name.len()); + ms.push(b'/'); + ms.extend_from_slice(&file_name[..]); + let parts = + [package_json.name.as_ref(), package_subpath]; + debug.add_note_fmt(format_args!( + "The import {} is missing the suffix {}", + bstr::BStr::new(ResolvePath::join( + &parts, + bun_paths::Platform::AUTO + )), + bstr::BStr::new(&ms) + )); } + break; } } } @@ -3848,7 +3857,7 @@ impl<'a> Resolver<'a> { primary: Path::init_with_namespace(absolute_out_path, b"file"), secondary: None, }, - dirname_fd: entries.fd, + dirname_fd, file_fd: entry_query.entry().cache().fd, dir_info: Some(resolved_dir_info), is_node_module: true, @@ -3885,10 +3894,14 @@ impl<'a> Resolver<'a> { } /// Wildcard `exports`/`imports` target isn't a file: probe extensions like `load_as_file` does. - fn probe_wildcard_extensions<'e>( + /// + /// Each probe goes through [`DirInfo::get_entry`] so the map walk happens + /// under `entries_mutex`; `dirname_fd` was captured under the caller's + /// critical section. + fn probe_wildcard_extensions( &mut self, - entries: &'e Fs::file_system::DirEntry, resolved_dir_info: DirInfoRef, + dirname_fd: FD, package_json: &PackageJSON, base: &[u8], extension_order: options::ExtOrder, @@ -3904,8 +3917,11 @@ impl<'a> Resolver<'a> { let ext: &[u8] = ext; let file_name = &mut buf[0..base.len() + ext.len()]; file_name[base.len()..].copy_from_slice(ext); - if let Some(ext_query) = entries.get(&file_name[..]) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. + if let Some(ext_query) = + resolved_dir_info.get_entry(self.generation, &file_name[..]) + { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { ext_query.entry().kind(rfs, self.store_fd) } == Fs::file_system::EntryKind::File { @@ -3917,8 +3933,8 @@ impl<'a> Resolver<'a> { )); } self.build_wildcard_match( - entries, resolved_dir_info, + dirname_fd, package_json, &ext_query, out, @@ -3936,7 +3952,7 @@ impl<'a> Resolver<'a> { &[b".ts", b".tsx", b".mts"] } else if ext == b".mjs" && (!FeatureFlags::DISABLE_AUTO_JS_TO_TS_IN_NODE_MODULES - || !strings::path_contains_node_modules_folder(entries.dir)) + || !strings::path_contains_node_modules_folder(resolved_dir_info.abs_path)) { &[b".mts"] } else { @@ -3950,8 +3966,11 @@ impl<'a> Resolver<'a> { for &replacement in ts_exts.iter() { let file_name = &mut buf[0..segment.len() + replacement.len()]; file_name[segment.len()..].copy_from_slice(replacement); - if let Some(ts_query) = entries.get(&file_name[..]) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. + if let Some(ts_query) = + resolved_dir_info.get_entry(self.generation, &file_name[..]) + { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { ts_query.entry().kind(rfs, self.store_fd) } == Fs::file_system::EntryKind::File { @@ -3963,8 +3982,8 @@ impl<'a> Resolver<'a> { )); } self.build_wildcard_match( - entries, resolved_dir_info, + dirname_fd, package_json, &ts_query, out, @@ -3979,12 +3998,12 @@ impl<'a> Resolver<'a> { false } - fn build_wildcard_match<'e>( + fn build_wildcard_match( &mut self, - entries: &'e Fs::file_system::DirEntry, resolved_dir_info: DirInfoRef, + dirname_fd: FD, package_json: &PackageJSON, - query: &crate::fs::EntryLookup<'e>, + query: &crate::fs::EntryLookup<'static>, out: &mut MatchResult, ) { let abs_path: &[u8] = { @@ -4013,7 +4032,7 @@ impl<'a> Resolver<'a> { primary: Path::init_with_namespace(abs_path, b"file"), secondary: None, }, - dirname_fd: entries.fd, + dirname_fd, file_fd: query.entry().cache().fd, dir_info: Some(resolved_dir_info), is_node_module: true, @@ -5375,58 +5394,59 @@ impl<'a> Resolver<'a> { base[0..b"index".len()].copy_from_slice(b"index"); base[b"index".len()..].copy_from_slice(ext); - if let Some(entries) = dir_info.get_entries_ref(self.generation) { - if let Some(lookup) = entries.get(&base[..]) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. - if unsafe { lookup.entry().kind(rfs, self.store_fd) } - == Fs::file_system::EntryKind::File - { - let out_buf: &[u8] = { - if lookup.entry().abs_path.is_empty() { - let parts = [dir_info.abs_path, &base[..]]; - let out_buf_ = self.fs_ref().abs_buf(&parts, bufs!(index)); - // SAFETY: EntryStore-owned slot; resolver mutex held. RHS fully - // evaluated before LHS `&mut Entry` is materialized. - unsafe { &mut *lookup.entry }.abs_path = Interned::from_static( - self.fs_ref() - .dirname_store - .append_slice(out_buf_) - .expect("unreachable"), - ); - } - lookup.entry().abs_path.as_bytes() - }; - - if let Some(debug) = self.debug_logs.as_mut() { - debug.add_note_fmt(format_args!( - "Found file: \"{}\"", - bstr::BStr::new(out_buf) - )); + // Probe the listing in one `entries_mutex` critical section: a + // concurrent resolver at a newer generation rewrites the `DirEntry`'s + // map in place under that lock. The entry pointer stays valid after + // unlock (EntryStore-owned). + if let Some(lookup) = dir_info.get_entry(self.generation, &base[..]) { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. + if unsafe { lookup.entry().kind(rfs, self.store_fd) } + == Fs::file_system::EntryKind::File + { + let out_buf: &[u8] = { + if lookup.entry().abs_path.is_empty() { + let parts = [dir_info.abs_path, &base[..]]; + let out_buf_ = self.fs_ref().abs_buf(&parts, bufs!(index)); + // SAFETY: EntryStore-owned slot; resolver mutex held. RHS fully + // evaluated before LHS `&mut Entry` is materialized. + unsafe { &mut *lookup.entry }.abs_path = Interned::from_static( + self.fs_ref() + .dirname_store + .append_slice(out_buf_) + .expect("unreachable"), + ); } + lookup.entry().abs_path.as_bytes() + }; - if let Some(package_json) = dir_info.package_json() { - *out = MatchResult { - path_pair: PathPair { - primary: Path::init(out_buf), - secondary: None, - }, - package_json: Some(std::ptr::from_ref(package_json)), - dirname_fd: dir_info.get_file_descriptor(), - ..Default::default() - }; - return MatchStatus::Success; - } + if let Some(debug) = self.debug_logs.as_mut() { + debug + .add_note_fmt(format_args!("Found file: \"{}\"", bstr::BStr::new(out_buf))); + } + if let Some(package_json) = dir_info.package_json() { *out = MatchResult { path_pair: PathPair { primary: Path::init(out_buf), secondary: None, }, + package_json: Some(std::ptr::from_ref(package_json)), dirname_fd: dir_info.get_file_descriptor(), ..Default::default() }; return MatchStatus::Success; } + + *out = MatchResult { + path_pair: PathPair { + primary: Path::init(out_buf), + secondary: None, + }, + dirname_fd: dir_info.get_file_descriptor(), + ..Default::default() + }; + return MatchStatus::Success; } } From 137b425fc365f6a0ebe313411603ba1819f3f58b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 16:39:25 +0000 Subject: [PATCH 2/5] resolver: capture the index-path dirname fd under entries_mutex, publish recycled cache before need_stat Review follow-ups: load_index_with_extension now captures the listing fd in the same critical section as the basename probe instead of re-reading the DirEntry field after unlock; the recycle path in add_entry_with_store performs its cache writes before the Release store of need_stat so the publication order matches kind()/symlink(); the .mjs-to-.mts probe gate uses the DirInfo node_modules flags instead of scanning the dir path for a separator-bounded needle that misses the node_modules directory itself. --- src/resolver/fs.rs | 22 ++++++++++++---------- src/resolver/resolver.rs | 34 ++++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/resolver/fs.rs b/src/resolver/fs.rs index 4534c5fe9198..1ced2d001270 100644 --- a/src/resolver/fs.rs +++ b/src/resolver/fs.rs @@ -495,22 +495,24 @@ impl DirEntry { let _guard = existing.mutex.lock_guard(); existing.dir = self.dir; + let kind_changed = Some(existing.cache().kind) != found_kind; + if kind_changed { + // if found_kind is null, need_stat is published as true + // below, so the arbitrary kind stored here only lasts + // until the lazy stat fills in the real one + existing.set_cache_kind(found_kind.unwrap_or(EntryKind::File)); + existing.set_cache_symlink(Interned::EMPTY); + } // Relaxed load: writes are serialized on the per-entry - // mutex held above; Release store pairs with the Acquire - // fast-path load in `kind()`/`symlink()`. + // mutex held above. The Release store runs after the cache + // writes and pairs with the Acquire fast-path load in + // `kind()`/`symlink()`. existing.need_stat.store( existing.need_stat.load(Ordering::Relaxed) || found_kind.is_none() - || Some(existing.cache().kind) != found_kind, + || kind_changed, Ordering::Release, ); - // TODO: is this right? - if Some(existing.cache().kind) != found_kind { - // if found_kind is null, we have set need_stat above, so we - // store an arbitrary kind - existing.set_cache_kind(found_kind.unwrap_or(EntryKind::File)); - existing.set_cache_symlink(Interned::EMPTY); - } break 'brk existing_ptr; } } diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index 09713c23b34f..a0d4343091b8 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -3952,7 +3952,8 @@ impl<'a> Resolver<'a> { &[b".ts", b".tsx", b".mts"] } else if ext == b".mjs" && (!FeatureFlags::DISABLE_AUTO_JS_TO_TS_IN_NODE_MODULES - || !strings::path_contains_node_modules_folder(resolved_dir_info.abs_path)) + || !(resolved_dir_info.is_node_modules() + || resolved_dir_info.is_inside_node_modules())) { &[b".mts"] } else { @@ -5394,11 +5395,28 @@ impl<'a> Resolver<'a> { base[0..b"index".len()].copy_from_slice(b"index"); base[b"index".len()..].copy_from_slice(ext); - // Probe the listing in one `entries_mutex` critical section: a - // concurrent resolver at a newer generation rewrites the `DirEntry`'s - // map in place under that lock. The entry pointer stays valid after - // unlock (EntryStore-owned). - if let Some(lookup) = dir_info.get_entry(self.generation, &base[..]) { + // One `entries_mutex` critical section for the probe and the listing + // fd: a concurrent resolver at a newer generation rewrites the + // `DirEntry` in place under that lock. The entry pointer stays valid + // after unlock (EntryStore-owned). The fd gate matches + // `DirInfo::get_file_descriptor`. + let looked_up = { + let realfs = &mut Fs::FileSystem::instance().fs; + let _entries_lock = realfs.entries_mutex.lock_guard(); + dir_info + .get_entries_ref_locked(self.generation) + .map(|entries| { + ( + entries.get(&base[..]), + if FeatureFlags::STORE_FILE_DESCRIPTORS { + entries.fd + } else { + FD::INVALID + }, + ) + }) + }; + if let Some((Some(lookup), dirname_fd)) = looked_up { // SAFETY: rfs points at the process-global RealFS; the lazy-stat // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { lookup.entry().kind(rfs, self.store_fd) } @@ -5432,7 +5450,7 @@ impl<'a> Resolver<'a> { secondary: None, }, package_json: Some(std::ptr::from_ref(package_json)), - dirname_fd: dir_info.get_file_descriptor(), + dirname_fd, ..Default::default() }; return MatchStatus::Success; @@ -5443,7 +5461,7 @@ impl<'a> Resolver<'a> { primary: Path::init(out_buf), secondary: None, }, - dirname_fd: dir_info.get_file_descriptor(), + dirname_fd, ..Default::default() }; return MatchStatus::Success; From a9f3629939dcd9a3baebf04be3a79b7f01086b0c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 17:11:53 +0000 Subject: [PATCH 3/5] resolver: lock the load_as_file probe family, stop rewriting recycled entry caches load_as_file and load_extension probed the DirEntry map through the EntriesOption slot after read_directory released entries_mutex, with SAFETY comments claiming the lock was held; this was the same freed bucket walk the earlier commits closed elsewhere, on the hottest lookup path. EntriesOption::lookup now probes and captures the listing fd in one critical section (and dir_and_fd does the same for the watch registration read), with load_extension taking the slot handle instead of a bare &DirEntry. The recycle path in add_entry_with_store no longer overwrites the cached kind and symlink when getdents disagrees with the cache. A lock-free kind()/symlink() reader that already observed need_stat == false reads the cache without the per-entry mutex, and no store order can publish anything to a reader that loaded the old false, so the overwrite could tear a populated 16-byte Interned under that reader. Publishing need_stat = true alone routes every later reader through the mutex, where the lazy stat writes the fresh cache; a racing reader sees the old cache, stale but untorn. set_cache_kind had no other callers and is removed. Also trims the duplicated critical-section comments down to the accessor docs that carry the invariant. --- src/bundler/transpiler.rs | 13 ++++----- src/resolver/fs.rs | 30 ++++++++------------- src/resolver/lib.rs | 35 ++++++++++++++++++++++++ src/resolver/resolver.rs | 57 ++++++++++++++++++--------------------- 4 files changed, 77 insertions(+), 58 deletions(-) diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index 96fbc815f1fe..ffc0ad8dd447 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -766,11 +766,9 @@ impl<'a> Transpiler<'a> { merge_tsconfig_jsx_into(tsconfig, &mut self.options.jsx); } - // Refresh the listing at our generation (takes and releases - // `entries_mutex`), then copy the basenames out under the - // lock: `dot_env::Loader::load` probes the listing between - // file reads, and another resolver at a newer generation - // rewrites the `DirEntry` map in place under `entries_mutex`. + // Refresh the listing at our generation, then copy the + // basenames out under `entries_mutex`: concurrent resolvers + // rewrite the `DirEntry` map in place under that lock. if dir_info.get_entries(self.resolver.generation).is_none() { return Ok(()); } @@ -819,9 +817,8 @@ impl<'a> Transpiler<'a> { } } -/// Basenames copied out of a cached `DirEntry` under `entries_mutex` so -/// `dot_env::Loader::load` can probe them without holding the lock across its -/// file reads (see `run_env_loader`). +/// Basenames copied out of a cached `DirEntry` under `entries_mutex` so the +/// dotenv loader can probe them without the lock (see `run_env_loader`). struct DotEnvProbeKeys(Vec>); impl dot_env::DirEntryProbe for DotEnvProbeKeys { diff --git a/src/resolver/fs.rs b/src/resolver/fs.rs index 1ced2d001270..273669a85681 100644 --- a/src/resolver/fs.rs +++ b/src/resolver/fs.rs @@ -175,13 +175,6 @@ impl Entry { self.cache.set(c); } - #[inline(always)] - pub(crate) fn set_cache_kind(&self, kind: EntryKind) { - let mut c = self.cache.get(); - c.kind = kind; - self.cache.set(c); - } - #[inline(always)] pub(crate) fn set_cache_symlink(&self, symlink: Interned) { let mut c = self.cache.get(); @@ -495,22 +488,21 @@ impl DirEntry { let _guard = existing.mutex.lock_guard(); existing.dir = self.dir; - let kind_changed = Some(existing.cache().kind) != found_kind; - if kind_changed { - // if found_kind is null, need_stat is published as true - // below, so the arbitrary kind stored here only lasts - // until the lazy stat fills in the real one - existing.set_cache_kind(found_kind.unwrap_or(EntryKind::File)); - existing.set_cache_symlink(Interned::EMPTY); - } + // No cache rewrite here, even when the kind changed: a + // lock-free `kind()`/`symlink()` reader that already + // observed `need_stat == false` reads `cache` without the + // per-entry mutex, so overwriting it would race that read + // (a torn 16-byte `Interned` faults in `as_bytes`). + // Publishing `need_stat = true` instead routes every later + // reader through the mutex, where the lazy stat writes the + // fresh cache; a reader that raced the flag sees the old + // cache, stale but untorn. // Relaxed load: writes are serialized on the per-entry - // mutex held above. The Release store runs after the cache - // writes and pairs with the Acquire fast-path load in - // `kind()`/`symlink()`. + // mutex held above. existing.need_stat.store( existing.need_stat.load(Ordering::Relaxed) || found_kind.is_none() - || kind_changed, + || Some(existing.cache().kind) != found_kind, Ordering::Release, ); break 'brk existing_ptr; diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 6976d8f56a5f..2e3bb7c7cc48 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -885,6 +885,41 @@ pub mod fs { impl EntriesOption { // Payload is `&'static mut DirEntry`; auto-deref coerces to `&DirEntry` / `&mut DirEntry`. bun_core::enum_unwrap!(pub EntriesOption, Entries => fn entries / entries_mut -> DirEntry); + + /// Probe the cached listing for `query` and return the lookup plus the + /// listing fd, in one `entries_mutex` critical section. A + /// stale-generation re-read rewrites the slot's `DirEntry` (and frees + /// the old map's buckets) in place under that lock, so the map must + /// not be walked after unlock; the returned entry pointer is + /// EntryStore-owned and stays valid. The caller must NOT already hold + /// `entries_mutex` (it is non-recursive). + pub(crate) fn lookup( + &self, + query: &[u8], + ) -> (Option>, Fd) { + let _lock = FileSystem::instance().fs.entries_mutex.lock_guard(); + match self { + EntriesOption::Entries(entries) => { + // SAFETY: ARENA — the `DirEntry` is a process-lifetime + // BSSMap slot (see the enum doc), so widening the local + // reborrow to `'static` is sound. + let entries: &'static DirEntry = + unsafe { &*core::ptr::from_ref::(&**entries) }; + (entries.get(query), entries.fd) + } + EntriesOption::Err(_) => (None, Fd::INVALID), + } + } + + /// Listing dir + fd snapshot under `entries_mutex` (watch + /// registration); `None` when the slot holds a read error. + pub(crate) fn dir_and_fd(&self) -> Option<(&'static [u8], Fd)> { + let _lock = FileSystem::instance().fs.entries_mutex.lock_guard(); + match self { + EntriesOption::Entries(entries) => Some((entries.dir, entries.fd)), + EntriesOption::Err(_) => None, + } + } } // SAFETY: ARENA — `EntriesOption` holds an unbounded `&mut DirEntry` (whose `data` diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index a0d4343091b8..30c2390a9f3d 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -5395,10 +5395,8 @@ impl<'a> Resolver<'a> { base[0..b"index".len()].copy_from_slice(b"index"); base[b"index".len()..].copy_from_slice(ext); - // One `entries_mutex` critical section for the probe and the listing - // fd: a concurrent resolver at a newer generation rewrites the - // `DirEntry` in place under that lock. The entry pointer stays valid - // after unlock (EntryStore-owned). The fd gate matches + // Lookup + listing fd in one critical section (see `DirInfo::get_entry` + // for the rewrite this guards against); the fd gate matches // `DirInfo::get_file_descriptor`. let looked_up = { let realfs = &mut Fs::FileSystem::instance().fs; @@ -5890,15 +5888,6 @@ impl<'a> Resolver<'a> { dec_ret!(None); } - // ARENA-backed `DirEntry` (see `dir_entry` note above) — `BackRef` so each - // `entries!()` is a fresh safe shared borrow instead of an open-coded raw deref. - let entries = bun_ptr::BackRef::new(dir_entry.entries()); - macro_rules! entries { - () => { - entries.get() - }; - } - let base = bun_paths::basename(path); // Try the plain path without any extensions @@ -5909,8 +5898,13 @@ impl<'a> Resolver<'a> { )); } - if let Some(query) = entries!().get(base) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. + // Each probe of the listing goes through `EntriesOption::lookup`, a + // single `entries_mutex` critical section (see its doc for the + // in-place rewrite this guards against). + let (plain_query, plain_dirname_fd) = dir_entry.get().lookup(base); + if let Some(query) = plain_query { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { query.entry().kind(rfs, self.store_fd) } == Fs::file_system::EntryKind::File { if let Some(debug) = self.debug_logs.as_mut() { @@ -5935,7 +5929,7 @@ impl<'a> Resolver<'a> { dec_ret!(Some(LoadResult { path: abs_path, - dirname_fd: entries!().fd, + dirname_fd: plain_dirname_fd, file_fd: query.entry().cache().fd, })); } @@ -5952,7 +5946,7 @@ impl<'a> Resolver<'a> { // body can take `&mut self`. Backing `Box<[u8]>` is owned by // `self.opts` and never mutated while the resolver runs. let ext = bun_ptr::RawSlice::new(&*self.opts.ext_order_slice(extension_order)[i]); - if let Some(result) = self.load_extension(base, path, &ext, entries!()) { + if let Some(result) = self.load_extension(base, path, &ext, dir_entry) { dec_ret!(Some(result)); } } @@ -5965,7 +5959,7 @@ impl<'a> Resolver<'a> { // BACKREF: see `RawSlice` note above — backing `Box<[u8]>` in // `extra_cjs_extensions` is heap-stable for the resolver's life. let ext = bun_ptr::RawSlice::new(&*self.opts.extra_cjs_extensions[i]); - if let Some(result) = self.load_extension(base, path, &ext, entries!()) { + if let Some(result) = self.load_extension(base, path, &ext, dir_entry) { dec_ret!(Some(result)); } } @@ -6008,8 +6002,10 @@ impl<'a> Resolver<'a> { let buffer = &mut tail[0..segment.len() + ext_to_replace.len()]; buffer[segment.len()..].copy_from_slice(ext_to_replace); - if let Some(query) = entries!().get(&buffer[..]) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. + let (ts_query, ts_dirname_fd) = dir_entry.get().lookup(&buffer[..]); + if let Some(query) = ts_query { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { query.entry().kind(rfs, self.store_fd) } == Fs::file_system::EntryKind::File { @@ -6054,7 +6050,7 @@ impl<'a> Resolver<'a> { } query.entry().abs_path.as_bytes() }, - dirname_fd: entries!().fd, + dirname_fd: ts_dirname_fd, file_fd: query.entry().cache().fd, })); } @@ -6080,7 +6076,9 @@ impl<'a> Resolver<'a> { // For existent directories which don't find a match // Start watching it automatically, if let Some(watcher) = self.watcher.as_ref() { - watcher.watch(entries!().dir, entries!().fd); + if let Some((dir, fd)) = dir_entry.get().dir_and_fd() { + watcher.watch(dir, fd); + } } } dec_ret!(None); @@ -6091,17 +6089,12 @@ impl<'a> Resolver<'a> { base: &[u8], path: &[u8], ext: &[u8], - entries: &Fs::file_system::DirEntry, + dir_entry: bun_ptr::BackRef, ) -> Option { // SAFETY: PORT — see load_as_file; derive `rfs` from the raw `*mut FileSystem` // field so `unsafe { &mut *self.fs() }` calls below (`filename_store.append_parts`) don't pop // its provenance under Stacked Borrows. let rfs: *mut Fs::file_system::RealFS = self.rfs_ptr(); - // BACKREF — `entries` is a slot in the BSSMap-backed `DirEntry` arena - // (see `load_as_file`); detach the borrowck lifetime via `BackRef` so the - // `&mut self` calls below (debug_logs / fs_ref) don't conflict, while - // each read stays a safe `BackRef: Deref`. - let entries = bun_ptr::BackRef::new(entries); let buffer = &mut bufs!(load_as_file)[0..path.len() + ext.len()]; buffer[path.len()..].copy_from_slice(ext); let file_name = &buffer[path.len() - base.len()..buffer.len()]; @@ -6113,8 +6106,10 @@ impl<'a> Resolver<'a> { )); } - if let Some(query) = entries.get().get(file_name) { - // SAFETY: entries_mutex held; rfs points at the process-global RealFS. + let (ext_query, dirname_fd) = dir_entry.get().lookup(file_name); + if let Some(query) = ext_query { + // SAFETY: rfs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. if unsafe { query.entry().kind(rfs, self.store_fd) } == Fs::file_system::EntryKind::File { if let Some(debug) = self.debug_logs.as_mut() { @@ -6143,7 +6138,7 @@ impl<'a> Resolver<'a> { }; query.entry().abs_path.as_bytes() }, - dirname_fd: entries.fd, + dirname_fd, file_fd: query.entry().cache().fd, }); } From 1be4e6ab537a33cf04742c843e74bc3e5b01582c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 17:41:50 +0000 Subject: [PATCH 4/5] resolver: assert the entries_mutex contract at every DirEntry probe DirEntry::get, get_comptime_query and has_comptime_query now debug_assert that the calling thread holds entries_mutex, so the next unlocked probe fails deterministically in debug builds instead of surfacing as a rare segfault. Since the race has no reproducing test, this assert is the enforcement mechanism for the locking contract the earlier commits established. Callers that probed without the lock are converted: the dotenv loader now always takes a DirEntryKeys snapshot (basenames copied out under the lock; the DirEntryProbe impl for the live DirEntry is removed so the type system forces the copy), PackageManager's lockfile-presence and pm view package-name probes take the uncontended guard, and the hot reloader's changed-file probe runs under the lock. DirInfo::get_entries (raw pointer return, doc claiming callers can prove exclusivity locally) had one caller left, which only used its refresh side effect; run_env_loader now refreshes and snapshots in one critical section through get_entries_ref_locked, which becomes pub, and get_entries plus the now-uncalled RealFS::entries_at are deleted. get_entries_const documents which fields may be read unlocked. --- src/bundler/transpiler.rs | 25 ++++++--------------- src/dotenv/env_loader.rs | 23 ++++++++++++-------- src/dotenv/lib.rs | 6 ++--- src/install/PackageManager.rs | 24 +++++++++++++++----- src/install/lockfile.rs | 23 ++++++++++++++------ src/jsc/hot_reloader.rs | 12 ++++++++-- src/resolver/dir_info.rs | 35 +++++++++--------------------- src/resolver/fs.rs | 28 ++++++++++++++++++------ src/resolver/lib.rs | 27 +++++------------------ src/runtime/cli/pm_view_command.rs | 11 +++++++++- 10 files changed, 116 insertions(+), 98 deletions(-) diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index ffc0ad8dd447..536e3ea26598 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -766,19 +766,18 @@ impl<'a> Transpiler<'a> { merge_tsconfig_jsx_into(tsconfig, &mut self.options.jsx); } - // Refresh the listing at our generation, then copy the - // basenames out under `entries_mutex`: concurrent resolvers - // rewrite the `DirEntry` map in place under that lock. - if dir_info.get_entries(self.resolver.generation).is_none() { - return Ok(()); - } + // Copy the listing's basenames out under `entries_mutex`, + // refreshing it at our generation in the same critical + // section: concurrent resolvers rewrite the `DirEntry` map in + // place under that lock, and `dot_env::Loader::load` does + // file I/O between probes. let dir = { let _entries_lock = bun_resolver::fs::FileSystem::instance() .fs .entries_mutex .lock_guard(); - match dir_info.get_entries_const() { - Some(entries) => DotEnvProbeKeys( + match dir_info.get_entries_ref_locked(self.resolver.generation) { + Some(entries) => dot_env::DirEntryKeys( entries.data.iter().map(|(k, _)| Box::from(&**k)).collect(), ), None => return Ok(()), @@ -817,16 +816,6 @@ impl<'a> Transpiler<'a> { } } -/// Basenames copied out of a cached `DirEntry` under `entries_mutex` so the -/// dotenv loader can probe them without the lock (see `run_env_loader`). -struct DotEnvProbeKeys(Vec>); - -impl dot_env::DirEntryProbe for DotEnvProbeKeys { - fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool { - self.0.iter().any(|k| **k == *query_lower) - } -} - // ══════════════════════════════════════════════════════════════════════════ // `ParseResult` / `AlreadyBundled` / `ParseOptions` + `Transpiler::parse*` // — used by `ModuleLoader::transpile_source_code` (jsc_hooks.rs) and diff --git a/src/dotenv/env_loader.rs b/src/dotenv/env_loader.rs index 84222b703430..f97680c84596 100644 --- a/src/dotenv/env_loader.rs +++ b/src/dotenv/env_loader.rs @@ -22,20 +22,25 @@ pub enum DotEnvFileSuffix { } /// Directory-entry probe used by `Loader::load`. `bun_dotenv` sits below -/// `bun_resolver` in the crate graph, so the concrete -/// `bun_resolver::fs::DirEntry` is taken generically; the only operation -/// `load_default_files` performs is a fast O(1) lookup of a -/// known-at-compile-time filename in the directory's entry map. Implemented -/// for `bun_resolver::fs::DirEntry`. +/// `bun_resolver` in the crate graph, so the directory listing is taken +/// generically; the only operation `load_default_files` performs is a lookup +/// of a known-at-compile-time filename. Callers snapshot the resolver's +/// listing into a [`DirEntryKeys`] (the live `DirEntry` map may be rewritten +/// in place by a concurrent resolver, so `load` must not probe it directly). pub trait DirEntryProbe { /// The argument MUST already be ASCII-lowercase. fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool; } -// LAYERING: the concrete `DirEntry` lives in `bun_resolver::fs` (higher tier, -// depends on this crate). `impl DirEntryProbe for bun_resolver::fs::DirEntry` -// is provided there — see src/resolver/lib.rs. No impl here; that would be a -// dep-cycle. +/// Directory-listing basenames copied out under the resolver's +/// `entries_mutex`, probed between the `.env` file reads in `Loader::load`. +pub struct DirEntryKeys(pub Vec>); + +impl DirEntryProbe for DirEntryKeys { + fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool { + self.0.iter().any(|k| **k == *query_lower) + } +} /// Canonical definition; re-exported as /// `bun_options_types::schema::api::DotEnvBehavior` for higher tiers. diff --git a/src/dotenv/lib.rs b/src/dotenv/lib.rs index 9661695ef0df..8a60edc0ec4f 100644 --- a/src/dotenv/lib.rs +++ b/src/dotenv/lib.rs @@ -6,9 +6,9 @@ pub mod error; pub use error::{Error, Result}; pub use env_loader::{ - DirEntryProbe, DotEnvBehavior, DotEnvFileSuffix, HAS_NO_CLEAR_SCREEN_CLI_FLAG, HashTable, - HashTableValue, INSTANCE, Loader, Map, NullDelimitedEnvMap, S3Credentials, StdEnvMapWrapper, - instance, set_instance, + DirEntryKeys, DirEntryProbe, DotEnvBehavior, DotEnvFileSuffix, HAS_NO_CLEAR_SCREEN_CLI_FLAG, + HashTable, HashTableValue, INSTANCE, Loader, Map, NullDelimitedEnvMap, S3Credentials, + StdEnvMapWrapper, instance, set_instance, }; /// `dotenv::map::{HashTable, Entry}` namespace expected by `install_jsc::ini_jsc` et al. diff --git a/src/install/PackageManager.rs b/src/install/PackageManager.rs index ee11c66510a1..ce93caa60233 100644 --- a/src/install/PackageManager.rs +++ b/src/install/PackageManager.rs @@ -1736,11 +1736,20 @@ pub fn init( }; env.load_process()?; - // Reborrow the BSSMap-owned `*DirEntry` for the - // call; `env.load` only reads it (`hasComptimeQuery` lookups for `.env*`). + // Copy the listing's basenames out under `entries_mutex`; `.data` must + // only be probed while the lock is held. + let env_probe_keys = { + let _entries_lock = FileSystem::instance().fs.entries_mutex.lock_guard(); + dot_env::DirEntryKeys( + entries_option + .data + .iter() + .map(|(k, _)| Box::from(&**k)) + .collect(), + ) + }; env.load( - // SAFETY: see `entries_option` above — single-threaded init, BSSMap-owned. - unsafe { &mut *std::ptr::from_mut::(entries_option) }, + &env_probe_keys, &[], dot_env::DotEnvFileSuffix::Production, false, @@ -2454,7 +2463,12 @@ fn init_with_runtime_once( // `root_dir` was moved into `*manager` above (the field is // an unbounded `&mut DirEntry`, so the local reborrow is for `'static` and the // original binding is dead). Read it back through `manager.root_dir`. - if manager.root_dir.has_comptime_query(b"bun.lockb") { + // `.data` probes must hold `entries_mutex`. + let has_lockb = { + let _entries_lock = FileSystem::instance().fs.entries_mutex.lock_guard(); + manager.root_dir.has_comptime_query(b"bun.lockb") + }; + if has_lockb { let mut lockfile = core::mem::replace(&mut manager.lockfile, Box::new(Lockfile::default())); match lockfile.load_from_cwd::(Some(&mut *manager), log) { lockfile::LoadResult::Ok(_) => {} diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index fdc2d43b1e3f..5e5b8ec418f6 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -1736,20 +1736,29 @@ impl<'a> Printer<'a> { // Capture the `'static` cwd slice // before borrowing `fs.fs` mutably. let top_level_dir = fs.top_level_dir; - let entries_option = fs.fs.read_directory(top_level_dir, None, 0, true)?; - let entries: &mut Fs::DirEntry = match entries_option { - Fs::EntriesOption::Entries(e) => &mut **e, - Fs::EntriesOption::Err(e) => return Err(e.canonical_error.into()), + // Erase to raw so the `entries_mutex` reborrow below doesn't conflict + // with the `&mut self` borrow `read_directory` took. + let entries_option: *const Fs::EntriesOption = + fs.fs.read_directory(top_level_dir, None, 0, true)?; + // Copy the listing's basenames out under `entries_mutex`; `.data` must + // only be probed while the lock is held. + let entries = { + let _entries_lock = fs.fs.entries_mutex.lock_guard(); + // SAFETY: BSSMap-owned slot; shared read under `entries_mutex`. + match unsafe { &*entries_option } { + Fs::EntriesOption::Entries(e) => { + DotEnv::DirEntryKeys(e.data.iter().map(|(k, _)| Box::from(&**k)).collect()) + } + Fs::EntriesOption::Err(e) => return Err(e.canonical_error.into()), + } }; let mut env_loader = DotEnv::Loader::init(); env_loader.quiet = true; env_loader.load_process()?; - // `DotEnv::Loader::load` takes `impl DirEntryProbe` (bun_dotenv sits - // below `bun_resolver` in the crate graph); `Fs::DirEntry` impls it. env_loader.load( - &*entries, + &entries, &[] as &[&[u8]], DotEnv::DotEnvFileSuffix::Production, false, diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index d8fc65976bbe..79f47b1a8cac 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -1171,8 +1171,16 @@ where let path_string: bun_ptr::Interned; let file_hash: bun_watcher::HashType; let abs_path: &[u8] = 'brk: { - if let Some(file_ent) = dir_ent.entries().get(changed_name) - { + // Probe `.data` under `entries_mutex`; a + // resolver at a newer generation rewrites + // the map in place under that lock. The + // entry pointer stays valid after unlock + // (EntryStore-owned). + let looked_up = { + let _entries_lock = rfs.entries_mutex.lock_guard(); + dir_ent.entries().get(changed_name) + }; + if let Some(file_ent) = looked_up { // reset the file descriptor let ent = file_ent.entry(); { diff --git a/src/resolver/dir_info.rs b/src/resolver/dir_info.rs index 858c6a45de8b..9b45c9608bd2 100644 --- a/src/resolver/dir_info.rs +++ b/src/resolver/dir_info.rs @@ -216,40 +216,21 @@ impl DirInfo { pub fn get_file_descriptor(&self) -> Fd { if FeatureFlags::STORE_FILE_DESCRIPTORS { - // `entries_at(_, 0)` never re-reads (`u16 < 0` is always false), so the - // lock it would take covers no mutation; go through the same plain - // `at_index` lookup `get_entries_const` uses. + // Scalar field read; see `get_entries_const` for the contract. return self.get_entries_const().map_or(Fd::INVALID, |e| e.fd); } Fd::INVALID } - /// Returns a - /// raw pointer (not `&'static mut`) because the BSSMap singleton is - /// shared-mutable and Rust forbids manufacturing aliased `&mut`. Callers - /// dereference at the use site where exclusivity is locally provable. - pub fn get_entries(&self, generation: Generation) -> Option<*mut fs::DirEntry> { - let entries_ptr = fs::FileSystem::instance() - .fs - .entries_at(self.entries, generation)?; - match entries_ptr { - fs::EntriesOption::Entries(entries) => Some(std::ptr::from_mut(*entries)), - fs::EntriesOption::Err(_) => None, - } - } - - /// Shared-borrow variant of [`get_entries`](Self::get_entries) for call - /// sites that already hold `entries_mutex` (the mutex is non-recursive); - /// see [`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked). The + /// Generation-checked listing accessor for call sites that already hold + /// `entries_mutex` (the mutex is non-recursive); see + /// [`RealFS::entries_at_locked`](fs::RealFS::entries_at_locked). The /// `DirEntry` is a slot in the BSSMap-backed `EntriesOptionMap` singleton /// (ARENA — process lifetime), so a `&'static` reborrow of the /// `&'static mut` returned by `entries_at_locked` is sound and needs no /// `unsafe` here. The `.data` map must only be probed/iterated while the /// lock is held; use [`get_entry`](Self::get_entry) for one-shot lookups. - pub(crate) fn get_entries_ref_locked( - &self, - generation: Generation, - ) -> Option<&'static fs::DirEntry> { + pub fn get_entries_ref_locked(&self, generation: Generation) -> Option<&'static fs::DirEntry> { let entries_ptr = fs::FileSystem::instance() .fs .entries_at_locked(self.entries, generation)?; @@ -274,7 +255,7 @@ impl DirInfo { ) -> Option> { let rfs = &mut fs::FileSystem::instance().fs; // `MutexGuard` stores the mutex by raw pointer, so holding it does not - // keep `rfs` borrowed (same pattern as `RealFS::entries_at`). + // keep `rfs` borrowed. let _lock = rfs.entries_mutex.lock_guard(); match rfs.entries_at_locked(self.entries, generation)? { fs::EntriesOption::Entries(entries) => { @@ -285,6 +266,10 @@ impl DirInfo { } } + /// As-cached listing access with no generation check and no locking. + /// Scalar fields (`fd`, `dir`) may be read through this unlocked; the + /// `.data` map must only be probed/iterated while `entries_mutex` is held + /// (a concurrent stale-generation re-read rewrites it in place). pub fn get_entries_const(&self) -> Option<&fs::DirEntry> { let entries_ptr = fs::FileSystem::instance() .fs diff --git a/src/resolver/fs.rs b/src/resolver/fs.rs index 273669a85681..d1edb9f43517 100644 --- a/src/resolver/fs.rs +++ b/src/resolver/fs.rs @@ -599,11 +599,30 @@ impl DirEntry { Ok(()) } + /// Debug-only contract check: every `.data` probe must run under + /// `entries_mutex`, because a stale-generation re-read + /// (`RealFS::entries_at_locked`) rewrites the `DirEntry` in place under + /// that lock and frees the old map's buckets. This assert turns the next + /// unlocked probe into a deterministic debug-build failure instead of a + /// rare segfault. + #[inline] + fn debug_assert_entries_mutex_held() { + debug_assert!( + crate::fs::FileSystem::instance() + .fs + .entries_mutex + .is_held_by_current_thread(), + "DirEntry.data probed without entries_mutex; a concurrent \ + stale-generation re-read frees the map's buckets in place" + ); + } + // `query_` borrow is detached from the returned `Entry` lifetime so callers // can pass a slice into the same threadlocal buffer they then mutate. The // lookup key is the lowercased basename; a case-mismatched query still // returns the stored entry. pub fn get<'a>(&'a self, query_: &[u8]) -> Option> { + Self::debug_assert_entries_mutex_held(); if query_.is_empty() || query_.len() > MAX_PATH_BYTES { return None; } @@ -623,6 +642,7 @@ impl DirEntry { &'a self, query_lower: &'static [u8], ) -> Option> { + Self::debug_assert_entries_mutex_held(); let &result_ptr = self.data.get(query_lower)?; Some(EntryLookup { entry: result_ptr, @@ -632,6 +652,7 @@ impl DirEntry { /// True if a cached entry exists for the given already-lowercase name. pub fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool { + Self::debug_assert_entries_mutex_held(); self.data.contains_key(query_lower) } } @@ -639,13 +660,6 @@ impl DirEntry { // `data` drops itself and `dir` is interned in DirnameStore (see the comment // on `DirEntry::dir`). Body would be empty, so no `impl Drop`. -impl bun_dotenv::DirEntryProbe for DirEntry { - #[inline] - fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool { - DirEntry::has_comptime_query(self, query_lower) - } -} - #[derive(Default, Clone, Copy)] pub struct ModKey { pub(crate) size: u64, diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 2e3bb7c7cc48..4592dcf555f8 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1579,26 +1579,11 @@ pub mod fs { } /// Index lookup with generation-check - /// re-read (open + readdir + cache replace) when the cached listing is stale. - /// - /// Takes `entries_mutex` for the whole lookup: the generation-stale branch - /// drops the existing `DirEntry` (and the bucket allocation behind its - /// `data` map) in place, and the route loaders iterate that map under the - /// same lock. Call [`entries_at_locked`](Self::entries_at_locked) instead - /// from inside a critical section that already holds `entries_mutex`. - pub(crate) fn entries_at( - &mut self, - index: bun_alloc::IndexType, - generation: Generation, - ) -> Option<&mut EntriesOption> { - // `MutexGuard` stores the mutex by raw pointer (see `EntriesGuard`), - // so holding it does not keep `&mut self` borrowed. - let _g = self.entries_mutex.lock_guard(); - self.entries_at_locked(index, generation) - } - - /// [`entries_at`](Self::entries_at) for call sites that already hold - /// `entries_mutex` (the mutex is non-recursive). + /// re-read (open + readdir + cache replace) when the cached listing is + /// stale. The generation-stale branch drops the existing `DirEntry` + /// (and the bucket allocation behind its `data` map) in place, and + /// every `.data` reader holds `entries_mutex` for the probe, so the + /// caller must already hold it (the mutex is non-recursive). pub(crate) fn entries_at_locked( &mut self, index: bun_alloc::IndexType, @@ -1610,7 +1595,7 @@ pub mod fs { ); // erase to raw immediately so re-borrowing `&mut self` for // `open_dir`/`readdir`/`read_directory_error` doesn't conflict. - // `entries_mutex` held (by `entries_at` or the caller); sole `&mut` to this slot. + // `entries_mutex` held by the caller; sole `&mut` to this slot. let result_ptr = std::ptr::from_mut::(self.entries.at_index(index)?); // SAFETY: BSSMap-owned slot; uniquely held under `entries_mutex`. if let EntriesOption::Entries(existing) = unsafe { &mut *result_ptr } { diff --git a/src/runtime/cli/pm_view_command.rs b/src/runtime/cli/pm_view_command.rs index 2e25269bf962..ebeca184aac6 100644 --- a/src/runtime/cli/pm_view_command.rs +++ b/src/runtime/cli/pm_view_command.rs @@ -36,7 +36,16 @@ pub(crate) fn view( 'from_package_json: { // `root_dir` is set once by `PackageManager::init()` and points // into the resolver's directory cache for the process lifetime. - if !manager.root_dir.has_comptime_query(b"package.json") { + // `.data` probes must hold `entries_mutex` (uncontended on + // this single-threaded CLI path). + let has_package_json = { + let _entries_lock = bun_resolver::fs::FileSystem::instance() + .fs + .entries_mutex + .lock_guard(); + manager.root_dir.has_comptime_query(b"package.json") + }; + if !has_package_json { break 'from_package_json; } let fd = manager.root_dir.fd; From ea7d69f001cce34146682f5cde4ee23e85f1d1bb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:18:11 +0000 Subject: [PATCH 5/5] resolver: take entries_mutex in the run listing iterations, drop stale probe comments The bun run script and bin listing loops iterate DirEntry.data through get_entries_const; they now hold the uncontended entries_mutex so the iteration matches the documented contract, and their SAFETY comments no longer claim a lock that was not held. The filter glob walker holds a live map iterator across calls, which the lock cannot cover; its comments now state the single-threaded-CLI reasoning instead. Also removes a dotenv comment referencing the DirEntryProbe impl this branch deleted. --- src/dotenv/env_loader.rs | 3 --- src/resolver/lib.rs | 11 +++++++---- src/runtime/cli/run_command.rs | 22 ++++++++++++++++++---- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/dotenv/env_loader.rs b/src/dotenv/env_loader.rs index f97680c84596..0ae6c8e6a18d 100644 --- a/src/dotenv/env_loader.rs +++ b/src/dotenv/env_loader.rs @@ -687,9 +687,6 @@ impl Loader { ) -> crate::Result<()> { let dir_handle = bun_sys::Fd::cwd(); - // `bun_dotenv` sits below `bun_resolver` in the crate graph, so the - // directory entry is taken generically — `bun_resolver::fs::DirEntry` - // impls `DirEntryProbe`. match suffix { DotEnvFileSuffix::Development => { self.try_load_default(dir, dir_handle, b".env.development.local", value_buffer)? diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 4592dcf555f8..23bed02537d5 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1900,14 +1900,17 @@ pub mod dir_entry_accessor { return Ok(None); }; // BACKREF: ARENA — `*mut Entry` points into the EntryStore - // BSSList singleton ('static lifetime); `RealFS.entries_mutex` - // serializes access. `BackRef::from(NonNull)` + `Deref` keeps - // the read site safe. + // BSSList singleton ('static lifetime). This iterator holds a + // live `.data` iterator across calls, which `entries_mutex` + // cannot cover; it is only sound on the single-threaded CLI + // glob walk (`bun run --filter`), before any concurrent + // resolver exists to rewrite the map. let entry = bun_ptr::BackRef::::from( core::ptr::NonNull::new(*val).expect("EntryStore slot"), ); let fs: *mut Implementation = &raw mut FS::instance().fs; - // SAFETY: entries_mutex held; fs points at the process-global RealFS. + // SAFETY: fs points at the process-global RealFS; the lazy-stat + // rewrite inside `kind()` is serialized on the per-entry mutex. let kind = unsafe { entry.kind(fs, true) }; let fskind = match kind { EntryKind::File => bun_sys::FileKind::File, diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index 0e37e83b9cde..aa99cd121241 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -3663,6 +3663,12 @@ impl RunCommand { .flatten() { if let Some(entries) = bin_dir.get_entries_const() { + // `.data` iteration must hold `entries_mutex` + // (uncontended on this single-threaded CLI path). + let _entries_lock = bun_resolver::fs::FileSystem::instance() + .fs + .entries_mutex + .lock_guard(); let mut path_buf = PathBuffer::uninit(); let mut iter = entries.data.iter(); let mut has_copied = false; @@ -3671,8 +3677,9 @@ impl RunCommand { // SAFETY: `EntryMap` stores non-null `*mut Entry` values owned by // the resolver dir-cache for the process lifetime. let value = unsafe { &**entry.1 }; - // SAFETY: entries_mutex held; `Transpiler::fs` is the - // non-null process-static singleton. + // SAFETY: `Transpiler::fs` is the non-null process-static + // singleton; the lazy-stat rewrite inside `kind()` is + // serialized on the per-entry mutex. if unsafe { value.kind(&raw mut (*this_transpiler.fs).fs, true) } == bun_resolver::fs::EntryKind::File { @@ -3718,6 +3725,12 @@ impl RunCommand { .flatten() { if let Some(entries) = dir_info.get_entries_const() { + // `.data` iteration must hold `entries_mutex` + // (uncontended on this single-threaded CLI path). + let _entries_lock = bun_resolver::fs::FileSystem::instance() + .fs + .entries_mutex + .lock_guard(); let mut iter = entries.data.iter(); while let Some(entry) = iter.next() { @@ -3734,8 +3747,9 @@ impl RunCommand { && !strings::contains(name, b".d.ts") && !strings::contains(name, b".d.mts") && !strings::contains(name, b".d.cts") - // SAFETY: entries_mutex held; `Transpiler::fs` is the - // non-null process-static singleton. + // SAFETY: `Transpiler::fs` is the non-null process-static + // singleton; the lazy-stat rewrite inside `kind()` is + // serialized on the per-entry mutex. && unsafe { value.kind(&raw mut (*this_transpiler.fs).fs, true) } == bun_resolver::fs::EntryKind::File {