diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
index c2254aa976e..3ab8b79c638 100644
--- a/src/cargo/sources/git/source.rs
+++ b/src/cargo/sources/git/source.rs
@@ -6,7 +6,7 @@ use url::Url;
use core::source::{Source, SourceId};
use core::GitReference;
use core::{Package, PackageId, Summary, Registry, Dependency};
-use util::{CargoResult, Config, FileLock, to_hex};
+use util::{CargoResult, Config, to_hex};
use sources::PathSource;
use sources::git::utils::{GitRemote, GitRevision};
@@ -18,7 +18,6 @@ pub struct GitSource<'cfg> {
source_id: SourceId,
path_source: Option<PathSource<'cfg>>,
rev: Option<GitRevision>,
- checkout_lock: Option<FileLock>,
ident: String,
config: &'cfg Config,
}
@@ -42,7 +41,6 @@ impl<'cfg> GitSource<'cfg> {
source_id: source_id.clone(),
path_source: None,
rev: None,
- checkout_lock: None,
ident: ident,
config: config,
}
@@ -128,14 +126,9 @@ impl<'cfg> Registry for GitSource<'cfg> {
impl<'cfg> Source for GitSource<'cfg> {
fn update(&mut self) -> CargoResult<()> {
- // First, lock both the global database and checkout locations that
- // we're going to use. We may be performing a fetch into these locations
- // so we need writable access.
- let db_lock = format!(".cargo-lock-{}", self.ident);
- let db_lock = try!(self.config.git_db_path()
- .open_rw(&db_lock, self.config,
- "the git database"));
- let db_path = db_lock.parent().join(&self.ident);
+ let lock = try!(self.config.lock_git());
+
+ let db_path = lock.parent().join("db").join(&self.ident);
let reference_path = match self.source_id.git_reference() {
Some(&GitReference::Branch(ref s)) |
@@ -143,13 +136,8 @@ impl<'cfg> Source for GitSource<'cfg> {
Some(&GitReference::Rev(ref s)) => s,
None => panic!("not a git source"),
};
- let checkout_lock = format!(".cargo-lock-{}-{}", self.ident,
- reference_path);
- let checkout_lock = try!(self.config.git_checkout_path()
- .join(&self.ident)
- .open_rw(&checkout_lock, self.config,
- "the git checkout"));
- let checkout_path = checkout_lock.parent().join(reference_path);
+ let checkout_path = lock.parent().join("checkouts")
+ .join(&self.ident).join(reference_path);
// Resolve our reference to an actual revision, and check if the
// databaes already has that revision. If it does, we just load a
@@ -187,7 +175,6 @@ impl<'cfg> Source for GitSource<'cfg> {
// swipe our checkout location to another revision while we're using it!
self.path_source = Some(path_source);
self.rev = Some(actual_rev);
- self.checkout_lock = Some(checkout_lock);
self.path_source.as_mut().unwrap().update()
}
diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs
index df319125d83..329538663ae 100644
--- a/src/cargo/util/config.rs
+++ b/src/cargo/util/config.rs
@@ -15,7 +15,7 @@ use toml;
use core::shell::{Verbosity, ColorConfig};
use core::MultiShell;
use util::{CargoResult, CargoError, ChainError, Rustc, internal, human};
-use util::{Filesystem, LazyCell};
+use util::{Filesystem, FileLock, LazyCell};
use util::toml as cargo_toml;
@@ -31,6 +31,7 @@ pub struct Config {
extra_verbose: Cell<bool>,
frozen: Cell<bool>,
locked: Cell<bool>,
+ git_lock: LazyCell<FileLock>,
}
impl Config {
@@ -47,6 +48,7 @@ impl Config {
extra_verbose: Cell::new(false),
frozen: Cell::new(false),
locked: Cell::new(false),
+ git_lock: LazyCell::new(),
}
}
@@ -64,12 +66,23 @@ impl Config {
pub fn home(&self) -> &Filesystem { &self.home_path }
- pub fn git_db_path(&self) -> Filesystem {
- self.home_path.join("git").join("db")
- }
-
- pub fn git_checkout_path(&self) -> Filesystem {
- self.home_path.join("git").join("checkouts")
+ pub fn git_path(&self) -> Filesystem {
+ self.home_path.join("git")
+ }
+
+ /// All git sources are protected by a single file lock, which is stored
+ /// in the `Config` struct. Ideally, we should use a lock per repository,
+ /// but this leads to deadlocks when several instances of Cargo acquire
+ /// locks in different order (see #2987).
+ ///
+ /// Holding the lock only during checkout is not enough. For example, two
+ /// instances of Cargo may checkout different commits from the same branch
+ /// to the same directory. So the lock is acquired when the first git source
+ /// is updated and released when the `Config` struct is destroyed.
+ pub fn lock_git(&self) -> CargoResult<&FileLock> {
+ self.git_lock.get_or_try_init(|| {
+ self.git_path().open_rw(".cargo-lock-git", self, "the git checkouts")
+ })
}
pub fn registry_index_path(&self) -> Filesystem {
diff --git a/tests/concurrent.rs b/tests/concurrent.rs
index 0243e782f83..a2b827f565d 100644
--- a/tests/concurrent.rs
+++ b/tests/concurrent.rs
@@ -9,6 +9,8 @@ use std::io::Write;
use std::net::TcpListener;
use std::process::Stdio;
use std::thread;
+use std::sync::mpsc::channel;
+use std::time::Duration;
use cargotest::install::{has_installed_exe, cargo_home};
use cargotest::support::git;
@@ -428,3 +430,70 @@ fn debug_release_ok() {
[FINISHED] release [optimized] target(s) in [..]
"));
}
+
+#[test]
+fn no_deadlock_with_git_dependencies() {
+ let dep1 = git::new("dep1", |project| {
+ project.file("Cargo.toml", r#"
+ [project]
+ name = "dep1"
+ version = "0.5.0"
+ authors = []
+ "#).file("src/lib.rs", "")
+ }).unwrap();
+
+ let dep2 = git::new("dep2", |project| {
+ project.file("Cargo.toml", r#"
+ [project]
+ name = "dep2"
+ version = "0.5.0"
+ authors = []
+ "#).file("src/lib.rs", "")
+ }).unwrap();
+
+ let p = project("foo")
+ .file("Cargo.toml", &format!(r#"
+ [package]
+ name = "foo"
+ authors = []
+ version = "0.0.0"
+
+ [dependencies]
+ dep1 = {{ git = '{}' }}
+ dep2 = {{ git = '{}' }}
+ "#, dep1.url(), dep2.url()))
+ .file("src/main.rs", "fn main() { }");
+ p.build();
+
+ let n_concurrent_builds = 5;
+
+ let (tx, rx) = channel();
+ for _ in 0..n_concurrent_builds {
+ let cmd = p.cargo("build").build_command()
+ .stdout(Stdio::piped())
+ .stderr(Stdio::piped())
+ .spawn();
+ let tx = tx.clone();
+ thread::spawn(move || {
+ let result = cmd.unwrap().wait_with_output().unwrap();
+ tx.send(result).unwrap()
+ });
+ }
+
+ //TODO: use `Receiver::recv_timeout` once it is stable.
+ let recv_timeout = |chan: &::std::sync::mpsc::Receiver<_>| {
+ for _ in 0..3000 {
+ if let Ok(x) = chan.try_recv() {
+ return x
+ }
+ thread::sleep(Duration::from_millis(10));
+ }
+ chan.try_recv().expect("Deadlock!")
+ };
+
+ for _ in 0..n_concurrent_builds {
+ let result = recv_timeout(&rx);
+ assert_that(result, execs().with_status(0))
+ }
+
+}
\ No newline at end of file