Skip to content

Commit

Permalink
Fix a flaky concurrent test with correct locking
Browse files Browse the repository at this point in the history
The recent refactoring to clone the bare registry left in an accidental path
where index checkouts could clobber one another. This commit updates the logic
with proper locking and attempt ordering, attempting a full retry of the open
operation after the index locked.
  • Loading branch information
alexcrichton committed May 15, 2017
1 parent 62acd6e commit 7b44acc
Showing 1 changed file with 21 additions and 12 deletions.
33 changes: 21 additions & 12 deletions src/cargo/sources/registry/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,18 @@ impl<'cfg> RemoteRegistry<'cfg> {
self.repo.get_or_try_init(|| {
let path = self.index_path.clone().into_path_unlocked();

// Fast path without a lock
if let Ok(repo) = git2::Repository::open(&path) {
return Ok(repo)
}

// Ok, now we need to lock and try the whole thing over again.
let lock = self.index_path.open_rw(Path::new(INDEX_LOCK),
self.config,
"the registry index")?;
match git2::Repository::open(&path) {
Ok(repo) => Ok(repo),
Err(_) => {
self.index_path.create_dir()?;
let lock = self.index_path.open_rw(Path::new(INDEX_LOCK),
self.config,
"the registry index")?;
let _ = lock.remove_siblings();
Ok(git2::Repository::init_bare(&path)?)
}
Expand Down Expand Up @@ -153,25 +158,29 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
"the registry index")?;
self.config.shell().status("Updating",
format!("registry `{}`", self.source_id.url()))?;
let mut needs_fetch = true;

if self.source_id.url().host_str() == Some("github.com") {
if let Ok(oid) = self.head() {
let mut handle = self.easy()?.borrow_mut();
debug!("attempting github fast path for {}",
self.source_id.url());
if github_up_to_date(&mut handle, self.source_id.url(), &oid) {
return Ok(())
needs_fetch = false;
} else {
debug!("fast path failed, falling back to a git fetch");
}
debug!("fast path failed, falling back to a git fetch");
}
}

// git fetch origin master
let url = self.source_id.url().to_string();
let refspec = "refs/heads/master:refs/remotes/origin/master";
git::fetch(&repo, &url, refspec, self.config).chain_error(|| {
human(format!("failed to fetch `{}`", url))
})?;
if needs_fetch {
// git fetch origin master
let url = self.source_id.url().to_string();
let refspec = "refs/heads/master:refs/remotes/origin/master";
git::fetch(&repo, &url, refspec, self.config).chain_error(|| {
human(format!("failed to fetch `{}`", url))
})?;
}
self.head.set(None);
*self.tree.borrow_mut() = None;
Ok(())
Expand Down

0 comments on commit 7b44acc

Please sign in to comment.