diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index c6848d9bfce9..536e3ea26598 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -766,16 +766,23 @@ impl<'a> Transpiler<'a> { merge_tsconfig_jsx_into(tsconfig, &mut self.options.jsx); } - let Some(dir) = dir_info.get_entries(self.resolver.generation) else { - 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_ref_locked(self.resolver.generation) { + Some(entries) => dot_env::DirEntryKeys( + 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 +796,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()?; diff --git a/src/dotenv/env_loader.rs b/src/dotenv/env_loader.rs index 84222b703430..0ae6c8e6a18d 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. @@ -682,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/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 a95eba4b929e..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(); { @@ -1180,7 +1188,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..9b45c9608bd2 100644 --- a/src/resolver/dir_info.rs +++ b/src/resolver/dir_info.rs @@ -216,60 +216,60 @@ 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> { + /// 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 fn get_entries_ref_locked(&self, generation: Generation) -> Option<&'static 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 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> { - 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. + 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, } } + /// 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 5dec4abe7aca..d1edb9f43517 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, } @@ -171,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(); @@ -226,10 +223,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 +237,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 +260,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 +274,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,18 +488,23 @@ impl DirEntry { let _guard = existing.mutex.lock_guard(); existing.dir = self.dir; - existing.need_stat.set( - existing.need_stat.get() + // 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. + 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 { - // 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; } } @@ -536,7 +550,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 @@ -585,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; } @@ -609,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, @@ -618,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) } } @@ -625,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 6976d8f56a5f..23bed02537d5 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` @@ -1544,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, @@ -1575,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 } { @@ -1880,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/resolver/resolver.rs b/src/resolver/resolver.rs index f9670b03931e..30c2390a9f3d 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,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(entries.dir)) + || !(resolved_dir_info.is_node_modules() + || resolved_dir_info.is_inside_node_modules())) { &[b".mts"] } else { @@ -3950,8 +3967,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 +3983,8 @@ impl<'a> Resolver<'a> { )); } self.build_wildcard_match( - entries, resolved_dir_info, + dirname_fd, package_json, &ts_query, out, @@ -3979,12 +3999,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 +4033,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 +5395,74 @@ 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) - )); + // 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; + 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) } + == 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, }, - dirname_fd: dir_info.get_file_descriptor(), + package_json: Some(std::ptr::from_ref(package_json)), + dirname_fd, ..Default::default() }; return MatchStatus::Success; } + + *out = MatchResult { + path_pair: PathPair { + primary: Path::init(out_buf), + secondary: None, + }, + dirname_fd, + ..Default::default() + }; + return MatchStatus::Success; } } @@ -5852,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 @@ -5871,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() { @@ -5897,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, })); } @@ -5914,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)); } } @@ -5927,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)); } } @@ -5970,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 { @@ -6016,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, })); } @@ -6042,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); @@ -6053,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()]; @@ -6075,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() { @@ -6105,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, }); } 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; 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 {