Skip to content

Commit 51f4ce5

Browse files
elrrrrrrrclaude
andcommitted
fix(pm): normalize path separators in CLONE_CACHE keys on Windows
OnceMap dedupes by *target path*, but install.rs and pipeline workers construct the same logical target with different separators on Windows: install.rs: `cwd.join("node_modules/foo")` → forward slashes (lockfile paths use `/`) pipeline: `cwd.join(PathBuf::from("..."))` → mixed, with `\` injected at `Path::join` boundaries These produce distinct strings (`a/b/c` vs `a/b\c`) and OnceMap sees them as two unrelated keys. Both clone tasks then race on the same real directory file-by-file, surfacing as `ERROR_SHARING_VIOLATION` (os error 32). The parent-wait via `wait_clone_if_pending` is also defeated for the same reason. Fix: switch the OnceMap key from `String` to `PathBuf`, and feed it through `Path::components().collect()`. `components()` parses both `/` and `\` as separators on Windows, and `collect::<PathBuf>()` rebuilds with the OS-preferred separator — giving a stable key without ad-hoc string replace and without an extra crate. Linux/macOS unchanged (separator is already `/`). Includes a Windows-only regression test asserting `cache_key("a/b") == cache_key("a\\b")`. Surfaced by the eggjs/egg Windows e2e smoke test, which triggered the race in `@langchain/core`'s nested node_modules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ac548a7 commit 51f4ce5

1 file changed

Lines changed: 39 additions & 4 deletions

File tree

crates/pm/src/util/cloner.rs

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,16 @@ use super::retry::create_retry_strategy;
1313
use crate::fs;
1414

1515
/// Global clone cache shared between pipeline and install phases.
16-
/// Key: target path, Value: ()
17-
static CLONE_CACHE: Lazy<OnceMap<String, ()>> = Lazy::new(OnceMap::new);
16+
///
17+
/// Key: normalized target path. Install (`cwd.join("node_modules/foo")` →
18+
/// forward slashes) and pipeline (`Path::join` injects backslashes on
19+
/// Windows) produce the same logical target with different separators;
20+
/// without normalization OnceMap sees them as distinct keys, dedup fails,
21+
/// and concurrent tasks race on the same destination — manifesting as
22+
/// `ERROR_SHARING_VIOLATION` (os error 32) on Windows. `PathBuf` from
23+
/// `Path::components().collect()` parses both separators uniformly and
24+
/// rebuilds with the OS-preferred one, giving a stable key.
25+
static CLONE_CACHE: Lazy<OnceMap<PathBuf, ()>> = Lazy::new(OnceMap::new);
1826

1927
/// Number of clones completed.
2028
static CLONE_COUNT: AtomicUsize = AtomicUsize::new(0);
@@ -24,12 +32,25 @@ pub fn clone_count() -> usize {
2432
CLONE_COUNT.load(Ordering::Relaxed)
2533
}
2634

35+
/// Normalize a target path into the canonical key used by `CLONE_CACHE`.
36+
#[cfg(windows)]
37+
fn cache_key(target_path: &Path) -> PathBuf {
38+
target_path.components().collect()
39+
}
40+
41+
#[cfg(not(windows))]
42+
fn cache_key(target_path: &Path) -> PathBuf {
43+
target_path.to_path_buf()
44+
}
45+
2746
/// Wait for a pending clone at the given target path to complete (if any).
2847
///
2948
/// Used by the pipeline clone worker to ensure parent packages are
3049
/// cloned before their children.
3150
pub async fn wait_clone_if_pending(target_path: &str) {
32-
CLONE_CACHE.wait_if_pending(&target_path.to_string()).await;
51+
CLONE_CACHE
52+
.wait_if_pending(&cache_key(Path::new(target_path)))
53+
.await;
3354
}
3455

3556
/// Clone a package to target path, downloading to cache first if needed.
@@ -42,7 +63,7 @@ pub async fn clone_package_once(
4263
tarball_url: &str,
4364
target_path: &Path,
4465
) -> Result<()> {
45-
let key = target_path.to_string_lossy().to_string();
66+
let key = cache_key(target_path);
4667
let err_label = format!("{name}@{version}");
4768
let name = name.to_string();
4869
let version = version.to_string();
@@ -440,6 +461,20 @@ mod tests {
440461

441462
use super::*;
442463

464+
#[cfg(windows)]
465+
#[test]
466+
fn cache_key_normalizes_path_separators() {
467+
// install.rs joins lockfile-derived strings (forward slashes) while
468+
// pipeline workers go through `Path::join` (backslashes). Both must
469+
// produce the same OnceMap key — otherwise concurrent clones race
470+
// and Windows raises ERROR_SHARING_VIOLATION.
471+
let forward = cache_key(Path::new("node_modules/@scope/pkg/node_modules/dep"));
472+
let backward = cache_key(Path::new("node_modules\\@scope\\pkg\\node_modules\\dep"));
473+
let mixed = cache_key(Path::new("node_modules/@scope/pkg\\node_modules\\dep"));
474+
assert_eq!(forward, backward);
475+
assert_eq!(forward, mixed);
476+
}
477+
443478
async fn create_test_file(dir: &Path, name: &str, content: &[u8]) -> Result<PathBuf> {
444479
let path = dir.join(name);
445480
let mut file = fs::File::create(&path).await?;

0 commit comments

Comments
 (0)