resolver: keep the key of a busted folder, so a miss is retried after another thread's bust

src/bun_alloc/lib.rs

@@ -2241,11 +2241,18 @@ impl<ValueType, const COUNT: usize, const REMOVE_TRAILING_SLASHES: bool>
         Ok(ret)
     }
 
-    /// Returns true if the entry was removed.
+    /// Forgets the value of the key. Returns true if the key was ever looked up, also after an earlier `remove`.
     pub fn remove(&mut self, denormalized_key: &[u8]) -> bool {
         let _guard = self.mutex.lock();
         let _key = Self::key_hash(denormalized_key);
-        self.index.remove(&_key).is_some()
+        // The key stays: a caller that retries a miss after `remove` must not depend on who removed first.
+        match self.index.get_mut(&_key) {
+            Some(index) => {
+                *index = UNASSIGNED;
+                true
+            }
+            None => false,
+        }
     }
 }
 

src/bundler/bundle_v2.rs

@@ -2736,7 +2736,7 @@ pub mod bv2_impl {
                         if err == _resolver::Error::ModuleNotFound {
                             if let Some(dev) = &self.dev_server {
                                 if !had_busted_dir_cache {
-                                    // Only re-query if we previously had something cached.
+                                    // Only re-query if the folder was ever looked up. Another thread can have busted it first.
                                     // SAFETY: see `transpiler` note above.
                                     if unsafe { &mut *transpiler }
                                         .resolver
@@ -6773,7 +6773,7 @@ pub mod bv2_impl {
                                             bstr::BStr::new(&source.path.text),
                                             bstr::BStr::new(&import_record.path.text)
                                         );
-                                        // Only re-query if we previously had something cached.
+                                        // Only re-query if the folder was ever looked up. Another thread can have busted it first.
                                         if transpiler.resolver.bust_dir_cache_from_specifier(
                                             source.path.text,
                                             import_record.path.text,

src/bundler/transpiler.rs

@@ -390,7 +390,7 @@ impl<'a> Transpiler<'a> {
                     )
                 };
 
-                // Only re-query if we previously had something cached.
+                // Only re-query if the folder was ever looked up. Another thread can have busted it first.
                 if busted {
                     if let Ok(result) = self._resolve_entry_point(entry_point, dot_slash) {
                         return self.reject_unbundleable_entry_point(result, entry_point);

src/jsc/VirtualMachine.rs

@@ -5662,7 +5662,7 @@ impl VirtualMachine {
                         .as_bytes()
                     };
 
-                    // Only re-query if we previously had something cached.
+                    // Only re-query if the folder was ever looked up. Another thread can have busted it first.
                     if self.transpiler.resolver.bust_dir_cache(
                         bun_paths::string_paths::without_trailing_slash_windows_path(buster_name),
                     ) || busted_as_spelled

src/jsc/hot_reloader.rs

@@ -184,7 +184,7 @@ pub trait HotReloaderCtx {
     /// Called from `Task::run` to perform the actual reload.
     fn reload(&mut self);
 
-    /// Returns whether anything was busted.
+    /// Returns whether the folder was ever looked up.
     fn bust_dir_cache(&mut self, path: &[u8]) -> bool;
 
     /// `&transpiler.options.loaders`.

src/resolver/lib.rs

@@ -1185,7 +1185,7 @@ pub mod fs {
         }
 
         /// Evicts `file_path` from the directory-entry cache; returns whether
-        /// an entry was removed.
+        /// it was ever looked up, also when another thread evicted it first.
         pub(crate) fn bust_entries_cache(&mut self, file_path: &[u8]) -> bool {
             // `entries` is the process-global
             // BSSMap singleton and `remove` mutates it; callers (transpiler /
@@ -2303,3 +2303,47 @@ pub mod package_names;
 pub mod resolver;
 pub mod result;
 pub mod standalone_module_graph;
+
+#[cfg(test)]
+mod dir_cache_tests {
+    use bun_alloc::{BSSMapInner, ItemStatus};
+
+    /// A caller retries a miss when `remove` says that the folder was cached. Another thread can remove it first.
+    #[test]
+    fn remove_answers_for_a_folder_that_another_caller_removed_first() {
+        // SAFETY: `init` returns a valid map that nothing else holds.
+        let map = unsafe { BSSMapInner::<u32, 4, true>::init().as_mut() };
+
+        assert!(!map.remove(b"/never/looked/up"));
+        assert!(!map.remove(b"/never/looked/up"));
+
+        let mut cached = map.get_or_put(b"/cached").unwrap();
+        map.put(&mut cached, 7).unwrap();
+        assert_eq!(map.get(b"/cached").copied(), Some(7));
+        assert!(map.remove(b"/cached"));
+        assert!(map.remove(b"/cached"));
+        // The key reads as one that was never put.
+        assert!(map.get(b"/cached").is_none());
+        let mut again = map.get_or_put(b"/cached").unwrap();
+        assert!(matches!(again.status, ItemStatus::Unknown));
+        assert!(!again.has_checked_if_exists());
+        // The next value goes in under the kept key.
+        map.put(&mut again, 9).unwrap();
+        assert_eq!(map.get(b"/cached").copied(), Some(9));
+        assert!(map.remove(b"/cached"));
+        assert!(map.get(b"/cached").is_none());
+
+        let missing = map.get_or_put(b"/missing").unwrap();
+        map.mark_not_found(missing);
+        assert!(matches!(
+            map.get_or_put(b"/missing").unwrap().status,
+            ItemStatus::NotFound
+        ));
+        assert!(map.remove(b"/missing"));
+        assert!(map.remove(b"/missing"));
+        assert!(matches!(
+            map.get_or_put(b"/missing").unwrap().status,
+            ItemStatus::Unknown
+        ));
+    }
+}

src/resolver/resolver.rs

@@ -2818,6 +2818,7 @@ impl<'a> Resolver<'a> {
     }
 
     /// Bust the directory cache for the given path, which may end in a separator.
+    /// True when the folder was ever looked up, also when another thread busted it first.
     pub fn bust_dir_cache(&mut self, path: &[u8]) -> bool {
         let path = strings::without_trailing_slash_windows_path(path);
         Self::assert_valid_cache_key(path);

test/cli/hot/hot.test.ts

@@ -1332,3 +1332,58 @@ it("--hot does not emit beforeExit to the listeners of a failed run", async () =
   expect(await stdout("still running")).toBe(true);
   expect(await runner.exited).toBe(0);
 });
+
+// The watcher thread drops the folder of each write. A lookup that failed on the old listing must still be tried again.
+it.each(["--hot", "--watch"])(
+  "%s finds each module that the program writes and looks up at once",
+  async flag => {
+    using dir = tempDir("hot-look-up-own-write", {
+      "main.js": `import fs from "node:fs";
+import { createRequire } from "node:module";
+import path from "node:path";
+
+const require = createRequire(import.meta.url);
+const here = import.meta.dir;
+const missed = [];
+const write = name => fs.writeFileSync(path.join(here, name), "export const v = 1;\\n");
+
+// As a program does it. A run that the watcher started again does not write the file a second time.
+if (!fs.existsSync(path.join(here, "first.js"))) write("./first.js");
+await import("./first.js").catch(() => missed.push("./first.js"));
+
+// More folders to look in, which do not exist, keep one lookup at work for longer: the watcher
+// thread then drops the folder while that lookup runs.
+const paths = [here, ...Array.from({ length: 192 }, (_, i) => path.join(here, "nowhere", String(i)))];
+for (let i = 0; i < 200; i++) {
+  const name = "./gen" + i + ".js";
+  write(name);
+  try {
+    // A lookup that finds another file is a miss too.
+    if (fs.realpathSync(require.resolve(name, { paths })) !== fs.realpathSync(path.join(here, name))) missed.push(name);
+  } catch {
+    missed.push(name);
+  }
+}
+console.log(JSON.stringify(missed));
+process.exit(0);
+`,
+    });
+    await using proc = spawn({
+      cmd: [bunExe(), flag, "--no-clear-screen", "main.js"],
+      cwd: String(dir),
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+      stdin: "ignore",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    // The watcher can handle the write of "first.js" after the import watches that file, and then
+    // the program starts again, once. Each run that comes to its end prints what it missed.
+    expect({ stdout: [...new Set(stdout.trim().split("\n"))], stderr, exitCode }).toEqual({
+      stdout: ["[]"],
+      stderr: "",
+      exitCode: 0,
+    });
+  },
+  longTimeout,
+);

test/js/bun/resolve/resolve.test.ts

@@ -356,6 +356,92 @@ it.if(isWindows)("import() of a missing file at the drive root rejects", async (
   await expect(import(`${drive}///doesnotexist.ts`)).rejects.toThrow();
 });
 
+// A lookup that fails drops the cached listing of the folder, and is tried again only when the
+// folder was listed. Another thread can drop the listing first: the watcher thread of `bun --hot`
+// does, and here a Worker does.
+it("finds a module that is written and looked up at once when a Worker dropped the listing of its folder first", async () => {
+  using dir = tempDir("resolve-listing-dropped-first", {
+    "gen/first.js": "export const v = 0;\n",
+    "main.js": `import fs from "node:fs";
+import { createRequire } from "node:module";
+import path from "node:path";
+import { Worker, isMainThread, workerData } from "node:worker_threads";
+
+const require = createRequire(import.meta.url);
+const here = import.meta.dir;
+
+if (isMainThread) {
+  // [0] counts the modules that this thread wrote, [1] is set when the Worker runs, [2] is the count that the Worker last answered.
+  const gate = new Int32Array(new SharedArrayBuffer(12));
+  new Worker(import.meta.path, { workerData: gate });
+  // This thread never goes back to the event loop, so no error event of the Worker reaches it. Each wait for the Worker has a limit.
+  const stopped = what => {
+    console.error("the Worker stopped before its " + what + " lookup");
+    process.exit(1);
+  };
+  if (Atomics.wait(gate, 1, 0, 5000) === "timed-out") stopped("first");
+
+  // More folders to look in, which do not exist, keep one lookup at work while the Worker drops the listing.
+  const paths = [here, ...Array.from({ length: 192 }, (_, i) => path.join(here, "nowhere", String(i)))];
+  const missed = [];
+  for (let i = 0; i < 200; i++) {
+    // The folder is listed before the write.
+    require.resolve("./gen/first.js");
+    const name = "./gen/gen" + i + ".js";
+    fs.writeFileSync(path.join(here, name), "export const v = 1;\\n");
+    Atomics.add(gate, 0, 1);
+    Atomics.notify(gate, 0);
+    try {
+      // A lookup that finds another file is a miss too.
+      if (fs.realpathSync(require.resolve(name, { paths })) !== fs.realpathSync(path.join(here, name))) missed.push(name);
+    } catch {
+      missed.push(name);
+    }
+  }
+  // One more count: the Worker answers it only if it still runs.
+  const last = Atomics.add(gate, 0, 1) + 1;
+  Atomics.notify(gate, 0);
+  for (let answered = Atomics.load(gate, 2); answered < last; answered = Atomics.load(gate, 2)) {
+    if (Atomics.wait(gate, 2, answered, 5000) === "timed-out") stopped("last");
+  }
+  console.log(JSON.stringify(missed));
+  process.exit(0);
+} else {
+  const gate = workerData;
+  // This lookup fails in the folder "gen", and drops its listing. The first lookup of a thread is slow.
+  const lookUp = () => {
+    try {
+      require.resolve("gen/missing");
+    } catch {}
+  };
+  lookUp();
+  Atomics.store(gate, 1, 1);
+  Atomics.notify(gate, 1);
+  for (let seen = 0; ; ) {
+    Atomics.wait(gate, 0, seen);
+    // The count is read before the lookup, so a module that is written during the lookup starts one more.
+    seen = Atomics.load(gate, 0);
+    lookUp();
+    Atomics.store(gate, 2, seen);
+    Atomics.notify(gate, 2);
+  }
+}
+`,
+  });
+  await using proc = Bun.spawn({
+    // The package name that fails must not go to the registry.
+    cmd: [bunExe(), "--no-install", "main.js"],
+    cwd: String(dir),
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+    stdin: "ignore",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({ stdout, stderr, exitCode }).toEqual({ stdout: "[]\n", stderr: "", exitCode: 0 });
+  // The program waits up to 5 s for its Worker, two times. The limit of the test is above that, so the program reports a Worker that stopped.
+}, 20_000);
+
 describe("NODE_PATH test", () => {
   const prepareTest = () => {
     const tempDir = tempDirWithFiles("node_path", {