resolver: take entries_mutex in entries_at before the in-place DirEntry rewrite

src/resolver/dir_info.rs

@@ -226,16 +226,10 @@ impl DirInfo {
 
     pub fn get_file_descriptor(&self) -> Fd {
         if FeatureFlags::STORE_FILE_DESCRIPTORS {
-            // Route through `entries_at` directly (returns `Option<&mut EntriesOption>`)
-            // instead of round-tripping the safe `&mut DirEntry` through `get_entries`'s
-            // `*mut` return just to deref it back here. With `generation = 0` the
-            // generation-check re-read in `entries_at` is a no-op, so this is the
-            // same lookup `get_entries(0)` performs.
-            if let Some(fs::EntriesOption::Entries(entries)) =
-                fs::FileSystem::instance().fs.entries_at(self.entries, 0)
-            {
-                return entries.fd;
-            }
+            // `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.
+            return self.get_entries_const().map_or(Fd::INVALID, |e| e.fd);
         }
         Fd::INVALID
     }
@@ -270,6 +264,19 @@ impl DirInfo {
         }
     }
 
+    /// [`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 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)?;
+        match entries_ptr {
+            fs::EntriesOption::Entries(entries) => Some(&**entries),
+            fs::EntriesOption::Err(_) => None,
+        }
+    }
+
     pub fn get_entries_const(&self) -> Option<&fs::DirEntry> {
         let entries_ptr = fs::FileSystem::instance()
             .fs

src/resolver/lib.rs

@@ -1597,14 +1597,37 @@ 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 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).
+        pub fn entries_at_locked(
+            &mut self,
+            index: bun_alloc::IndexType,
+            generation: Generation,
+        ) -> Option<&mut EntriesOption> {
+            debug_assert!(
+                self.entries_mutex.is_held_by_current_thread(),
+                "entries_at_locked: caller must hold entries_mutex",
+            );
             // erase to raw immediately so re-borrowing `&mut self` for
             // `open_dir`/`readdir`/`read_directory_error` doesn't conflict.
-            // `entries_mutex` held by caller; sole `&mut` to this slot.
+            // `entries_mutex` held (by `entries_at` or the caller); sole `&mut` to this slot.
             let result_ptr = std::ptr::from_mut::<EntriesOption>(self.entries.at_index(index)?);
             // SAFETY: BSSMap-owned slot; uniquely held under `entries_mutex`.
             if let EntriesOption::Entries(existing) = unsafe { &mut *result_ptr } {

src/resolver/resolver.rs

@@ -3466,7 +3466,10 @@ impl<'a> Resolver<'a> {
                 unsafe { &mut *rfs }
             };
         }
-        // resolver mutex held; `EntriesMap` methods are safe wrappers over the singleton.
+        // Hold `entries_mutex` across the in-place `DirEntry` rewrite below and
+        // the `dir_info_uncached` call, mirroring `dir_info_cached_miss`: the
+        // route loaders iterate the `DirEntry.data` map under this lock.
+        let _entries_unlock = rfs!().entries_mutex.lock_guard();
         let mut cached_dir_entry_result = rfs!().entries.get_or_put(dir_path)?;
 
         // NOTE: always assigned by either the cached-hit arm or the
@@ -6201,7 +6204,10 @@ impl<'a> Resolver<'a> {
 
             // Make sure "absRealPath" is the real path of the directory (resolving any symlinks)
             if !self.opts.preserve_symlinks {
-                if let Some(parent_entries) = parent_.get_entries_ref(self.generation) {
+                // The only caller that reaches this with `parent` set
+                // (`dir_info_cached_miss`) already holds `entries_mutex`, and that
+                // mutex is non-recursive, so go through the `_locked` accessor.
+                if let Some(parent_entries) = parent_.get_entries_ref_locked(self.generation) {
                     if let Some(lookup) = parent_entries.get(base) {
                         let entries_fd = entries!().fd;
                         if entries_fd.is_valid()

test/js/bun/util/filesystem_router.test.ts

@@ -836,17 +836,26 @@ it("reload() while Bun.build() resolves the same directory", async () => {
         style: "nextjs",
         fileExtensions: [".tsx"],
       });
-      const builds = Array.from({ length: 4 }, () =>
-        Bun.build({ entrypoints, target: "bun", throw: false }),
-      );
+      // The first build completes with generation 0 and the bundle thread then
+      // bumps its generation, so every later build's resolver re-reads the
+      // directory listing in place. reload() iterates the same listing on the
+      // main thread, and that in-place re-read is what the reload loop races.
+      await 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++;
+      let buildsOk = true;
+      for (let round = 0; round < 40; round++) {
+        const builds = Array.from({ length: 4 }, () =>
+          Bun.build({ entrypoints, target: "bun", throw: false }),
+        );
+        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);
+        buildsOk &&= results.every(r => r.success);
       }
-      const results = await Promise.all(builds);
-      console.log("matches", matches, "builds-ok", results.every(r => r.success));
+      console.log("matches", matches, "builds-ok", buildsOk);
     `,
   };
   for (let i = 1; i <= 40; i++) {
@@ -863,8 +872,12 @@ it("reload() while Bun.build() resolves the same directory", async () => {
     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 });
+  expect({
+    stdout: normalizeBunSnapshot(stdout, String(dir)),
+    stderr: normalizeBunSnapshot(stderr, String(dir)),
+    exitCode,
+    signalCode: proc.signalCode,
+  }).toEqual({ stdout: "matches 2000 builds-ok true", stderr: "", exitCode: 0, signalCode: null });
 }, 60_000);
 
 it("loads routes from a directory already cached by Bun.build()", async () => {
@@ -897,6 +910,10 @@ it("loads routes from a directory already cached by Bun.build()", async () => {
     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 });
+  expect({
+    stdout: normalizeBunSnapshot(stdout, String(dir)),
+    stderr: normalizeBunSnapshot(stderr, String(dir)),
+    exitCode,
+    signalCode: proc.signalCode,
+  }).toEqual({ stdout: "/a /b /sub/c /b", stderr: "", exitCode: 0, signalCode: null });
 });