diff --git a/Cargo.lock b/Cargo.lock index 936c4c3b..3700093d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -642,6 +642,7 @@ dependencies = [ "glob-match", "hex", "json5", + "libc", "mimalloc", "rayon", "regex", @@ -655,6 +656,7 @@ dependencies = [ "tracing", "tracing-subscriber", "ureq", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index d725b2e9..a3763f00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ workspace = true [features] default = ["cli"] -cli = ["dep:gix", "dep:gix-traverse", "dep:gix-diff", "dep:ureq", "dep:clap", "dep:clap_complete", "dep:colored", "dep:mimalloc", "dep:rayon", "dep:tracing-subscriber", "dep:serde_norway"] +cli = ["dep:gix", "dep:gix-traverse", "dep:gix-diff", "dep:ureq", "dep:clap", "dep:clap_complete", "dep:colored", "dep:mimalloc", "dep:rayon", "dep:tracing-subscriber", "dep:serde_norway", "dep:libc", "dep:windows-sys"] [dependencies] serde = { version = "1", features = ["derive"] } @@ -79,6 +79,12 @@ tracing-subscriber = { version = "0.3", default-features = false, features = ["s # the user's config directory is a symlink TOCTOU). Tests also use it. tempfile = "3" +[target.'cfg(unix)'.dependencies] +libc = { version = "0.2", optional = true } + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", optional = true, features = ["Win32_Foundation", "Win32_Storage_FileSystem", "Win32_System_Threading"] } + [dev-dependencies] cargo-husky = { version = "1", default-features = false, features = ["user-hooks"] } criterion = { version = "0.8", features = ["html_reports"] } diff --git a/docs/site/docs-en/ci/pipeline-triggers.md b/docs/site/docs-en/ci/pipeline-triggers.md index 90a0aa69..e265061f 100644 --- a/docs/site/docs-en/ci/pipeline-triggers.md +++ b/docs/site/docs-en/ci/pipeline-triggers.md @@ -258,7 +258,9 @@ Since v5.2, `ferrflow release` acquires `ferrflow.lock` atomically (`O_CREAT|O_E You don't need to wire anything up. The lock is automatic on every `release` invocation. Read-only commands (`check`, `status`, `version`, `tag`) skip it. -If a previous run crashed without releasing the lock, the next invocation takes it over automatically after 30 minutes (the host + PID stamped inside the lockfile lets FerrFlow detect stale locks). To take it over sooner, delete `ferrflow.lock` from that git dir manually. +If a previous run crashed without releasing the lock, the next invocation reads the host and PID stamped inside the lockfile and takes it over as soon as it can see that the owner is gone. On the machine that wrote the lock that is immediate, with no waiting: a crashed CI job does not leave the next one blocked. By the same token a release that is still running keeps its lock however long it takes, so a large monorepo publishing for hours is never interrupted by a timeout. + +The 6 hour staleness timeout is only the fallback for a lock this machine cannot ask about, meaning one written by a different host on a shared filesystem, or a lockfile too damaged to read. To take a lock over sooner in that case, run `ferrflow release --force-unlock`, or delete `ferrflow.lock` from the git dir manually. diff --git a/docs/site/docs-fr/ci/pipeline-triggers.md b/docs/site/docs-fr/ci/pipeline-triggers.md index 3ef8828b..53152aa9 100644 --- a/docs/site/docs-fr/ci/pipeline-triggers.md +++ b/docs/site/docs-fr/ci/pipeline-triggers.md @@ -209,7 +209,9 @@ Depuis la v5.2, `ferrflow release` acquiert `ferrflow.lock` de maniere atomique Rien à brancher. Le verrou est automatique sur chaque invocation `release`. Les commandes en lecture seule (`check`, `status`, `version`, `tag`) ne le prennent pas. -Si une execution précédente a planté sans relacher le verrou, l'invocation suivante le reprend automatiquement apres 30 minutes (l'hote + le PID inscrits dans le lockfile permettent à FerrFlow de detecter les verrous orphelins). Pour le reprendre plus tot, supprimez `ferrflow.lock` de ce git dir a la main. +Si une execution precedente a plante sans relacher le verrou, l'invocation suivante lit l'hote et le PID inscrits dans le lockfile et le reprend des qu'elle constate que le proprietaire a disparu. Sur la machine qui a ecrit le verrou, c'est immediat, sans attente : un job CI qui a plante ne bloque pas le suivant. Symetriquement, une release toujours en cours garde son verrou aussi longtemps qu'il faut, donc un gros monorepo qui publie pendant des heures n'est jamais interrompu par un delai d'expiration. + +Le delai de peremption de 6 heures ne sert que de repli pour un verrou sur lequel cette machine ne peut rien savoir, c'est-a-dire ecrit par un autre hote sur un systeme de fichiers partage, ou dans un lockfile trop abime pour etre lu. Pour reprendre un tel verrou plus tot, lancez `ferrflow release --force-unlock`, ou supprimez `ferrflow.lock` du git dir a la main. diff --git a/src/monorepo/run/lock.rs b/src/monorepo/run/lock.rs index 0c2ddb03..92a946fc 100644 --- a/src/monorepo/run/lock.rs +++ b/src/monorepo/run/lock.rs @@ -7,7 +7,8 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use crate::error_code::{self, ErrorCodeExt}; -const STALE_LOCK_TTL: Duration = Duration::from_secs(30 * 60); +const STALE_LOCK_TTL: Duration = Duration::from_secs(6 * 60 * 60); +const UNKNOWN_HOST: &str = "unknown"; /// RAII lock guard for `ferrflow release`. Acquires `ferrflow.lock` in the /// repository's common git dir atomically via O_CREAT|O_EXCL, and releases @@ -30,8 +31,13 @@ pub struct ReleaseLock { impl ReleaseLock { /// Try to acquire the release lock. Returns Err if another live - /// release is in progress. Stale locks (older than STALE_LOCK_TTL - /// with the PID no longer alive) are taken over with a warning. + /// release is in progress. + /// + /// A lock written by this host is judged on whether its PID is still + /// alive: a dead owner is taken over at once however recent the lock, + /// and a live one is never taken over however old. STALE_LOCK_TTL is + /// the fallback for a lock whose owner this host cannot ask about, + /// meaning another machine's PID or an unreadable lockfile. pub fn acquire(repo: &Repository) -> Result { let path = lock_path(repo)?; @@ -119,7 +125,86 @@ fn read_lock_info(path: &Path) -> Option { Some(buf) } +struct LockOwner { + pid: u32, + host: String, +} + +fn parse_lock_info(raw: &str) -> Option { + let mut lines = raw.lines(); + // A pid of 0, or one that wraps negative into unix's pid_t, makes the + // liveness check address a process group rather than a process, which + // always reads as alive. `acquire` writes neither, so treat them as an + // unreadable lockfile and let the TTL decide. + let pid: u32 = lines + .next()? + .trim() + .parse() + .ok() + .filter(|&pid| pid != 0 && pid <= i32::MAX as u32)?; + let _written_at = lines.next()?; + let host = lines.next()?.trim().to_string(); + Some(LockOwner { pid, host }) +} + +#[cfg(unix)] +fn process_is_alive(pid: u32) -> bool { + // SAFETY: signal 0 runs the existence and permission checks without + // delivering anything, and takes no pointer arguments. + let sent = unsafe { libc::kill(pid as libc::pid_t, 0) }; + if sent == 0 { + return true; + } + std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) +} + +#[cfg(windows)] +fn process_is_alive(pid: u32) -> bool { + use windows_sys::Win32::Foundation::{ + CloseHandle, ERROR_ACCESS_DENIED, GetLastError, WAIT_TIMEOUT, + }; + use windows_sys::Win32::Storage::FileSystem::SYNCHRONIZE; + use windows_sys::Win32::System::Threading::{OpenProcess, WaitForSingleObject}; + + // SAFETY: OpenProcess takes no pointers and reports failure by + // returning a null handle, which is checked before the handle is used. + let handle = unsafe { OpenProcess(SYNCHRONIZE, 0, pid) }; + if handle.is_null() { + // A process's default DACL grants SYNCHRONIZE to its owner and + // SYSTEM only, so a release started by another account is opaque to + // us even though it is running. This is the EPERM case on unix. + // SAFETY: GetLastError takes no arguments and reads this thread's + // last error code, set by the failed call directly above. + return unsafe { GetLastError() } == ERROR_ACCESS_DENIED; + } + // SAFETY: `handle` is a live process handle from the call above, and is + // closed exactly once below. + let status = unsafe { WaitForSingleObject(handle, 0) }; + // SAFETY: same handle, not used again after this point. + unsafe { CloseHandle(handle) }; + status == WAIT_TIMEOUT +} + +#[cfg(not(any(unix, windows)))] +fn process_is_alive(_pid: u32) -> bool { + true +} + fn take_over_if_stale(path: &Path) -> Result { + if let Some(owner) = read_lock_info(path).as_deref().and_then(parse_lock_info) + && owner.host != UNKNOWN_HOST + && owner.host == hostname_or_unknown() + { + if process_is_alive(owner.pid) { + return Ok(false); + } + // Reporting a takeover the removal did not perform would recurse: + // `acquire` retries, `create_new` fails the same way, and this + // reaches the same verdict. Returning the removal's own outcome + // falls through to the "already running" error instead. + return Ok(std::fs::remove_file(path).is_ok()); + } + let metadata = match std::fs::metadata(path) { Ok(m) => m, Err(_) => return Ok(false), @@ -132,14 +217,35 @@ fn take_over_if_stale(path: &Path) -> Result { if modified < STALE_LOCK_TTL { return Ok(false); } - let _ = std::fs::remove_file(path); - Ok(true) + Ok(std::fs::remove_file(path).is_ok()) +} + +#[cfg(unix)] +fn system_hostname() -> Option { + let mut buf = vec![0u8; 256]; + // SAFETY: gethostname writes at most `buf.len()` bytes into a buffer we + // own, and the buffer stays alive for the whole call. + let rc = unsafe { libc::gethostname(buf.as_mut_ptr().cast(), buf.len()) }; + if rc != 0 { + return None; + } + let end = buf.iter().position(|b| *b == 0).unwrap_or(buf.len()); + buf.truncate(end); + String::from_utf8(buf).ok() +} + +#[cfg(not(unix))] +fn system_hostname() -> Option { + None } fn hostname_or_unknown() -> String { - std::env::var("HOSTNAME") - .or_else(|_| std::env::var("COMPUTERNAME")) - .unwrap_or_else(|_| "unknown".to_string()) + let named = |host: String| (!host.trim().is_empty()).then_some(host); + system_hostname() + .and_then(named) + .or_else(|| std::env::var("HOSTNAME").ok().and_then(named)) + .or_else(|| std::env::var("COMPUTERNAME").ok().and_then(named)) + .unwrap_or_else(|| UNKNOWN_HOST.to_string()) } #[cfg(test)] @@ -250,6 +356,130 @@ mod tests { drop(lock); } + fn write_lock(repo: &Repository, pid: u32, host: &str, age: Duration) -> PathBuf { + let path = lock_path(repo).unwrap(); + std::fs::write(&path, format!("{pid}\n0\n{host}\n")).unwrap(); + let when = SystemTime::now() - age; + std::fs::OpenOptions::new() + .write(true) + .open(&path) + .unwrap() + .set_times(std::fs::FileTimes::new().set_modified(when)) + .unwrap(); + path + } + + fn a_dead_pid() -> u32 { + let mut child = std::process::Command::new("git") + .arg("--version") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("git should be on PATH"); + let pid = child.id(); + child.wait().unwrap(); + drop(child); + assert!( + !process_is_alive(pid), + "pid {pid} was reaped but still reads as alive, so this test proves nothing" + ); + pid + } + + #[test] + fn the_host_names_itself_without_an_exported_env_var() { + assert_ne!( + hostname_or_unknown(), + UNKNOWN_HOST, + "HOSTNAME is a shell variable bash does not export, so an env-only lookup \ + leaves the liveness check inert on most Linux hosts and CI containers" + ); + } + + #[test] + fn a_process_this_user_may_not_open_still_reads_as_alive() { + // The OS refuses to talk about a process owned by another account: + // EPERM on unix, ERROR_ACCESS_DENIED on Windows. Reading that as + // "gone" would steal the lock from a running release. + #[cfg(unix)] + let foreign = 1; + #[cfg(windows)] + let foreign = 4; + assert!( + process_is_alive(foreign), + "pid {foreign} is always running, whether or not this user can open it" + ); + } + + #[test] + fn a_pid_that_would_address_a_process_group_is_not_trusted() { + let host = hostname_or_unknown(); + assert!(parse_lock_info(&format!("0\n0\n{host}\n")).is_none()); + assert!(parse_lock_info(&format!("4294967295\n0\n{host}\n")).is_none()); + assert!(parse_lock_info(&format!("1234\n0\n{host}\n")).is_some()); + } + + #[test] + fn a_dead_owner_on_this_host_is_taken_over_without_waiting_for_the_ttl() { + let (_dir, repo) = init_test_repo(); + write_lock(&repo, a_dead_pid(), &hostname_or_unknown(), Duration::ZERO); + + let _lock = ReleaseLock::acquire(&repo) + .expect("a lock whose owner is gone should be taken over at once"); + } + + #[test] + fn a_live_owner_keeps_its_lock_however_old_the_lockfile_is() { + let (_dir, repo) = init_test_repo(); + write_lock( + &repo, + std::process::id(), + &hostname_or_unknown(), + STALE_LOCK_TTL * 4, + ); + + let err = ReleaseLock::acquire(&repo) + .expect_err("a running release must not lose its lock to the TTL"); + assert!(format!("{err:?}").contains("already running"), "{err:?}"); + } + + #[test] + fn another_hosts_lock_is_still_judged_on_the_ttl_alone() { + let (_dir, repo) = init_test_repo(); + // Live here, so liveness would say "keep it" if the host were ignored. + write_lock(&repo, std::process::id(), "some-other-host", Duration::ZERO); + let err = ReleaseLock::acquire(&repo).expect_err("a fresh foreign lock still blocks"); + assert!(format!("{err:?}").contains("already running"), "{err:?}"); + + write_lock( + &repo, + std::process::id(), + "some-other-host", + STALE_LOCK_TTL * 2, + ); + let _lock = + ReleaseLock::acquire(&repo).expect("an expired foreign lock is taken over on the TTL"); + } + + #[test] + fn an_unnamed_host_is_never_trusted_for_liveness() { + let (_dir, repo) = init_test_repo(); + write_lock(&repo, a_dead_pid(), UNKNOWN_HOST, Duration::ZERO); + + let err = ReleaseLock::acquire(&repo) + .expect_err("two machines both calling themselves 'unknown' must not compare PIDs"); + assert!(format!("{err:?}").contains("already running"), "{err:?}"); + } + + #[test] + fn an_unreadable_lockfile_falls_back_to_the_ttl() { + let (_dir, repo) = init_test_repo(); + let path = lock_path(&repo).unwrap(); + std::fs::write(&path, "garbage").unwrap(); + let err = ReleaseLock::acquire(&repo).expect_err("a fresh unparseable lock still blocks"); + assert!(format!("{err:?}").contains("already running"), "{err:?}"); + } + #[test] fn lockfile_content_includes_pid() { let (dir, repo) = init_test_repo();