diff --git a/Cargo.lock b/Cargo.lock index 8f19676..bc50e4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -523,7 +523,7 @@ dependencies = [ [[package]] name = "furrow-cli" -version = "0.3.4" +version = "0.3.5" dependencies = [ "anyhow", "assert_cmd", @@ -544,7 +544,7 @@ dependencies = [ [[package]] name = "furrow-core" -version = "0.3.4" +version = "0.3.5" dependencies = [ "anyhow", "blake3", @@ -582,7 +582,7 @@ dependencies = [ [[package]] name = "furrow-daemon" -version = "0.3.4" +version = "0.3.5" dependencies = [ "anyhow", "blake3", diff --git a/Cargo.toml b/Cargo.toml index 9a17e03..6567b2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = ["furrow-core", "furrow-cli", "furrow-daemon"] [workspace.package] -version = "0.3.4" +version = "0.3.5" edition = "2021" rust-version = "1.83" license = "Apache-2.0" diff --git a/furrow-daemon/src/server.rs b/furrow-daemon/src/server.rs index a724060..653a70d 100644 --- a/furrow-daemon/src/server.rs +++ b/furrow-daemon/src/server.rs @@ -10,6 +10,8 @@ use std::io::BufReader; #[cfg(unix)] use std::os::fd::AsRawFd; #[cfg(unix)] +use std::os::unix::fs::MetadataExt; +#[cfg(unix)] use std::os::unix::net::{UnixListener, UnixStream}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -60,6 +62,8 @@ impl Activity { pub struct Server { listener: UnixListener, + #[cfg(unix)] + socket_identity: (u64, u64), config: Config, stop: Stop, router: Arc, @@ -80,10 +84,15 @@ impl Server { }; let _ = fs::remove_file(&config.socket); let listener = UnixListener::bind(&config.socket).context("bind socket")?; + #[cfg(unix)] + let metadata = + fs::symlink_metadata(&config.socket).context("read bound socket identity")?; furrow::platform::set_mode(&config.socket, 0o600)?; listener.set_nonblocking(true)?; Ok(Some(Self { listener, + #[cfg(unix)] + socket_identity: (metadata.dev(), metadata.ino()), config, stop: Stop::default(), router: Arc::new(Router::new()), @@ -100,16 +109,24 @@ impl Server { } /// Serves until stopped, signalled or idle, then finishes in-flight - /// requests, closes every repository and removes the socket. + /// requests, closes every repository and removes the socket if it still owns it. pub fn run(self) -> anyhow::Result<()> { while !self.should_exit() { self.wait_for_client(); self.accept_pending(); } let flushed = self.router.close_durable(); + // A surviving daemon must not unlink a replacement created after its socket disappeared. + #[cfg(unix)] + let remove_socket = fs::symlink_metadata(&self.config.socket) + .is_ok_and(|metadata| (metadata.dev(), metadata.ino()) == self.socket_identity); + #[cfg(windows)] + let remove_socket = true; #[cfg(windows)] drop(self.listener); - let _ = fs::remove_file(&self.config.socket); + if remove_socket { + let _ = fs::remove_file(&self.config.socket); + } #[cfg(windows)] crate::signals::finished(); flushed diff --git a/furrow-daemon/tests/socket_cleanup.rs b/furrow-daemon/tests/socket_cleanup.rs new file mode 100644 index 0000000..5f6a538 --- /dev/null +++ b/furrow-daemon/tests/socket_cleanup.rs @@ -0,0 +1,119 @@ +#![cfg(unix)] + +use furrow_daemon::{Config, Server}; +use std::fs; +use std::os::unix::fs::{symlink, MetadataExt}; +use std::os::unix::net::UnixStream; +use std::path::Path; +use std::time::Duration; + +fn bind(socket: &Path, idle: Duration) -> Server { + Server::bind(Config { + socket: socket.to_owned(), + idle, + }) + .unwrap() + .expect("the socket is available") +} + +fn identity(path: &Path) -> (u64, u64) { + let metadata = fs::symlink_metadata(path).unwrap(); + (metadata.dev(), metadata.ino()) +} + +#[test] +fn c1_idle_exit_preserves_a_replacement_socket() { + // Contract 1 and 4 require an idle exit to leave a newer daemon's socket intact. + let dir = tempfile::tempdir().unwrap(); + let socket = dir.path().join("daemon.sock"); + let first = bind(&socket, Duration::ZERO); + let original = identity(&socket); + fs::remove_file(&socket).unwrap(); + let lock = socket.with_extension("sock.lock"); + fs::remove_file(&lock).unwrap(); + let second = bind(&socket, Duration::from_secs(60)); + let replacement = identity(&socket); + let replacement_lock = identity(&lock); + assert_ne!(original, replacement); + + first.run().unwrap(); + + assert_eq!(identity(&socket), replacement); + assert_eq!(identity(&lock), replacement_lock); + assert!(Server::bind(Config { + socket: socket.clone(), + idle: Duration::ZERO, + }) + .unwrap() + .is_none()); + drop(UnixStream::connect(&socket).unwrap()); + second.stop_handle().stop(); + second.run().unwrap(); + assert!(!socket.exists()); +} + +#[test] +fn c1_stopped_exit_preserves_a_replacement_file() { + // Contract 1 and 4 require an ordinary stop to preserve a replacement file's identity and bytes. + let dir = tempfile::tempdir().unwrap(); + let socket = dir.path().join("daemon.sock"); + let server = bind(&socket, Duration::from_secs(60)); + fs::rename(&socket, dir.path().join("retired.sock")).unwrap(); + fs::write(&socket, b"replacement").unwrap(); + let replacement = identity(&socket); + server.stop_handle().stop(); + + server.run().unwrap(); + + assert_eq!(identity(&socket), replacement); + assert_eq!(fs::read(&socket).unwrap(), b"replacement"); +} + +#[test] +fn c1_stopped_exit_preserves_a_replacement_symlink() { + // Contract 1 requires a replacement link to remain even when it points to the original socket. + let dir = tempfile::tempdir().unwrap(); + let socket = dir.path().join("daemon.sock"); + let server = bind(&socket, Duration::from_secs(60)); + let retired = dir.path().join("retired.sock"); + fs::rename(&socket, &retired).unwrap(); + symlink(&retired, &socket).unwrap(); + let replacement = identity(&socket); + server.stop_handle().stop(); + + server.run().unwrap(); + + assert_eq!(identity(&socket), replacement); + assert_eq!(fs::read_link(&socket).unwrap(), retired); +} + +#[test] +fn c1_idle_exit_leaves_a_missing_socket_absent() { + // Contract 1 requires cleanup to succeed when the socket has already been removed. + let dir = tempfile::tempdir().unwrap(); + let socket = dir.path().join("daemon.sock"); + let server = bind(&socket, Duration::ZERO); + fs::remove_file(&socket).unwrap(); + + server.run().unwrap(); + + assert!(!socket.exists()); +} + +#[test] +fn c1_exit_removes_its_untouched_socket() { + // Contract 1 and 4 require both idle and stopped exits to remove their own unchanged socket. + let dir = tempfile::tempdir().unwrap(); + let socket = dir.path().join("daemon.sock"); + for idle in [Duration::ZERO, Duration::from_secs(60)] { + let server = bind(&socket, idle); + assert!(socket.exists()); + if !idle.is_zero() { + server.stop_handle().stop(); + } + + server.run().unwrap(); + + assert!(!socket.exists()); + } +}