fix: don't drop canonicalized path by cache clear (#791)

`Canonicalized path was dropped` error started to happen when I started
to call `resolver.cache_clear()` frequently (so that I can clear the
cache for each resolution to support
vitest-dev/vitest#8754 (comment)).

My guess of the cause is:

- `resolver.cache_clear()` is called while a resolution is happening
- `self.paths` is cleared because of that call
- this `Arc::downgrade` removes the last strong reference:
https://github.com/oxc-project/oxc-resolver/blob/9de30883232082e425ae0c837c69adf07e8570e6/src/cache/cache_impl.rs#L283
- this `weak.upgrade()` errors:
https://github.com/oxc-project/oxc-resolver/blob/9de30883232082e425ae0c837c69adf07e8570e6/src/cache/cache_impl.rs#L288

This PR solves that by using `Mutex<Weak<_>>>` instead of
`OnceLock<Weak<_>>`. Since `weak.upgrade()` may return None, I changed
the code to resolve when `weak` returned None. The previous code assumed
that `weak.upgrade()` always returns a value and that allowed us to use
`OnceLock`.

I guess this will have a negative impact on perf...

Author: sapphi-red

src/cache/cache_impl.rs

@@ -234,60 +234,46 @@ impl<Fs: FileSystem> Cache<Fs> {
             return Err(io::Error::new(io::ErrorKind::NotFound, "Circular symlink").into());
         }
 
-        path.canonicalized
-            .get_or_init(|| {
-                path.canonicalizing.store(tid, Ordering::Release);
-
-                let res = path.parent().map_or_else(
-                    || Ok(path.normalize_root(self)),
-                    |parent| {
-                        self.canonicalize_impl(&parent).and_then(|parent_canonical| {
-                            let normalized = parent_canonical.normalize_with(
-                                path.path().strip_prefix(parent.path()).unwrap(),
-                                self,
-                            );
+        let mut canonicalized_guard = path.canonicalized.lock().unwrap();
+        let canonicalized = canonicalized_guard.clone()?;
+        if let Some(cached_path) = canonicalized.upgrade() {
+            return Ok(CachedPath(cached_path));
+        }
 
-                            if self.fs.symlink_metadata(path.path()).is_ok_and(|m| m.is_symlink) {
-                                let link = self.fs.read_link(normalized.path())?;
-                                if link.is_absolute() {
-                                    return self.canonicalize_impl(&self.value(&link.normalize()));
-                                } else if let Some(dir) = normalized.parent() {
-                                    // Symlink is relative `../../foo.js`, use the path directory
-                                    // to resolve this symlink.
-                                    return self
-                                        .canonicalize_impl(&dir.normalize_with(&link, self));
-                                }
-                                debug_assert!(
-                                    false,
-                                    "Failed to get path parent for {}.",
-                                    normalized.path().display()
-                                );
-                            }
+        path.canonicalizing.store(tid, Ordering::Release);
 
-                            Ok(normalized)
-                        })
-                    },
-                );
+        let res = path.parent().map_or_else(
+            || Ok(path.normalize_root(self)),
+            |parent| {
+                self.canonicalize_impl(&parent).and_then(|parent_canonical| {
+                    let normalized = parent_canonical
+                        .normalize_with(path.path().strip_prefix(parent.path()).unwrap(), self);
 
-                path.canonicalizing.store(0, Ordering::Release);
-                // Store the canonicalized path in the cache before downgrading to weak reference
-                // This ensures there's always at least one strong reference to prevent dropping
-                if let Ok(ref cp) = res {
-                    // Only insert if not already present to avoid unnecessary operations
-                    let paths = self.paths.pin();
-                    if !paths.contains(cp) {
-                        paths.insert(cp.clone());
+                    if self.fs.symlink_metadata(path.path()).is_ok_and(|m| m.is_symlink) {
+                        let link = self.fs.read_link(normalized.path())?;
+                        if link.is_absolute() {
+                            return self.canonicalize_impl(&self.value(&link.normalize()));
+                        } else if let Some(dir) = normalized.parent() {
+                            // Symlink is relative `../../foo.js`, use the path directory
+                            // to resolve this symlink.
+                            return self.canonicalize_impl(&dir.normalize_with(&link, self));
+                        }
+                        debug_assert!(
+                            false,
+                            "Failed to get path parent for {}.",
+                            normalized.path().display()
+                        );
                     }
-                }
-                // Convert to Weak reference for storage
-                res.map(|cp| Arc::downgrade(&cp.0))
-            })
-            .as_ref()
-            .map_err(Clone::clone)
-            .and_then(|weak| {
-                weak.upgrade().map(CachedPath).ok_or_else(|| {
-                    ResolveError::from(io::Error::other("Canonicalized path was dropped"))
+
+                    Ok(normalized)
                 })
-            })
+            },
+        );
+
+        path.canonicalizing.store(0, Ordering::Release);
+        // Convert to Weak reference for storage
+        *canonicalized_guard = res.as_ref().map_err(Clone::clone).map(|cp| Arc::downgrade(&cp.0));
+
+        res
     }
 }

src/cache/cached_path.rs

@@ -4,7 +4,7 @@ use std::{
     hash::{Hash, Hasher},
     ops::Deref,
     path::{Component, Path, PathBuf},
-    sync::{Arc, Weak, atomic::AtomicU64},
+    sync::{Arc, Mutex, Weak, atomic::AtomicU64},
 };
 
 use cfg_if::cfg_if;
@@ -27,7 +27,7 @@ pub struct CachedPathImpl {
     pub is_node_modules: bool,
     pub inside_node_modules: bool,
     pub meta: OnceLock<Option<FileMetadata>>,
-    pub canonicalized: OnceLock<Result<Weak<CachedPathImpl>, ResolveError>>,
+    pub canonicalized: Mutex<Result<Weak<CachedPathImpl>, ResolveError>>,
     pub canonicalizing: AtomicU64,
     pub node_modules: OnceLock<Option<Weak<CachedPathImpl>>>,
     pub package_json: OnceLock<Option<Arc<PackageJson>>>,
@@ -49,7 +49,7 @@ impl CachedPathImpl {
             is_node_modules,
             inside_node_modules,
             meta: OnceLock::new(),
-            canonicalized: OnceLock::new(),
+            canonicalized: Mutex::new(Ok(Weak::new())),
             canonicalizing: AtomicU64::new(0),
             node_modules: OnceLock::new(),
             package_json: OnceLock::new(),