fix: store PathBuf with weak pointers to handle cache clearing (#879)

fixes of #872

The solution is not optimal, I spent the whole day trying to understand synchronization but couldn't figure it out, so resorting to this really bad implementation.

The good news is that I came up with a simple failing test for future improvements.

Author: Boshen

src/cache/cache_impl.rs

@@ -59,7 +59,7 @@ impl<Fs: FileSystem> Cache<Fs> {
         let is_node_modules = path.file_name().as_ref().is_some_and(|&name| name == "node_modules");
         let inside_node_modules =
             is_node_modules || parent.as_ref().is_some_and(|parent| parent.inside_node_modules);
-        let parent_weak = parent.as_ref().map(|p| Arc::downgrade(&p.0));
+        let parent_weak = parent.as_ref().map(|p| (Arc::downgrade(&p.0), p.to_path_buf()));
         let cached_path = CachedPath(Arc::new(CachedPathImpl::new(
             hash,
             path.to_path_buf().into_boxed_path(),
@@ -138,7 +138,7 @@ impl<Fs: FileSystem> Cache<Fs> {
         let mut path = path.clone();
         // Go up directories when the querying path is not a directory
         while !self.is_dir(&path, ctx) {
-            if let Some(cv) = path.parent() {
+            if let Some(cv) = path.parent(self) {
                 path = cv;
             } else {
                 break;
@@ -168,7 +168,7 @@ impl<Fs: FileSystem> Cache<Fs> {
                     if let Some(deps) = &mut ctx.missing_dependencies {
                         deps.push(package_json_path);
                     }
-                    return path.parent().map_or(Ok(None), |parent| {
+                    return path.parent(self).map_or(Ok(None), |parent| {
                         self.find_package_json_impl(&parent, options, ctx)
                     });
                 };
@@ -311,18 +311,25 @@ impl<Fs: FileSystem> Cache<Fs> {
         visited: &mut StdHashSet<u64, BuildHasherDefault<IdentityHasher>>,
     ) -> Result<CachedPath, ResolveError> {
         // Check cache first - if this path was already canonicalized, return the cached result
-        if let Some(weak) = path.canonicalized.get() {
-            return weak.upgrade().map(CachedPath).ok_or_else(|| {
-                io::Error::new(io::ErrorKind::NotFound, "Cached path no longer exists").into()
-            });
+        if let Some((weak, path_buf)) = path.canonicalized.get() {
+            return weak
+                .upgrade()
+                .map(CachedPath)
+                .or_else(|| {
+                    // Weak pointer upgrade failed - recreate from stored PathBuf
+                    Some(self.value(path_buf))
+                })
+                .ok_or_else(|| {
+                    io::Error::new(io::ErrorKind::NotFound, "Cached path no longer exists").into()
+                });
         }
 
         // Check for circular symlink by tracking visited paths in the current canonicalization chain
         if !visited.insert(path.hash) {
             return Err(io::Error::new(io::ErrorKind::NotFound, "Circular symlink").into());
         }
 
-        let res = path.parent().map_or_else(
+        let res = path.parent(self).map_or_else(
             || Ok(path.normalize_root(self)),
             |parent| {
                 self.canonicalize_with_visited(&parent, visited).and_then(|parent_canonical| {
@@ -336,7 +343,7 @@ impl<Fs: FileSystem> Cache<Fs> {
                                 &self.value(&link.normalize()),
                                 visited,
                             );
-                        } else if let Some(dir) = normalized.parent() {
+                        } else if let Some(dir) = normalized.parent(self) {
                             // Symlink is relative `../../foo.js`, use the path directory
                             // to resolve this symlink.
                             return self.canonicalize_with_visited(
@@ -358,7 +365,7 @@ impl<Fs: FileSystem> Cache<Fs> {
 
         // Cache the result before removing from visited set
         // This ensures parent canonicalization results are cached and reused
-        let _ = path.canonicalized.set(Arc::downgrade(&res.0));
+        let _ = path.canonicalized.set((Arc::downgrade(&res.0), res.to_path_buf()));
 
         // Remove from visited set when unwinding the recursion
         visited.remove(&path.hash);

src/cache/cached_path.rs

@@ -21,12 +21,12 @@ pub struct CachedPath(pub Arc<CachedPathImpl>);
 pub struct CachedPathImpl {
     pub hash: u64,
     pub path: Box<Path>,
-    pub parent: Option<Weak<CachedPathImpl>>,
+    pub parent: Option<(Weak<CachedPathImpl>, PathBuf)>,
     pub is_node_modules: bool,
     pub inside_node_modules: bool,
     pub meta: OnceLock<Option<(/* is_file */ bool, /* is_dir */ bool)>>, // None means not found.
-    pub canonicalized: OnceLock<Weak<CachedPathImpl>>,
-    pub node_modules: OnceLock<Option<Weak<CachedPathImpl>>>,
+    pub canonicalized: OnceLock<(Weak<CachedPathImpl>, PathBuf)>,
+    pub node_modules: OnceLock<Option<(Weak<CachedPathImpl>, PathBuf)>>,
     pub package_json: OnceLock<Option<PackageJsonIndex>>,
     /// `tsconfig.json` found at path.
     pub tsconfig: OnceLock<Option<Arc<TsConfig>>>,
@@ -40,7 +40,7 @@ impl CachedPathImpl {
         path: Box<Path>,
         is_node_modules: bool,
         inside_node_modules: bool,
-        parent: Option<Weak<Self>>,
+        parent: Option<(Weak<Self>, PathBuf)>,
     ) -> Self {
         Self {
             hash,
@@ -75,8 +75,14 @@ impl CachedPath {
         self.path.to_path_buf()
     }
 
-    pub(crate) fn parent(&self) -> Option<Self> {
-        self.0.parent.as_ref().and_then(|weak| weak.upgrade().map(CachedPath))
+    pub(crate) fn parent<Fs: FileSystem>(&self, cache: &Cache<Fs>) -> Option<Self> {
+        self.0.parent.as_ref().and_then(|(weak, path_buf)| {
+            weak.upgrade().map(CachedPath).or_else(|| {
+                // Weak pointer upgrade failed - parent was cleared from cache
+                // Recreate it from the stored PathBuf
+                Some(cache.value(path_buf))
+            })
+        })
     }
 
     pub(crate) fn is_node_modules(&self) -> bool {
@@ -104,10 +110,16 @@ impl CachedPath {
     ) -> Option<Self> {
         self.node_modules
             .get_or_init(|| {
-                self.module_directory("node_modules", cache, ctx).map(|cp| Arc::downgrade(&cp.0))
+                self.module_directory("node_modules", cache, ctx)
+                    .map(|cp| (Arc::downgrade(&cp.0), cp.to_path_buf()))
             })
             .as_ref()
-            .and_then(|weak| weak.upgrade().map(CachedPath))
+            .and_then(|(weak, path_buf)| {
+                weak.upgrade().map(CachedPath).or_else(|| {
+                    // Weak pointer upgrade failed - recreate from stored PathBuf
+                    Some(cache.value(path_buf))
+                })
+            })
     }
 
     pub(crate) fn add_extension<Fs: FileSystem>(&self, ext: &str, cache: &Cache<Fs>) -> Self {

src/lib.rs

@@ -309,7 +309,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
             // Go up directories when the querying path is not a directory
             let mut cp = cached_path.clone();
             if !self.cache.is_dir(&cp, ctx)
-                && let Some(cv) = cp.parent()
+                && let Some(cv) = cp.parent(&self.cache)
             {
                 cp = cv;
             }
@@ -319,7 +319,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                     break;
                 }
                 // Skip /node_modules/@scope/package.json
-                if let Some(parent) = p.parent()
+                if let Some(parent) = p.parent(&self.cache)
                     && parent.is_node_modules()
                     && let Some(filename) = p.path().file_name()
                     && filename.as_encoded_bytes().starts_with(b"@")
@@ -329,7 +329,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                 if let Some(package_json) = self.cache.get_package_json(&p, &self.options, ctx)? {
                     last = Some(package_json);
                 }
-                cp = p.parent();
+                cp = p.parent(&self.cache);
             }
             Ok(last)
         } else {
@@ -876,7 +876,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         // 1. let DIRS = NODE_MODULES_PATHS(START)
         // 2. for each DIR in DIRS:
         for module_name in &self.options.modules {
-            for cached_path in std::iter::successors(Some(cached_path.clone()), CachedPath::parent)
+            for cached_path in
+                std::iter::successors(Some(cached_path.clone()), |cp| cp.parent(&self.cache))
             {
                 // Skip if /path/to/node_modules does not exist
                 if !self.cache.is_dir(&cached_path, ctx) {
@@ -913,7 +914,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                         // Skip if the directory lead to the scope package does not exist
                         // i.e. `foo/node_modules/@scope` is not a directory for `foo/node_modules/@scope/package`
                         if package_name.starts_with('@')
-                            && let Some(path) = cached_path.parent().as_ref()
+                            && let Some(path) = cached_path.parent(&self.cache).as_ref()
                             && !self.cache.is_dir(path, ctx)
                         {
                             continue;
@@ -1426,7 +1427,8 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
 
         // 11. While parentURL is not the file system root,
         for module_name in &self.options.modules {
-            for cached_path in std::iter::successors(Some(cached_path.clone()), CachedPath::parent)
+            for cached_path in
+                std::iter::successors(Some(cached_path.clone()), |cp| cp.parent(&self.cache))
             {
                 // 1. Let packageURL be the URL resolution of "node_modules/" concatenated with packageSpecifier, relative to parentURL.
                 let Some(cached_path) = self.get_module_directory(&cached_path, module_name, ctx)

src/tests/clear_cache.rs

@@ -0,0 +1,37 @@
+use std::{env, fs};
+
+use rayon::prelude::*;
+
+use crate::Resolver;
+
+#[test]
+fn test_parallel_resolve_with_clear_cache() {
+    let target_dir = env::current_dir().unwrap().join("./target");
+    let test_dir = target_dir.join("test_clear_cache");
+    let node_modules = test_dir.join("node_modules");
+
+    let _ = fs::remove_dir_all(&test_dir);
+    fs::create_dir_all(&node_modules).unwrap();
+
+    let packages: Vec<String> = (1..=100).map(|i| format!("package_{i}")).collect();
+    for package in &packages {
+        let package_dir = node_modules.join(package);
+        fs::create_dir_all(&package_dir).unwrap();
+        let package_json = format!(r#"{{"name": "{package}", "main": "index.js"}}"#);
+        fs::write(package_dir.join("package.json"), package_json).unwrap();
+        fs::write(package_dir.join("index.js"), "").unwrap();
+    }
+
+    let resolver = Resolver::default();
+    for _ in 1..100 {
+        packages.par_iter().enumerate().for_each(|(i, package)| {
+            if i % 10 == 0 && i > 0 {
+                resolver.clear_cache();
+            }
+            let result = resolver.resolve(&test_dir, package);
+            assert!(result.is_ok(), "Failed to resolve {package}: {result:?}");
+        });
+    }
+
+    let _ = fs::remove_dir_all(&test_dir);
+}

src/tests/mod.rs

@@ -1,6 +1,7 @@
 mod alias;
 mod browser_field;
 mod builtins;
+mod clear_cache;
 mod dependencies;
 mod exports_field;
 mod extension_alias;

src/tsconfig_resolver.rs

@@ -120,7 +120,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
             })? {
                 return Ok(Some(Arc::clone(tsconfig)));
             }
-            cache_value = cv.parent();
+            cache_value = cv.parent(&self.cache);
         }
         Ok(None)
     }