Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upFix running Cargo concurrently #2486
Conversation
rust-highfive
assigned
wycats
Mar 15, 2016
This comment has been minimized.
This comment has been minimized.
rust-highfive
commented
Mar 15, 2016
|
r? @wycats (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
|
r? @brson |
rust-highfive
assigned
brson
and unassigned
wycats
Mar 15, 2016
This comment has been minimized.
This comment has been minimized.
brson
reviewed
Mar 16, 2016
| }) | ||
| } | ||
|
|
||
| fn create_dir_all(path: &Path) -> io::Result<()> { |
This comment has been minimized.
This comment has been minimized.
brson
Mar 16, 2016
Contributor
Why does this module have it's own function for this instead of using std?
This comment has been minimized.
This comment has been minimized.
alexcrichton
Mar 16, 2016
Author
Member
The version in the standard library doesn't currently handle the NotFound error as it's not intended to be run concurrently (which I have personally advocated for). This is basically the concurrent variant of those functions.
Maybe we should consider adding it to libstd at some point...
This comment has been minimized.
This comment has been minimized.
|
Hard to evaluate how correct it is but it looks awesome. r=me |
alexcrichton
force-pushed the
alexcrichton:flock
branch
3 times, most recently
from
97c6a31
to
e3ffa0c
Mar 16, 2016
This comment has been minimized.
This comment has been minimized.
|
@bors: r=brson |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton
force-pushed the
alexcrichton:flock
branch
from
e3ffa0c
to
e22b08c
Mar 17, 2016
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Mar 17, 2016
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton
force-pushed the
alexcrichton:flock
branch
from
e22b08c
to
8eac1d6
Mar 17, 2016
This comment has been minimized.
This comment has been minimized.
|
@bors: r=brson |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Mar 17, 2016
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton commentedMar 15, 2016
Cargo has historically had no protections against running it concurrently. This
is pretty unfortunate, however, as it essentially just means that you can only
run one instance of Cargo at a time globally on a system.
An "easy solution" to this would be the use of file locks, except they need to
be applied judiciously. It'd be a pretty bad experience to just lock the entire
system globally for Cargo (although it would work), but otherwise Cargo must be
principled how it accesses the filesystem to ensure that locks are properly
held. This commit intends to solve all of these problems.
A new utility module is added to cargo,
util::flock, which contains two types:FileLock- a locked version of aFile. This RAII guard will unlock thelock on
Dropand I/O can be performed through this object. The actualunderlying
Pathcan be read from this object as well.Filesystem- an unlocked representation of aPath. There is no "safe"method to access the underlying path without locking a file on the filesystem
first.
Built on the fs2 library, these locks use the
flocksystem call on Unix andLockFileExon Windows. Although file locking on Unix is documented as not sogreat, but largely only because of NFS, these are just advisory, and
there's no byte-range locking. These issues don't necessarily plague Cargo,
however, so we should try to leverage them. On both Windows and Unix the file
locks are released when the underlying OS handle is closed, which means that
if the process dies the locks are released.
Cargo has a number of global resources which it now needs to lock, and the
strategy is done in a fairly straightforward way:
index requires a read/write lock while reading the index requires a shared
lock. This should allow each process to ensure a registry update happens while
not blocking out others for an unnecessarily long time. Additionally any
number of processes can read the index.
for the downloaded crate implies a lock on the output directory as well.
Because downloaded crates are immutable, once the downloaded directory exists
the lock is no longer needed as it won't be modified, so it can be released.
This granularity of locking allows multiple Cargo instances to download
dependencies in parallel.
checkout. The datbase and checkout are locked for read/write access when an
update is performed, and the lock of the checkout is held for the entire
lifetime of the git source. This is done to ensure that any other Cargo
processes must wait while we use the git repository. Unfortunately there's
just not that much parallelism here.
cargo installare locked by the local metadata file thatCargo manages. This is relatively straightforward.
build. It's hypothesized that running Cargo concurrently in one directory is
less of a feature needed rather than running multiple instances of Cargo
globally (for now at least). It would be possible to have finer grained
locking here, but that can likely be deferred to a future PR.
So with all of this infrastructure in place, Cargo is now ready to grab some
locks and ensure that you can call it concurrently anywhere at any time and
everything always works out as one might expect.
One interesting question, however, is what does Cargo do on contention? On one
hand Cargo could immediately abort, but this would lead to a pretty poor UI as
any Cargo process on the system could kick out any other. Instead this PR takes
a more nuanced approach.
succeeds, we're done.
for a lock. This is done because it's indeterminate how long Cargo will wait
for the lock to become available, and most long-lasting operations in Cargo
have a message printed for them.
become available.
So all in all this should help Cargo fix any future concurrency bugs with file
locking in a principled fashion while also allowing concurrent Cargo processes
to proceed reasonably across the system.
Closes #354