diff --git a/src/cache/cache_impl.rs b/src/cache/cache_impl.rs index 174cfade..e792022c 100644 --- a/src/cache/cache_impl.rs +++ b/src/cache/cache_impl.rs @@ -59,7 +59,7 @@ impl Cache { 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 Cache { 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 Cache { 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,10 +311,17 @@ impl Cache { visited: &mut StdHashSet>, ) -> Result { // 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 @@ -322,7 +329,7 @@ impl Cache { 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 Cache { &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 Cache { // 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); diff --git a/src/cache/cached_path.rs b/src/cache/cached_path.rs index 81c4d1e3..e8449acc 100644 --- a/src/cache/cached_path.rs +++ b/src/cache/cached_path.rs @@ -21,12 +21,12 @@ pub struct CachedPath(pub Arc); pub struct CachedPathImpl { pub hash: u64, pub path: Box, - pub parent: Option>, + pub parent: Option<(Weak, PathBuf)>, pub is_node_modules: bool, pub inside_node_modules: bool, pub meta: OnceLock>, // None means not found. - pub canonicalized: OnceLock>, - pub node_modules: OnceLock>>, + pub canonicalized: OnceLock<(Weak, PathBuf)>, + pub node_modules: OnceLock, PathBuf)>>, pub package_json: OnceLock>, /// `tsconfig.json` found at path. pub tsconfig: OnceLock>>, @@ -40,7 +40,7 @@ impl CachedPathImpl { path: Box, is_node_modules: bool, inside_node_modules: bool, - parent: Option>, + parent: Option<(Weak, PathBuf)>, ) -> Self { Self { hash, @@ -75,8 +75,14 @@ impl CachedPath { self.path.to_path_buf() } - pub(crate) fn parent(&self) -> Option { - self.0.parent.as_ref().and_then(|weak| weak.upgrade().map(CachedPath)) + pub(crate) fn parent(&self, cache: &Cache) -> Option { + 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.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(&self, ext: &str, cache: &Cache) -> Self { diff --git a/src/lib.rs b/src/lib.rs index cdc6003e..ae80348c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -309,7 +309,7 @@ impl ResolverGeneric { // 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 ResolverGeneric { 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 ResolverGeneric { 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 ResolverGeneric { // 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 ResolverGeneric { // 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 ResolverGeneric { // 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) diff --git a/src/tests/clear_cache.rs b/src/tests/clear_cache.rs new file mode 100644 index 00000000..ba3dda57 --- /dev/null +++ b/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 = (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); +} diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 258a506a..6644983b 100644 --- a/src/tests/mod.rs +++ b/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; diff --git a/src/tsconfig_resolver.rs b/src/tsconfig_resolver.rs index 5f41baf7..184e2be5 100644 --- a/src/tsconfig_resolver.rs +++ b/src/tsconfig_resolver.rs @@ -120,7 +120,7 @@ impl ResolverGeneric { })? { return Ok(Some(Arc::clone(tsconfig))); } - cache_value = cv.parent(); + cache_value = cv.parent(&self.cache); } Ok(None) }