From b3f9af96ecd79f5a8dadeaf2c386d2c5b2b4aae6 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 21:15:27 -0400 Subject: [PATCH 1/2] fix: an exiting daemon removes only its own socket Record the Unix socket's device and inode when it is bound, and at shutdown remove the path only while it still names that socket. A daemon that outlived its client used to unlink the socket a newer daemon had bound at the same path, which left the newer one listening, holding the lock, and unreachable. Idle, signalled and requested shutdowns all go through the same check; Windows is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- furrow-daemon/src/server.rs | 21 ++++- furrow-daemon/tests/socket_cleanup.rs | 119 ++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 furrow-daemon/tests/socket_cleanup.rs 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()); + } +} From 4e0d4861325764aa428d07fe82396536ba350f8b Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Mon, 26 Oct 2026 21:15:27 -0400 Subject: [PATCH 2/2] release: 0.3.5 (a daemon removes only its own socket) Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 6 +++--- Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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"