From 75c5471e3d70d42b371c2a02f66b17d2bf723ef0 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 29 Jun 2026 08:58:53 +0000 Subject: [PATCH] resolver: serialize cached directory-entry rewrites on the per-entry lock The FileSystemRouter / FrameworkRouter load loops iterated a cached DirEntry's hashmap while another thread (e.g. Bun.build's resolver) could rewrite that map in place, and the lazily-populated Entry stat cache was rewritten from several threads without a lock. - Snapshot the DirEntry's entry pointers under the existing entries_mutex before each load/bust/scan loop (the guard is dropped before the loop so the read_dir_info recursion can re-acquire it). - Take the existing per-entry Entry.mutex (double-checked) inside Entry::kind/Entry::symlink's lazy-stat branch and at every other site that rewrites a cached Entry (Route::parse, the resolver's set_cache_* callers, the hot reloader), matching the lock the directory re-read path already takes; cached reads stay lock-free. - Keep a refreshed DirEntry's interned directory name stable across in-place refreshes, and accept both trailing-slash spellings of a cached directory name in the route loader (the resolver and the router spell the same directory differently, which also tripped a debug assertion when a Bun.build preceded the router on the same directory). --- src/jsc/hot_reloader.rs | 9 +- src/resolver/fs.rs | 98 +++++++------ src/resolver/lib.rs | 17 +-- src/resolver/resolver.rs | 35 +++-- src/router/lib.rs | 154 ++++++++++++--------- src/runtime/api/filesystem_router.rs | 18 ++- src/runtime/bake/FrameworkRouter.rs | 19 ++- test/js/bun/util/filesystem_router.test.ts | 86 +++++++++++- 8 files changed, 295 insertions(+), 141 deletions(-) diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index c4385b0b82c2..568e2613a4b2 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -1195,8 +1195,13 @@ where { // reset the file descriptor let ent = file_ent.entry(); - ent.set_cache_fd(Fd::INVALID); - ent.need_stat.set(true); + { + // Every cached-`Entry` rewrite takes + // the per-entry mutex. + let _entry_guard = ent.mutex.lock_guard(); + ent.set_cache_fd(Fd::INVALID); + ent.need_stat.set(true); + } path_string = ent.abs_path; file_hash = Watcher::get_hash(path_string.as_bytes()); for (entry_id, hash) in hashes.iter().enumerate() { diff --git a/src/resolver/fs.rs b/src/resolver/fs.rs index c72ebe8c1b77..fac9d63ecb56 100644 --- a/src/resolver/fs.rs +++ b/src/resolver/fs.rs @@ -382,9 +382,9 @@ impl Default for EntryCache { // `cache` / `need_stat` are lazily populated by `Entry::kind` / // `Entry::symlink` while callers hold a shared // `&Entry`. `EntryCache` is `Copy`, so `Cell` gives us safe -// `.get()/.set()` through `&self` — `RealFS.entries_mutex` serializes access -// across threads (the `unsafe impl Sync for Entry` below opts back in under -// that external-locking discipline). +// `.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). pub struct Entry { pub cache: core::cell::Cell, pub dir: &'static [u8], @@ -416,7 +416,7 @@ impl Entry { } /// Update a single cache field. Read-modify-write is fine: callers hold - /// `RealFS.entries_mutex` so no torn writes; `EntryCache` is `Copy`. + /// the per-entry `mutex` so no torn writes; `EntryCache` is `Copy`. #[inline(always)] pub fn set_cache_fd(&self, fd: Fd) { let mut c = self.cache.get(); @@ -469,26 +469,32 @@ impl Entry { /// /// # Safety /// `fs` must point to a live `EntryKindResolver` (the process-global - /// `RealFS` singleton in practice) and the caller must hold - /// `RealFS.entries_mutex` so the `&mut *fs` reborrow is exclusive for the - /// duration of the call. - // `Entry` lives in the EntryStore BSSMap singleton; all access is - // serialized through `RealFS.entries_mutex`. `fs` is `*mut` so the - // call site does not require a second exclusive `&mut RealFS` borrow while a - // `&mut Entry` (borrowed out of `RealFS.entries`) is live. Mutation of the - // lazily-populated `need_stat` / `cache` goes through `Cell`. Generic over - // `R: EntryKindResolver` so this block is independent of which `RealFS` - // copy `fs` points at (see file-top comment). + /// `RealFS` singleton in practice). `resolve_kind` must not re-enter + /// this entry's `mutex` (it only performs syscalls and string interning). + // `Entry` lives in the EntryStore BSSMap singleton. The lazy-stat rewrite + // of `need_stat` / `cache` is serialized on the per-entry `mutex` here + // (double-checked: the cached fast path stays lock-free). `fs` is `*mut` + // so the call site does not require a second exclusive `&mut RealFS` + // borrow while a `&mut Entry` (borrowed out of `RealFS.entries`) is live. + // 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() { - self.need_stat.set(false); - // This is technically incorrect, but we are choosing not to handle errors here - // SAFETY: `fs` points at the process-global RealFS singleton; caller holds - // `entries_mutex` so the `&mut` is exclusive for the duration of this call. - match unsafe { &mut *fs }.resolve_kind(self.dir, self.base(), self.cache().fd, store_fd) - { - Ok(c) => self.cache.set(c), - Err(_) => return self.cache().kind, + let _guard = self.mutex.lock_guard(); + if self.need_stat.get() { + self.need_stat.set(false); + // 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. + match unsafe { &mut *fs }.resolve_kind( + self.dir, + self.base(), + self.cache().fd, + store_fd, + ) { + Ok(c) => self.cache.set(c), + Err(_) => return self.cache().kind, + } } } self.cache().kind @@ -497,23 +503,28 @@ impl Entry { /// /// # Safety /// `fs` must point to a live `EntryKindResolver` (the process-global - /// `RealFS` singleton in practice) and the caller must hold - /// `RealFS.entries_mutex` so the `&mut *fs` reborrow is exclusive for the - /// duration of the call. + /// `RealFS` singleton in practice). See [`Entry::kind`]. pub unsafe fn symlink( &self, fs: *mut R, store_fd: bool, ) -> &'static [u8] { if self.need_stat.get() { - self.need_stat.set(false); - // 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`. - match unsafe { &mut *fs }.resolve_kind(self.dir, self.base(), self.cache().fd, store_fd) - { - Ok(c) => self.cache.set(c), - Err(_) => return b"", + let _guard = self.mutex.lock_guard(); + if self.need_stat.get() { + self.need_stat.set(false); + // 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`. + match unsafe { &mut *fs }.resolve_kind( + self.dir, + self.base(), + self.cache().fd, + store_fd, + ) { + Ok(c) => self.cache.set(c), + Err(_) => return b"", + } } } self.cache().symlink.as_bytes() @@ -565,7 +576,7 @@ pub struct DifferentCase<'a> { // `entry` is a RAW `*mut Entry`. A safe // `&self → &mut Entry` accessor would let two `get()` calls produce coexisting // aliased `&mut Entry` (PORTING.md §Forbidden). Callers `unsafe { &mut *entry }` -// at each write site under `entries_mutex`. +// at each write site under the per-entry `Entry.mutex`. pub struct EntryLookup<'a> { pub entry: *mut Entry, pub diff_case: Option>, @@ -593,8 +604,8 @@ impl<'a> EntryLookup<'a> { // (zero callers). `Entry`'s only mutable state (`cache`) is `Cell`-backed, // so all mutation goes through `entry().set_cache*()` on a shared borrow; // no `&mut Entry` escape hatch is needed. Write sites that bypass the - // accessor go through the raw `self.entry` field directly under - // `entries_mutex` (see struct doc above). + // accessor go through the raw `self.entry` field directly under the + // per-entry `Entry.mutex` (see struct doc above). } /// `DirEntry` companion items: the entry map, the global entry store, and the @@ -1775,13 +1786,14 @@ impl RealFS { }; // if we get this far, it's a real directory, so we can just store the dir name. - let dir: &'static [u8] = if !had_handle { - if let Some(existing) = in_place { - // SAFETY: in_place points to BSSMap-owned DirEntry - unsafe { (*existing).dir } - } else { - DirnameStore::instance().append(dir_maybe_trail_slash)? - } + // An in-place refresh always keeps the slot's existing interned name: callers + // spell the same directory with and without a trailing slash, and rewriting + // `dir` to the other spelling races every unlocked `Entry::dir()` reader. + let dir: &'static [u8] = if let Some(existing) = in_place { + // SAFETY: in_place points to BSSMap-owned DirEntry + unsafe { (*existing).dir } + } else if !had_handle { + DirnameStore::instance().append(dir_maybe_trail_slash)? } else { // Intern into DirnameStore so the cache entry never dangles — `append` is a // bump-pointer copy and dedups against the singleton, so cost is bounded. diff --git a/src/resolver/lib.rs b/src/resolver/lib.rs index 52a626aea0b8..0359fc8c614d 100644 --- a/src/resolver/lib.rs +++ b/src/resolver/lib.rs @@ -1305,14 +1305,15 @@ pub mod fs { }); // if we get this far, it's a real directory, so we can just store the dir name. - let dir: &'static [u8] = if !had_handle { - if let Some(existing) = in_place { - // SAFETY: `in_place` points to a `DirEntry` inside the BSSMap singleton; - // its `dir` field is DirnameStore-interned (&'static). - unsafe { (*existing).dir } - } else { - DirnameStore::instance().append_slice(dir_maybe_trail_slash)? - } + // An in-place refresh always keeps the slot's existing interned name: callers + // spell the same directory with and without a trailing slash, and rewriting + // `dir` to the other spelling races every unlocked `Entry::dir()` reader. + let dir: &'static [u8] = if let Some(existing) = in_place { + // SAFETY: `in_place` points to a `DirEntry` inside the BSSMap singleton; + // its `dir` field is DirnameStore-interned (&'static). + unsafe { (*existing).dir } + } else if !had_handle { + DirnameStore::instance().append_slice(dir_maybe_trail_slash)? } else { // Intern into DirnameStore so the cache entry never dangles — // `append_slice` is a bump-pointer copy, cost is bounded. diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index 1deea6ede33c..3e9fc7bca855 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -1766,7 +1766,11 @@ impl<'a> Resolver<'a> { // panic on EACCES/EMFILE/ELOOP here. let file = bun_sys::open(span, bun_sys::O::RDONLY, 0) .map_err(Into::::into)?; - query.entry().set_cache_fd(file); + { + // 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()); } @@ -1784,6 +1788,8 @@ impl<'a> Resolver<'a> { 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(); @@ -1801,9 +1807,13 @@ impl<'a> Resolver<'a> { bstr::BStr::new(path.text()) )); } - query - .entry() - .set_cache_symlink(Interned::from_static(symlink)); + { + // 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; } @@ -6224,13 +6234,16 @@ impl<'a> Resolver<'a> { && !lookup.entry().cache().fd.is_valid() && self.store_fd { + // Every cached-`Entry` rewrite takes the per-entry mutex. + let _entry_guard = lookup.entry().mutex.lock_guard(); lookup.entry().set_cache_fd(entries_fd); } - // SAFETY: EntryStore-owned slot; `entries_mutex` held — read-only borrow, + // SAFETY: EntryStore-owned slot — read-only borrow, // dies (NLL) before any later `&mut` to this slot. let entry = lookup.entry(); - // SAFETY: entries_mutex held; `rfs_ptr` points at the process-global RealFS. + // SAFETY: `rfs_ptr` points at the process-global RealFS; the lazy-stat + // rewrite inside `symlink()` is serialized on `Entry.mutex`. let mut symlink = unsafe { entry.symlink(rfs_ptr, self.store_fd) }; if !symlink.is_empty() { if let Some(logs) = self.debug_logs.as_mut() { @@ -6270,9 +6283,13 @@ impl<'a> Resolver<'a> { .ok(); logs.add_note(buf); } - lookup - .entry() - .set_cache_symlink(Interned::from_static(symlink)); + { + // Every cached-`Entry` rewrite takes the per-entry mutex. + let _entry_guard = lookup.entry().mutex.lock_guard(); + lookup + .entry() + .set_cache_symlink(Interned::from_static(symlink)); + } info.abs_real_path = symlink; } } diff --git a/src/router/lib.rs b/src/router/lib.rs index 743513c9ff9b..b014cfeda210 100644 --- a/src/router/lib.rs +++ b/src/router/lib.rs @@ -796,83 +796,94 @@ impl<'a> RouteLoader<'a> { ) { let fs = self.fs; - if let Some(entries) = root_dir_info.get_entries_const() { - let iter = entries.iter(); - 'outer: for entry_ptr in iter { - // NOTE: `iter()` yields raw `*mut Entry`. Reborrow locally for - // each access so `&` reads and the `&mut` `kind()` call do not - // overlap. Single iterator active for this scan; serialized via - // `RealFS.entries_mutex`. - // SAFETY: EntryStore-owned, valid for process lifetime. - if unsafe { &*entry_ptr }.base()[0] == b'.' { - continue 'outer; - } + // Snapshot the cached `DirEntry`'s entry pointers under `entries_mutex`: + // another thread (e.g. the bundler's resolver) rewrites this map in place + // under that lock, so iterating it live would walk freed buckets. + let entry_ptrs: Vec<*mut Fs::Entry> = { + let _entries_lock = fs.fs.entries_mutex.lock_guard(); + match root_dir_info.get_entries_const() { + Some(entries) => entries.iter().collect(), + None => return, + } + }; + // NOTE: the guard is dropped before this loop on purpose — the + // `read_dir_info_ignore_error` recursion below re-acquires `entries_mutex`. + 'outer: for entry_ptr in entry_ptrs { + // NOTE: the snapshot yields raw `*mut Entry`. Reborrow locally for + // each access so `&` reads and the `kind()` call do not + // overlap; the lazy-stat rewrite inside `kind()` is serialized on + // the per-entry `Entry.mutex`. + // SAFETY: EntryStore-owned, valid for process lifetime. + if unsafe { &*entry_ptr }.base()[0] == b'.' { + continue 'outer; + } - // Thread the resolver's fs `Implementation` through — - // `Entry.kind` derefs it to lazily stat when `need_stat` is - // true, so null would be a latent crash / silent route-drop - // once the stub forwards it. - // SAFETY: no other live borrow of `*entry_ptr` here; entries_mutex - // held; `resolver.fs_impl()` points at the process-global RealFS. - let kind = unsafe { (&*entry_ptr).kind(resolver.fs_impl(), false) }; - // SAFETY: shared read-only borrow for the match arms; the only - // subsequent mutation is via `Route::parse` which takes the raw - // pointer and reborrows internally. - let entry: &Fs::Entry = unsafe { &*entry_ptr }; - match kind { - Fs::EntryKind::Dir => { - for banned_dir in BANNED_DIRS.iter() { - if entry.base() == *banned_dir { - continue 'outer; - } + // Thread the resolver's fs `Implementation` through — + // `Entry.kind` derefs it to lazily stat when `need_stat` is + // true, so null would be a latent crash / silent route-drop + // once the stub forwards it. + // SAFETY: no other live borrow of `*entry_ptr` here; + // `resolver.fs_impl()` points at the process-global RealFS. + let kind = unsafe { (&*entry_ptr).kind(resolver.fs_impl(), false) }; + // SAFETY: shared read-only borrow for the match arms; the only + // subsequent mutation is via `Route::parse` which takes the raw + // pointer and reborrows internally. + let entry: &Fs::Entry = unsafe { &*entry_ptr }; + match kind { + Fs::EntryKind::Dir => { + for banned_dir in BANNED_DIRS.iter() { + if entry.base() == *banned_dir { + continue 'outer; } + } - let abs_parts = [entry.dir(), entry.base()]; - if let Some(dir_info) = - resolver.read_dir_info_ignore_error(fs.abs(&abs_parts)) - { - self.load(resolver, &dir_info, base_dir); - } + let abs_parts = [entry.dir(), entry.base()]; + if let Some(dir_info) = resolver.read_dir_info_ignore_error(fs.abs(&abs_parts)) + { + self.load(resolver, &dir_info, base_dir); } + } - Fs::EntryKind::File => { - let extname = bun_paths::extension(entry.base()); - // exclude "." or "" - if extname.len() < 2 { - continue; - } + Fs::EntryKind::File => { + let extname = bun_paths::extension(entry.base()); + // exclude "." or "" + if extname.len() < 2 { + continue; + } - for _extname in self.config.extensions.iter() { - if &extname[1..] == _extname.as_ref() { - // length is extended by one - // entry.dir is a string with a trailing slash - let entry_dir = entry.dir(); - if cfg!(debug_assertions) { - debug_assert!(bun_paths::resolve_path::is_sep_any( - entry_dir[base_dir.len() - 1] - )); - } + for _extname in self.config.extensions.iter() { + if &extname[1..] == _extname.as_ref() { + // `entry.dir()` is `base_dir` or a subdirectory of it, cached + // with or without a trailing slash depending on which resolver + // spelled it first (`base_dir` always has one). Both spellings + // trim to the same `public_dir`. + let entry_dir = entry.dir(); + debug_assert!(entry_dir.len() + 1 >= base_dir.len()); + if entry_dir.len() >= base_dir.len() { + debug_assert!(bun_paths::resolve_path::is_sep_any( + entry_dir[base_dir.len() - 1] + )); + } - // SAFETY: entry.dir is at least base_dir.len()-1 bytes; verified above in debug - let public_dir = &entry_dir[base_dir.len() - 1..entry_dir.len()]; - - // SAFETY: `entry_ptr` is EntryStore-owned (process - // lifetime) with no other live `&mut` borrow here. - let route = unsafe { - Route::parse( - entry.base(), - extname, - entry_ptr, - self.log, - public_dir, - self.route_dirname_len, - ) - }; - if let Some(route) = route { - self.append_route(route); - } - break; + // SAFETY: entry.dir is at least base_dir.len()-1 bytes; verified above in debug + let public_dir = &entry_dir[base_dir.len() - 1..entry_dir.len()]; + + // SAFETY: `entry_ptr` is EntryStore-owned (process + // lifetime) with no other live `&mut` borrow here. + let route = unsafe { + Route::parse( + entry.base(), + extname, + entry_ptr, + self.log, + public_dir, + self.route_dirname_len, + ) + }; + if let Some(route) = route { + self.append_route(route); } + break; } } } @@ -1132,6 +1143,11 @@ impl Route { }; if abs_path_str.is_empty() { + // The reads of `cache().fd` and the `set_abs_path` write below + // rewrite the cached `Entry`; serialize them on the per-entry + // mutex (the same lock every other `Entry` rewrite path takes). + // SAFETY: see fn-level NOTE — read-only reborrow. + let _entry_guard = unsafe { &*entry }.mutex.lock_guard(); // NOTE: reshaped for borrowck — `defer if (needs_close) file.close()` // becomes a scopeguard owning the Option; `needs_close` is a // Cell so the drop closure can read it while the body still mutates. diff --git a/src/runtime/api/filesystem_router.rs b/src/runtime/api/filesystem_router.rs index c8be0bf8b389..22b263499156 100644 --- a/src/runtime/api/filesystem_router.rs +++ b/src/runtime/api/filesystem_router.rs @@ -386,8 +386,18 @@ impl FileSystemRouter { }; if let Some(dir_ref) = root_dir_info { - if let Some(entries) = dir_ref.get_entries_const() { - 'outer: for &entry_ptr in entries.data.values() { + // Snapshot the cached `DirEntry`'s entry pointers under `entries_mutex` + // (other threads rewrite the map in place under that lock), then drop + // the guard: `bust_dir_cache` / the recursion below re-acquire it. + let entry_ptrs: Vec<*mut Fs::Entry> = { + let _entries_lock = Fs::FileSystem::instance().fs.entries_mutex.lock_guard(); + match dir_ref.get_entries_const() { + Some(entries) => entries.data.values().copied().collect(), + None => Vec::new(), + } + }; + { + 'outer: for entry_ptr in entry_ptrs { // BACKREF: `entry_ptr` is a `*mut Entry` into the process-static // EntryStore; the store outlives this loop. Wrap once so the // shared-only reads below are safe `Deref`s. @@ -405,8 +415,8 @@ impl FileSystemRouter { let kind = { let fs_impl = &mut vm.transpiler.fs_mut().fs; // SAFETY: `entry_ptr` is a live `*mut Entry` in the process-static - // EntryStore (checked non-null above); no shared `&Entry` is live - // here. entries_mutex held; fs_impl is the process-global RealFS. + // EntryStore (checked non-null above); the lazy-stat rewrite is + // serialized on `Entry.mutex`; fs_impl is the process-global RealFS. unsafe { (&*entry_ptr).kind(fs_impl, false) } }; if kind == Fs::EntryKind::Dir { diff --git a/src/runtime/bake/FrameworkRouter.rs b/src/runtime/bake/FrameworkRouter.rs index 4f7fb1f503a5..345f3a4d8e63 100644 --- a/src/runtime/bake/FrameworkRouter.rs +++ b/src/runtime/bake/FrameworkRouter.rs @@ -1536,7 +1536,7 @@ impl FrameworkRouter { // `addr_of_mut!` only computes a field address without forming a reference. let fs_impl = unsafe { core::ptr::addr_of_mut!((*fs).fs) }; - if let Some(entries) = dir_info.get_entries_const() { + { // Note: `entries.data` is backed by `std::collections::HashMap`, // whose iteration order is unspecified. The route-tree child order // is this iteration order (see `insert`), and @@ -1550,14 +1550,23 @@ impl FrameworkRouter { *mut bun_resolver::fs::Entry, ZigStringHashContext, > = Default::default(); - for (k, &v) in entries.data.iter() { - let _ = zig_order.put(Box::from(&**k), v); + { + // Copy under `entries_mutex`: other threads rewrite the cached + // `DirEntry` map in place under that lock. Dropped before the + // walk so the `read_dir_info_ignore_error` recursion can re-lock. + let _entries_lock = fs_ref.fs.entries_mutex.lock_guard(); + if let Some(entries) = dir_info.get_entries_const() { + for (k, &v) in entries.data.iter() { + let _ = zig_order.put(Box::from(&**k), v); + } + } } let mut it = zig_order.iter(); 'outer: while let Some(entry) = it.next() { let file_ptr: *mut bun_resolver::fs::Entry = *entry.1; - // SAFETY: EntryMap stores `*mut Entry` into the EntryStore singleton; entries - // outlive this scan and are serialized via `RealFS.entries_mutex`. + // SAFETY: EntryMap stores `*mut Entry` into the EntryStore singleton + // (process lifetime); the lazy-stat rewrite in `kind()` below is + // serialized on the per-entry `Entry.mutex`. let file = unsafe { &*file_ptr }; let base = file.base(); // Note: reshaped for borrowck — fetch type fields fresh each iteration. diff --git a/test/js/bun/util/filesystem_router.test.ts b/test/js/bun/util/filesystem_router.test.ts index 4831bdb1067c..0ca15bbd12f8 100644 --- a/test/js/bun/util/filesystem_router.test.ts +++ b/test/js/bun/util/filesystem_router.test.ts @@ -1,7 +1,7 @@ import { FileSystemRouter } from "bun"; import { expect, it } from "bun:test"; import fs, { mkdirSync, rmSync } from "fs"; -import { bunEnv, bunExe, isASAN, isMacOS, isWindows, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isASAN, isMacOS, isWindows, normalizeBunSnapshot, tempDir, tmpdirSync } from "harness"; import path, { dirname } from "path"; function createTree(basedir: string, paths: string[]) { @@ -734,3 +734,87 @@ it("match() does not panic on a leading '?' or a path that percent-decodes to em }); expect(exitCode).toBe(0); }); + +it("reload() while Bun.build() resolves the same directory", async () => { + // The router's route-load loop and Bun.build's entry-point resolution (which + // runs on the bundler thread) share the process-global directory-entry cache. + // Run in a subprocess so a crash is observable as a signal instead of taking + // down the test runner. + const files: Record = { + "fixture.ts": /* ts */ ` + import path from "path"; + const pagesDir = path.join(import.meta.dir, "pages"); + const entrypoints: string[] = []; + for (let i = 1; i <= 40; i++) { + entrypoints.push(path.join(pagesDir, "p" + i + ".tsx")); + entrypoints.push(path.join(pagesDir, "sub", "s" + i + ".tsx")); + } + const router = new Bun.FileSystemRouter({ + dir: pagesDir, + style: "nextjs", + fileExtensions: [".tsx"], + }); + const builds = Array.from({ length: 4 }, () => + Bun.build({ entrypoints, target: "bun", throw: false }), + ); + let matches = 0; + for (let i = 0; i < 50; i++) { + router.reload(); + const m = router.match("/p7"); + if (m && m.filePath.endsWith("p7.tsx")) matches++; + } + const results = await Promise.all(builds); + console.log("matches", matches, "builds-ok", results.every(r => r.success)); + `, + }; + for (let i = 1; i <= 40; i++) { + files[`pages/p${i}.tsx`] = `export default ${i};\n`; + files[`pages/sub/s${i}.tsx`] = `export default ${i};\n`; + } + using dir = tempDir("fsr-reload-build-race", files); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(normalizeBunSnapshot(stdout, String(dir))).toBe("matches 50 builds-ok true"); + expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 0, signalCode: null }); +}, 60_000); + +it("loads routes from a directory already cached by Bun.build()", async () => { + // The resolver caches the directory name without a trailing slash while the + // router spells it with one; loading routes out of the already-populated + // entry cache must accept either spelling. Run in a subprocess so a crash is + // observable as a nonzero exit instead of taking down the test runner. + using dir = tempDir("fsr-prewarmed-entry-cache", { + "fixture.ts": /* ts */ ` + import path from "path"; + const pagesDir = path.join(import.meta.dir, "pages"); + await Bun.build({ entrypoints: [path.join(pagesDir, "a.tsx")], target: "bun", throw: false }); + const router = new Bun.FileSystemRouter({ + dir: pagesDir, + style: "nextjs", + fileExtensions: [".tsx"], + }); + console.log(Object.keys(router.routes).sort().join(" "), router.match("/b")?.name); + `, + "pages/a.tsx": "export default 1;\n", + "pages/b.tsx": "export default 2;\n", + "pages/sub/c.tsx": "export default 3;\n", + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(normalizeBunSnapshot(stdout, String(dir))).toBe("/a /b /sub/c /b"); + expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 0, signalCode: null }); +});