Skip to content

Commit 5cd68f8

Browse files
committed
Fix race condition in local registry crate downloading.
Copy crate and keep exclusive lock to it with local registries. Ensures that only one instance will try to extract the source of a new package. Fixes #6588.
1 parent 56cbbec commit 5cd68f8

File tree

1 file changed

+25
-24
lines changed

1 file changed

+25
-24
lines changed

src/cargo/sources/registry/local.rs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ use hex;
1111

1212
pub struct LocalRegistry<'cfg> {
1313
index_path: Filesystem,
14+
cache_path: Filesystem,
1415
root: Filesystem,
15-
src_path: Filesystem,
1616
config: &'cfg Config,
1717
}
1818

1919
impl<'cfg> LocalRegistry<'cfg> {
2020
pub fn new(root: &Path, config: &'cfg Config, name: &str) -> LocalRegistry<'cfg> {
2121
LocalRegistry {
22-
src_path: config.registry_source_path().join(name),
22+
cache_path: config.registry_cache_path().join(name),
2323
index_path: Filesystem::new(root.join("index")),
2424
root: Filesystem::new(root.to_path_buf()),
2525
config,
@@ -70,38 +70,39 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> {
7070
}
7171

7272
fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult<MaybeLock> {
73-
let crate_file = format!("{}-{}.crate", pkg.name(), pkg.version());
74-
let mut crate_file = self.root.open_ro(&crate_file, self.config, "crate file")?;
75-
76-
// If we've already got an unpacked version of this crate, then skip the
77-
// checksum below as it is in theory already verified.
78-
let dst = format!("{}-{}", pkg.name(), pkg.version());
79-
if self.src_path.join(dst).into_path_unlocked().exists() {
80-
return Ok(MaybeLock::Ready(crate_file));
73+
let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
74+
75+
// Attempt to open an read-only lock first to avoid an exclusive write lock.
76+
//
77+
// If this fails then we fall through to the exclusive path where we copy
78+
// the file.
79+
if let Ok(dst) = self.cache_path.open_ro(&filename, self.config, &filename) {
80+
let meta = dst.file().metadata()?;
81+
if meta.len() > 0 {
82+
return Ok(MaybeLock::Ready(dst));
83+
}
8184
}
8285

8386
self.config.shell().status("Unpacking", pkg)?;
8487

85-
// We don't actually need to download anything per-se, we just need to
86-
// verify the checksum matches the .crate file itself.
88+
// Verify the checksum and copy over the .crate.
89+
let mut buf = Vec::new();
90+
let mut crate_file_source = self.root.open_ro(&filename, self.config, "crate file")?;
91+
let _ = crate_file_source
92+
.read_to_end(&mut buf)
93+
.chain_err(|| format!("failed to read `{}`", crate_file_source.path().display()))?;
94+
8795
let mut state = Sha256::new();
88-
let mut buf = [0; 64 * 1024];
89-
loop {
90-
let n = crate_file
91-
.read(&mut buf)
92-
.chain_err(|| format!("failed to read `{}`", crate_file.path().display()))?;
93-
if n == 0 {
94-
break;
95-
}
96-
state.update(&buf[..n]);
97-
}
96+
state.update(&buf);
9897
if hex::encode(state.finish()) != checksum {
9998
failure::bail!("failed to verify the checksum of `{}`", pkg)
10099
}
101100

102-
crate_file.seek(SeekFrom::Start(0))?;
101+
let mut dst = self.cache_path.open_rw(&filename, self.config, &filename)?;
102+
dst.write_all(&buf)?;
103+
dst.seek(SeekFrom::Start(0))?;
103104

104-
Ok(MaybeLock::Ready(crate_file))
105+
Ok(MaybeLock::Ready(dst))
105106
}
106107

107108
fn finish_download(

0 commit comments

Comments
 (0)