Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies to support illumos target #8093

Merged
merged 1 commit into from
Apr 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ curl-sys = "0.4.22"
env_logger = "0.7.0"
pretty_env_logger = { version = "0.4", optional = true }
anyhow = "1.0"
filetime = "0.2"
filetime = "0.2.9"
flate2 = { version = "1.0.3", default-features = false, features = ["zlib"] }
fs2 = "0.4"
git2 = "0.13.0"
git2 = "0.13.1"
git2-curl = "0.14.0"
glob = "0.3.0"
hex = "0.4"
Expand All @@ -45,7 +44,7 @@ jobserver = "0.1.21"
lazycell = "1.2.0"
libc = "0.2"
log = "0.4.6"
libgit2-sys = "0.12.0"
libgit2-sys = "0.12.1"
memchr = "2.1.3"
num_cpus = "1.0"
opener = "0.4"
Expand Down
154 changes: 131 additions & 23 deletions src/cargo/util/flock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ use std::io;
use std::io::{Read, Seek, SeekFrom, Write};
use std::path::{Display, Path, PathBuf};

use fs2::{lock_contended_error, FileExt};
use termcolor::Color::Cyan;
#[cfg(windows)]
use winapi::shared::winerror::ERROR_INVALID_FUNCTION;

use crate::util::errors::{CargoResult, CargoResultExt};
use crate::util::paths;
use crate::util::Config;
use sys::*;

#[derive(Debug)]
pub struct FileLock {
Expand Down Expand Up @@ -95,7 +93,7 @@ impl Drop for FileLock {
fn drop(&mut self) {
if self.state != State::Unlocked {
if let Some(f) = self.f.take() {
let _ = f.unlock();
let _ = unlock(&f);
}
}
}
Expand Down Expand Up @@ -231,13 +229,13 @@ impl Filesystem {
.chain_err(|| format!("failed to open: {}", path.display()))?;
match state {
State::Exclusive => {
acquire(config, msg, &path, &|| f.try_lock_exclusive(), &|| {
f.lock_exclusive()
acquire(config, msg, &path, &|| try_lock_exclusive(&f), &|| {
lock_exclusive(&f)
})?;
}
State::Shared => {
acquire(config, msg, &path, &|| f.try_lock_shared(), &|| {
f.lock_shared()
acquire(config, msg, &path, &|| try_lock_shared(&f), &|| {
lock_shared(&f)
})?;
}
State::Unlocked => {}
Expand Down Expand Up @@ -281,8 +279,8 @@ fn acquire(
config: &Config,
msg: &str,
path: &Path,
r#try: &dyn Fn() -> io::Result<()>,
block: &dyn Fn() -> io::Result<()>,
lock_try: &dyn Fn() -> io::Result<()>,
lock_block: &dyn Fn() -> io::Result<()>,
) -> CargoResult<()> {
// File locking on Unix is currently implemented via `flock`, which is known
// to be broken on NFS. We could in theory just ignore errors that happen on
Expand All @@ -298,24 +296,16 @@ fn acquire(
return Ok(());
}

match r#try() {
match lock_try() {
Ok(()) => return Ok(()),

// In addition to ignoring NFS which is commonly not working we also
// just ignore locking on filesystems that look like they don't
// implement file locking. We detect that here via the return value of
// locking (e.g., inspecting errno).
#[cfg(unix)]
Err(ref e) if e.raw_os_error() == Some(libc::ENOTSUP) => return Ok(()),

#[cfg(target_os = "linux")]
Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => return Ok(()),

#[cfg(windows)]
Err(ref e) if e.raw_os_error() == Some(ERROR_INVALID_FUNCTION as i32) => return Ok(()),
// implement file locking.
Err(e) if error_unsupported(&e) => return Ok(()),

Err(e) => {
if e.raw_os_error() != lock_contended_error().raw_os_error() {
if !error_contended(&e) {
let e = anyhow::Error::from(e);
let cx = format!("failed to lock file: {}", path.display());
return Err(e.context(cx).into());
Expand All @@ -325,7 +315,7 @@ fn acquire(
let msg = format!("waiting for file lock on {}", msg);
config.shell().status_with_color("Blocking", &msg, Cyan)?;

block().chain_err(|| format!("failed to lock file: {}", path.display()))?;
lock_block().chain_err(|| format!("failed to lock file: {}", path.display()))?;
return Ok(());

#[cfg(all(target_os = "linux", not(target_env = "musl")))]
Expand All @@ -352,3 +342,121 @@ fn acquire(
false
}
}

#[cfg(unix)]
mod sys {
use std::fs::File;
use std::io::{Error, Result};
use std::os::unix::io::AsRawFd;

pub(super) fn lock_shared(file: &File) -> Result<()> {
flock(file, libc::LOCK_SH)
}

pub(super) fn lock_exclusive(file: &File) -> Result<()> {
flock(file, libc::LOCK_EX)
}

pub(super) fn try_lock_shared(file: &File) -> Result<()> {
flock(file, libc::LOCK_SH | libc::LOCK_NB)
}

pub(super) fn try_lock_exclusive(file: &File) -> Result<()> {
flock(file, libc::LOCK_EX | libc::LOCK_NB)
}

pub(super) fn unlock(file: &File) -> Result<()> {
flock(file, libc::LOCK_UN)
}

pub(super) fn error_contended(err: &Error) -> bool {
err.raw_os_error().map_or(false, |x| x == libc::EWOULDBLOCK)
}

pub(super) fn error_unsupported(err: &Error) -> bool {
match err.raw_os_error() {
Some(libc::ENOTSUP) => true,
#[cfg(target_os = "linux")]
Some(libc::ENOSYS) => true,
_ => false,
}
}

#[cfg(not(target_os = "solaris"))]
fn flock(file: &File, flag: libc::c_int) -> Result<()> {
let ret = unsafe { libc::flock(file.as_raw_fd(), flag) };
if ret < 0 {
Err(Error::last_os_error())
} else {
Ok(())
}
}

#[cfg(target_os = "solaris")]
fn flock(file: &File, flag: libc::c_int) -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a few thoughts about this implementation. First off it seems like it's different enough from flock that it may want its own little submodule too?

Additionally I was reviewing the differences between fcntl vs flock-based locks. I don't think fcntl is what we want in terms of synchronization in that it releases the lock if any file description in the process closes the file or it also doesn't protect against threads. I don't think that Cargo runs afoul of this today but I don't think we can really guarantee that we won't ever run afoul of this ever.

If solaris doesn't implement the flock-style file locking, could solaris perhaps just eschew locking entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the fcntl-style locking is an ugly hack. In illumos, we ended up adding a "real" flock implementation to the OS. Considering the existing bail-outs for NFS and other filesystems which don't tolerate locking, it's probably OK to use no-op locking for solaris targets. I suspect that illumos systems awaiting their own target_os are probably the biggest pool of systems using the solaris target today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah in general file locking is largely best-effort within Cargo and it's only relevant for concurrent Cargo invocations which are somewhat rare. Internally Cargo doesn't rely on file locks at all, so having it be a noop should keep everything largely working.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds great. I'm happy to see that stuff go.

// Solaris lacks flock(), so simply succeed with a no-op
Ok(())
}
}

#[cfg(windows)]
mod sys {
use std::fs::File;
use std::io::{Error, Result};
use std::mem;
use std::os::windows::io::AsRawHandle;

use winapi::shared::minwindef::DWORD;
use winapi::shared::winerror::{ERROR_INVALID_FUNCTION, ERROR_LOCK_VIOLATION};
use winapi::um::fileapi::{LockFileEx, UnlockFile};
use winapi::um::minwinbase::{LOCKFILE_EXCLUSIVE_LOCK, LOCKFILE_FAIL_IMMEDIATELY};

pub(super) fn lock_shared(file: &File) -> Result<()> {
lock_file(file, 0)
}

pub(super) fn lock_exclusive(file: &File) -> Result<()> {
lock_file(file, LOCKFILE_EXCLUSIVE_LOCK)
}

pub(super) fn try_lock_shared(file: &File) -> Result<()> {
lock_file(file, LOCKFILE_FAIL_IMMEDIATELY)
}

pub(super) fn try_lock_exclusive(file: &File) -> Result<()> {
lock_file(file, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY)
}

pub(super) fn error_contended(err: &Error) -> bool {
err.raw_os_error()
.map_or(false, |x| x == ERROR_LOCK_VIOLATION as i32)
}

pub(super) fn error_unsupported(err: &Error) -> bool {
err.raw_os_error()
.map_or(false, |x| x == ERROR_INVALID_FUNCTION as i32)
}

pub(super) fn unlock(file: &File) -> Result<()> {
unsafe {
let ret = UnlockFile(file.as_raw_handle(), 0, 0, !0, !0);
if ret == 0 {
Err(Error::last_os_error())
} else {
Ok(())
}
}
}

fn lock_file(file: &File, flags: DWORD) -> Result<()> {
unsafe {
let mut overlapped = mem::zeroed();
let ret = LockFileEx(file.as_raw_handle(), flags, 0, !0, !0, &mut overlapped);
if ret == 0 {
Err(Error::last_os_error())
} else {
Ok(())
}
}
}
}