diff --git a/Cargo.lock b/Cargo.lock index f94cbed..ad112c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1314,6 +1314,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs4" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8640e34b88f7652208ce9e88b1a37a2ae95227d84abec377ccd3c5cfeb141ed4" +dependencies = [ + "rustix 1.1.4", + "windows-sys 0.59.0", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -2636,6 +2646,7 @@ dependencies = [ "arc-swap", "async-trait", "bytes", + "fs4", "hex", "nuncio-cal", "nuncio-cli", diff --git a/Cargo.toml b/Cargo.toml index e2e4e76..a0ca121 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,7 @@ prost = "0.14" tonic-prost-build = "0.14" protoc-bin-vendored = "3.2" subtle = "2.6" +fs4 = "0.13" diff --git a/crates/nunciod/Cargo.toml b/crates/nunciod/Cargo.toml index a43ef22..d1ee17f 100644 --- a/crates/nunciod/Cargo.toml +++ b/crates/nunciod/Cargo.toml @@ -34,6 +34,7 @@ subtle.workspace = true hex.workspace = true bytes.workspace = true zeroize.workspace = true +fs4.workspace = true [dev-dependencies] nuncio-cli = { path = "../nuncio-cli" } diff --git a/crates/nunciod/src/lib.rs b/crates/nunciod/src/lib.rs index 9a10ad9..0f3ef2d 100644 --- a/crates/nunciod/src/lib.rs +++ b/crates/nunciod/src/lib.rs @@ -6,6 +6,7 @@ pub mod calendar_sync; pub mod contacts_sync; pub mod grpc; pub mod lifecycle; +pub mod lock; pub mod orchestrator; pub mod outbox; pub mod send; diff --git a/crates/nunciod/src/lock.rs b/crates/nunciod/src/lock.rs new file mode 100644 index 0000000..130eea5 --- /dev/null +++ b/crates/nunciod/src/lock.rs @@ -0,0 +1,180 @@ +//! Exclusive, cross-platform advisory lock on the daemon's database path. +//! +//! Nothing about starting a second `nunciod` process today prevents it from +//! opening the same SQLite database another instance already has open -- +//! the only accidental protection is a port-bind collision on the gRPC +//! listener, and that only fires if both instances also happen to pick the +//! same port. Two writers on one SQLite file corrupt state. +//! +//! [`InstanceLock::acquire`] takes an OS-level exclusive lock (`flock` on +//! Unix, `LockFileEx` on Windows via the `fs4` crate) on a sidecar file next +//! to the database, `.lock`. The lock is scoped to the OS file +//! handle, not tracked by a PID written into the file, so it cannot go +//! stale: if the holding process dies for any reason (crash, kill -9), the +//! OS releases the lock the moment the file descriptor/handle closes. + +use std::fs::{File, OpenOptions}; +use std::path::{Path, PathBuf}; + +use fs4::fs_std::FileExt; + +/// Failure to acquire the single-instance lock. +#[derive(Debug, thiserror::Error)] +pub enum InstanceLockError { + /// The lock is already held by another live process. The caller must + /// treat this as fatal and refuse to open the store. + #[error("another nunciod instance is already running for this database at {0}")] + AlreadyRunning(PathBuf), + + /// The lock file itself could not be created/opened/locked, e.g. a + /// permissions problem or an unwritable parent directory. + #[error("failed to acquire instance lock at {path}: {source}")] + Io { + path: PathBuf, + #[source] + source: std::io::Error, + }, +} + +/// Holds the exclusive lock on the database path for as long as this guard +/// is alive. Drop it (or let it fall out of scope at process exit) to +/// release the lock; a later call to [`InstanceLock::acquire`] for the same +/// path then succeeds again. +#[derive(Debug)] +pub struct InstanceLock { + file: File, + path: PathBuf, +} + +impl InstanceLock { + /// Acquires the exclusive lock for `db_path`, creating the sidecar lock + /// file (`.lock`) if it does not already exist. + /// + /// Returns [`InstanceLockError::AlreadyRunning`] if another process + /// already holds it -- callers must fail startup on this rather than + /// proceed to open the store, since a second writer on the same SQLite + /// file is exactly the corruption this guards against. + pub fn acquire(db_path: &Path) -> Result { + let lock_path = lock_path_for(db_path); + + if let Some(parent) = lock_path.parent() { + std::fs::create_dir_all(parent).map_err(|source| InstanceLockError::Io { + path: lock_path.clone(), + source, + })?; + } + + let file = OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(&lock_path) + .map_err(|source| InstanceLockError::Io { + path: lock_path.clone(), + source, + })?; + + let acquired = file + .try_lock_exclusive() + .map_err(|source| InstanceLockError::Io { + path: lock_path.clone(), + source, + })?; + + if !acquired { + return Err(InstanceLockError::AlreadyRunning(db_path.to_path_buf())); + } + + Ok(Self { + file, + path: lock_path, + }) + } +} + +impl Drop for InstanceLock { + fn drop(&mut self) { + // The OS releases a `flock`/`LockFileEx` lock automatically when the + // underlying handle closes, so a failure here is not a leaked lock + // (the process exiting, or `self.file` being dropped right after, + // clears it regardless) -- it is only logged so an operator sees + // anything unexpected in the unlock call itself. + if let Err(err) = self.file.unlock() { + tracing::warn!( + path = %self.path.display(), + error = %err, + "failed to explicitly release nunciod instance lock; it will still be \ + released when the process exits and the underlying file handle closes" + ); + } + } +} + +fn lock_path_for(db_path: &Path) -> PathBuf { + let mut file_name = db_path.as_os_str().to_owned(); + file_name.push(".lock"); + PathBuf::from(file_name) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn second_acquire_for_same_path_fails_while_first_guard_is_held() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let first = InstanceLock::acquire(&db_path).unwrap(); + + let second = InstanceLock::acquire(&db_path); + assert!( + matches!(second, Err(InstanceLockError::AlreadyRunning(ref p)) if p == &db_path), + "expected AlreadyRunning({db_path:?}), got {second:?}" + ); + + drop(first); + } + + #[test] + fn acquire_succeeds_again_after_the_first_guard_is_dropped() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let first = InstanceLock::acquire(&db_path).unwrap(); + drop(first); + + let second = InstanceLock::acquire(&db_path); + assert!( + second.is_ok(), + "acquiring after the prior guard dropped must succeed, got {:?}", + second.err() + ); + } + + #[test] + fn error_message_names_the_database_path() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let first = InstanceLock::acquire(&db_path).unwrap(); + let second = InstanceLock::acquire(&db_path).unwrap_err(); + + let message = second.to_string(); + assert!(message.contains("already running")); + assert!(message.contains(&db_path.display().to_string())); + + drop(first); + } + + #[test] + fn lock_file_is_created_beside_the_database_path() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let guard = InstanceLock::acquire(&db_path).unwrap(); + assert!(dir.path().join("nuncio.db.lock").exists()); + + drop(guard); + } +} diff --git a/crates/nunciod/src/main.rs b/crates/nunciod/src/main.rs index d50f90b..29bb348 100644 --- a/crates/nunciod/src/main.rs +++ b/crates/nunciod/src/main.rs @@ -46,6 +46,14 @@ async fn main() -> Result<(), Box> { .map(std::path::PathBuf::from) .unwrap_or_else(|_| nunciod::default_db_path()); + // Exclusive single-instance lock on the database path, acquired BEFORE + // the store is opened. Held for the entire process lifetime (dropped at + // the end of `main`, releasing it on any clean exit) so a second + // `nunciod` pointed at the same database fails fast instead of the two + // instances corrupting SQLite state between them. + let _instance_lock = + nunciod::lock::InstanceLock::acquire(&db_path).map_err(|e| format!("{e}"))?; + let orchestrator = nunciod::SelfHealingSyncOrchestrator::new(&db_path, event_bus.clone()); let (db, _summary) = orchestrator.initialize_and_recover().await?;