Add process lock to avoid parallel cross issues.

Docker can have deadlocks or similar issues when invoked multiple times
in rapid succession, so this implements a small and correct process lock
with an identical API to `named_lock`. This allows us to prevent
invoking Docker multiple times.

The implementation uses `flock` on Linux/UNIX-like systems, which on
Linux is guaranteed to be cleaned up when the process exits. On Windows, it uses `CreateMutexW`, which also guarantees to be cleaned up when the process exits.

We use this rather than `fd-lock` because `fd-lock` has more dependencies, and it also uses `LockFile` on Windows which is not guaranteed to be cleaned up in a timely manner. The API is also designed for file locks, not process locks, which is only a good abstraction on UNIX-like systems.

We use this rather than `named_lock` since it has numerous, unnecessary dependencies. This allows us to have no external dependencies besides libc and the Windows API, for an extremely lightweight locking library.

.changes/963.json

@@ -0,0 +1,5 @@
+{
+    "description": "add lock to prevent deadlock with docker when invoked multiple times.",
+    "type": "fixed",
+    "issues": [496]
+}

Cargo.toml

@@ -23,7 +23,7 @@ default = []
 dev = []
 
 [workspace]
-members = ["xtask"]
+members = ["small-lock", "xtask"]
 
 [dependencies]
 atty = "0.2"
@@ -47,6 +47,7 @@ walkdir = { version = "2", optional = true }
 tempfile = "3.3.0"
 owo-colors = { version = "3.4.0", features = ["supports-colors"] }
 semver = "1"
+small-lock = { version = "0.0.1", path = "small-lock" }
 
 [target.'cfg(not(windows))'.dependencies]
 nix = { version = "0.24", default-features = false, features = ["user"] }

small-lock/Cargo.toml

@@ -0,0 +1,23 @@
+[package]
+documentation = "https://github.com/cross-rs/cross"
+license = "MIT OR Apache-2.0"
+name = "small-lock"
+repository = "https://github.com/cross-rs/cross"
+version = "0.0.1"
+edition = "2021"
+
+[target.'cfg(not(windows))'.dependencies]
+libc = "0.2"
+
+[target.'cfg(windows)'.dependencies.winapi]
+version = "0.3"
+features = [
+    "errhandlingapi",
+    "handleapi",
+    "synchapi",
+    "winbase",
+    "winerror",
+]
+
+[dev-dependencies]
+threadpool = "1.8.1"

small-lock/src/lib.rs

@@ -0,0 +1,154 @@
+//! A minimal-dependency, safe API for a named process lock.
+//!
+//! This uses `flock` on Unix-like systems and `CreateMutex` on Linux.
+//! This is meant to be a drop-in-replacement for `named_lock`, and
+//! therefore features an identical API.
+
+#![deny(missing_debug_implementations, rust_2018_idioms)]
+#![warn(
+    clippy::explicit_into_iter_loop,
+    clippy::explicit_iter_loop,
+    clippy::implicit_clone,
+    clippy::inefficient_to_string,
+    clippy::map_err_ignore,
+    clippy::map_unwrap_or,
+    clippy::ref_binding_to_reference,
+    clippy::semicolon_if_nothing_returned,
+    clippy::str_to_string,
+    clippy::string_to_string,
+    // needs clippy 1.61 clippy::unwrap_used
+)]
+
+#[cfg(target_family = "unix")]
+mod unix;
+#[cfg(target_family = "unix")]
+pub use unix::*;
+
+#[cfg(target_family = "windows")]
+mod windows;
+#[cfg(target_family = "windows")]
+pub use windows::*;
+
+use std::error;
+use std::fmt;
+use std::io;
+use std::result;
+
+#[cfg(target_family = "unix")]
+use std::path::Path;
+
+/// Named (or by path) lock.
+#[derive(Debug)]
+pub struct NamedLock {
+    inner: InnerLock,
+}
+
+impl NamedLock {
+    /// Create a lock file with a given name.
+    ///
+    /// By default, this will the current temporary directory
+    /// and create a file at `$TMP/{name}.lock`.
+    pub fn create(name: impl AsRef<str>) -> Result<NamedLock> {
+        Ok(NamedLock {
+            inner: InnerLock::create(name)?,
+        })
+    }
+
+    /// Creates a lock file at a specific path.
+    /// The parent directory must exist.
+    #[cfg(target_family = "unix")]
+    pub fn with_path(path: impl AsRef<Path>) -> Result<NamedLock> {
+        Ok(NamedLock {
+            inner: InnerLock::with_path(path)?,
+        })
+    }
+
+    /// Lock the named lock.
+    pub fn lock(&self) -> Result<NamedLockGuard<'_>> {
+        Ok(NamedLockGuard {
+            _inner: self.inner.lock()?,
+        })
+    }
+
+    /// Try to lock the named lock.
+    ///
+    /// Returns `Error::WouldBlock` if it cannot acquire the lock
+    /// because it would block.
+    pub fn try_lock(&self) -> Result<NamedLockGuard<'_>> {
+        Ok(NamedLockGuard {
+            _inner: self.inner.try_lock()?,
+        })
+    }
+
+    /// Unlock the named lock.
+    pub fn unlock(&self) -> Result<()> {
+        self.inner.unlock()
+    }
+}
+
+/// Scoped guard for the named lock.
+#[derive(Debug)]
+pub struct NamedLockGuard<'a> {
+    _inner: InnerLockGuard<'a>,
+}
+
+/// Error type for the lockfile.
+#[derive(Debug)]
+pub enum Error {
+    InvalidCharacter,
+    CreateFailed(io::Error),
+    LockFailed(io::Error),
+    UnlockFailed(io::Error),
+    WouldBlock,
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Error::InvalidCharacter => f.write_str("invalid character found in file name"),
+            Error::CreateFailed(e) => f.write_fmt(format_args!("lock creation failed with {e}")),
+            Error::LockFailed(e) => f.write_fmt(format_args!("unable to acquire lock with {e}")),
+            Error::UnlockFailed(e) => f.write_fmt(format_args!("unable to release lock with {e}")),
+            Error::WouldBlock => f.write_str("acquiring lock would block"),
+        }
+    }
+}
+
+impl error::Error for Error {
+    fn source(&self) -> Option<&(dyn error::Error + 'static)> {
+        match self {
+            Error::CreateFailed(err) => Some(err),
+            _ => None,
+        }
+    }
+}
+
+/// Lock type for creating results.
+pub type Result<T> = result::Result<T, Error>;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::{thread, time};
+
+    #[test]
+    fn test_lock() {
+        let workers = 4;
+        let jobs = 8;
+        let pool = threadpool::ThreadPool::new(workers);
+        let start = time::SystemTime::now();
+        for _ in 0..jobs {
+            pool.execute(move || {
+                let lock = NamedLock::create("cross-rs-test-lock").unwrap();
+                let _guard = lock.lock().unwrap();
+                thread::sleep(time::Duration::from_millis(500));
+            });
+        }
+        pool.join();
+
+        // should be ~4s elapsed, due to the locks
+        let end = time::SystemTime::now();
+        let diff = end.duration_since(start).unwrap();
+        assert!((4000..=6000).contains(&diff.as_millis()));
+    }
+}

small-lock/src/unix.rs

@@ -0,0 +1,89 @@
+#![cfg(target_family = "unix")]
+#![doc(hidden)]
+
+use std::env;
+use std::fs::File;
+use std::io;
+use std::os::unix::io::AsRawFd;
+use std::path::Path;
+
+use crate::{Error, Result};
+
+#[derive(Debug)]
+pub struct InnerLock {
+    file: File,
+}
+
+impl InnerLock {
+    pub fn create(name: impl AsRef<str>) -> Result<InnerLock> {
+        match is_valid_filename(name.as_ref()) {
+            true => {
+                let mut file_name = name.as_ref().to_owned();
+                file_name.push_str(".lock");
+                InnerLock::with_path(env::temp_dir().join(file_name))
+            }
+            false => Err(Error::InvalidCharacter),
+        }
+    }
+
+    pub fn with_path(path: impl AsRef<Path>) -> Result<InnerLock> {
+        Ok(InnerLock {
+            file: File::create(path).map_err(Error::CreateFailed)?,
+        })
+    }
+
+    pub fn lock(&self) -> Result<InnerLockGuard<'_>> {
+        // SAFETY: safe, since we have valid flags.
+        unsafe { self._lock(libc::LOCK_EX) }
+    }
+
+    pub fn try_lock(&self) -> Result<InnerLockGuard<'_>> {
+        // SAFETY: safe, since we have valid flags.
+        unsafe { self._lock(libc::LOCK_EX | libc::LOCK_NB) }
+    }
+
+    /// # SAFETY
+    ///
+    /// Safe as long as the flags are correct.
+    unsafe fn _lock(&self, flags: libc::c_int) -> Result<InnerLockGuard<'_>> {
+        // SAFETY: safe if we have a valid file descriptor.
+        let fd = self.file.as_raw_fd();
+        match libc::flock(fd, flags) {
+            0 => Ok(InnerLockGuard { lock: self }),
+            libc::EWOULDBLOCK => Err(Error::WouldBlock),
+            code => Err(Error::LockFailed(io::Error::from_raw_os_error(code))),
+        }
+    }
+
+    pub fn unlock(&self) -> Result<()> {
+        // SAFETY: safe, since we use valid flags
+        unsafe { self._unlock(libc::LOCK_UN) }
+    }
+
+    /// # SAFETY
+    ///
+    /// Safe as long as the flags are correct.
+    unsafe fn _unlock(&self, flags: libc::c_int) -> Result<()> {
+        let fd = self.file.as_raw_fd();
+        match libc::flock(fd, flags) {
+            0 => Ok(()),
+            code => Err(Error::UnlockFailed(io::Error::from_raw_os_error(code))),
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct InnerLockGuard<'a> {
+    lock: &'a InnerLock,
+}
+
+impl<'a> Drop for InnerLockGuard<'a> {
+    fn drop(&mut self) {
+        self.lock.unlock().ok();
+    }
+}
+
+fn is_valid_filename(name: &str) -> bool {
+    // the name cannot have a /
+    !name.contains('/')
+}