Deadlock when downloading git dependencies

[matklad]

Repro:

$ rm -rf ~/.cargo/registry
$ git clone [email protected]:tokio-rs/tokio.git
$ cd tokio
$ cargo metadata& 
$ cargo metadata& 

Updating registry `https://github.com/rust-lang/crates.io-index`  
Blocking waiting for file lock on the registry index 

Blocking waiting for file lock on the git checkout
Blocking waiting for file lock on the git checkout

[matklad]

Looks like the same issue as #2566.

[matklad]

Some logs: https://gist.github.com/matklad/ba1dd1d2238e2dcb4aa29e8d1fc785c4

[matklad]

So looks like a textbook deadlock: git sources keep locks while other git sources are being updated, and the locks are captured in no particular order (resolution graph is non deterministic because of HashMaps).

Looks like making locking order deterministic is not an option: how would one order git dependencies from different packages?

[alexcrichton]

Yeah this happens frequently with two git deps, and you're right that it's because git locks aren't ordered. Unfortunately it's actually not possible to order locks because we don't know the set of locks we need to take at the beginning, or at least not with the current strategy.

Some options we have here are:

• Keep track of all locks held, and also impose a global ordering. If we acquire a lock we need to drop all locks after it in the ordering, then grab all locks again.
• Hold locks temporarily during resolution and then grab them afterwards once we know the whole set.

I'd probably prefer the latter option (or another alternative) to solve this, dropping and re-acquiring locks sounds like a pain!

[matklad]

What about using a single lock for all git deps?

Hm, and what if the resolution DAG includes the same git dependency twice, but with different revisions? Is it true that it is impossible to build a package with such dependencies because both revisions should be checked out to the same directory? EDIT: apparently this is false, because they will be checked out to different directories.

[matklad]

Some more alternatives:

• just bail out with an error if we try to lock a repository and it is already locked. Sort of conservative deadlock detection.
• always checkout a precise revision, so that the lock is needed only during the checkout itself. That is, instead of using mutable branches, use append only commit hashes.

[alexcrichton]

Hm I think I like the single lock idea actually. It's pretty bad in terms of UI (makes the feature almost unusable), but it should be rarer than crates.io and it's better UI than deadlocking. So sounds good to me!

[matklad]

The question is who should own that single FileLock? The only "global" object readily available in the git source is Config. Stuffing a RefCell<Option<FileLock>> inside the Config would probably work, but perhaps there is a better way?

[alexcrichton]

@matklad yeah I'd expect Config to hold it as it's the "global state" of Cargo