From bb296717339904ed264895ffdbf4b88139518d34 Mon Sep 17 00:00:00 2001 From: Brummel Date: Sat, 18 Apr 2026 11:37:16 +0200 Subject: [PATCH] Implement single-instance lock Add a mechanism to prevent multiple instances of the desktop client from running simultaneously. This is achieved by creating a lock file. The first instance successfully acquires the lock, while subsequent attempts by other instances will fail, prompting them to exit gracefully. This ensures data integrity and prevents potential conflicts. --- client-desktop/src/main.rs | 56 ++++++++++++++++++- client-desktop/src/paths.rs | 9 +++ client-desktop/src/uploader.rs | 26 ++++++++- .../tests/single_instance_lock_test.rs | 49 ++++++++++++++++ 4 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 client-desktop/tests/single_instance_lock_test.rs diff --git a/client-desktop/src/main.rs b/client-desktop/src/main.rs index 93a0650..9c6a812 100644 --- a/client-desktop/src/main.rs +++ b/client-desktop/src/main.rs @@ -11,8 +11,10 @@ use eframe::egui; use tracing::info; use tracing_subscriber::EnvFilter; +use std::fs::{File, OpenOptions, TryLockError}; + use crate::app::DoctateApp; -use crate::paths::{cases_dir, pending_dir, snapshot_cache_path}; +use crate::paths::{cases_dir, lock_file_path, pending_dir, snapshot_cache_path}; fn main() -> eframe::Result { tracing_subscriber::fmt() @@ -39,6 +41,12 @@ fn main() -> eframe::Result { std::process::exit(1); } }; + + // Single-instance guard. The lock is held by the `_lock_file` binding + // for the entire `main` lifetime — OS releases it automatically on + // process exit or crash, so a stale lock file after a crash is + // harmless (the next launch re-acquires). + let _lock_file = acquire_single_instance_lock(); let cases = match cases_dir() { Ok(p) => p, Err(e) => { @@ -71,3 +79,49 @@ fn main() -> eframe::Result { }), ) } + +/// Open and exclusively lock a per-user `client.lock`. Returns the +/// handle; dropping it (process exit) releases the lock. If another +/// instance already holds it, prints a friendly message and exits. +fn acquire_single_instance_lock() -> File { + let path = match lock_file_path() { + Ok(p) => p, + Err(e) => { + eprintln!("cannot determine lock file path: {e}"); + std::process::exit(1); + } + }; + if let Some(parent) = path.parent() + && let Err(e) = std::fs::create_dir_all(parent) + { + eprintln!("cannot create data directory {}: {e}", parent.display()); + std::process::exit(1); + } + let file = match OpenOptions::new() + .create(true) + .read(true) + .write(true) + .truncate(false) + .open(&path) + { + Ok(f) => f, + Err(e) => { + eprintln!("cannot open lock file {}: {e}", path.display()); + std::process::exit(1); + } + }; + match file.try_lock() { + Ok(()) => file, + Err(TryLockError::WouldBlock) => { + eprintln!( + "doctate läuft bereits (Lock hält auf {}). Nur eine Instanz pro User zulässig.", + path.display() + ); + std::process::exit(1); + } + Err(TryLockError::Error(e)) => { + eprintln!("lock acquire failed on {}: {e}", path.display()); + std::process::exit(1); + } + } +} diff --git a/client-desktop/src/paths.rs b/client-desktop/src/paths.rs index 9d9df47..fec4051 100644 --- a/client-desktop/src/paths.rs +++ b/client-desktop/src/paths.rs @@ -38,3 +38,12 @@ pub fn snapshot_cache_path() -> Result { .map(|dirs| dirs.data_local_dir().join("oneliners_cache.json")) .ok_or(PathError::NoDataDir) } + +/// File path for the single-instance advisory lock. Held by the running +/// process for its entire lifetime; a second launch attempting to lock +/// the same file gets `WouldBlock` and exits cleanly. +pub fn lock_file_path() -> Result { + ProjectDirs::from("", "", "doctate") + .map(|dirs| dirs.data_local_dir().join("client.lock")) + .ok_or(PathError::NoDataDir) +} diff --git a/client-desktop/src/uploader.rs b/client-desktop/src/uploader.rs index 8129fa4..4712d90 100644 --- a/client-desktop/src/uploader.rs +++ b/client-desktop/src/uploader.rs @@ -101,14 +101,20 @@ pub fn sidecar_path_for(m4a: &Path) -> PathBuf { dir.join(format!("{stem}.meta.json")) } -/// Persist a pending upload's metadata next to its m4a file. +/// Persist a pending upload's metadata next to its m4a file. Atomic: +/// writes a `*.tmp` sibling and renames it into place, so a crash +/// mid-write cannot leave a zero-byte or partial sidecar on disk. +/// Without this guarantee, the `scan_pending_dir` recovery path would +/// silently drop the upload as an unreadable orphan on next launch. pub fn write_sidecar(upload: &PendingUpload) -> Result<(), UploaderError> { let sidecar = sidecar_path_for(&upload.file); if let Some(parent) = sidecar.parent() { std::fs::create_dir_all(parent)?; } let json = serde_json::to_string_pretty(upload)?; - std::fs::write(&sidecar, json)?; + let tmp = sidecar.with_extension("json.tmp"); + std::fs::write(&tmp, json)?; + std::fs::rename(&tmp, &sidecar)?; Ok(()) } @@ -314,6 +320,22 @@ mod tests { assert!(scan_pending_dir(&missing).is_empty()); } + /// Atomic sidecar write must leave no `.tmp` leftover on success — + /// the recovery scan only accepts exact `.meta.json` files, so a + /// stray temp file would be invisible but wastes inodes, and hints + /// at a rename bug if it ever appears. + #[test] + fn write_sidecar_leaves_no_tmp_artefact() { + let tmp = TempDir::new().unwrap(); + let upload = sample_pending(tmp.path()); + write_sidecar(&upload).unwrap(); + + let sidecar = sidecar_path_for(&upload.file); + assert!(sidecar.exists(), "sidecar must be at final path"); + let tmp_path = sidecar.with_extension("json.tmp"); + assert!(!tmp_path.exists(), "tmp artefact must not remain"); + } + #[tokio::test] async fn successful_upload_deletes_files_and_emits_events() { let server = MockServer::start().await; diff --git a/client-desktop/tests/single_instance_lock_test.rs b/client-desktop/tests/single_instance_lock_test.rs new file mode 100644 index 0000000..15289ac --- /dev/null +++ b/client-desktop/tests/single_instance_lock_test.rs @@ -0,0 +1,49 @@ +//! Regression guard for the single-instance lock in `main.rs`. The +//! implementation relies on `std::fs::File::try_lock` (stable since +//! Rust 1.89) — specifically that opening the same file a second time +//! from within the same process and calling `try_lock` yields +//! `TryLockError::WouldBlock` while the first handle lives. +//! +//! Two-process testing is out of scope (would need `std::process::Command` +//! with shared state); this test covers the in-process assumption which +//! is what our `acquire_single_instance_lock` depends on. + +use std::fs::{OpenOptions, TryLockError}; + +use tempfile::TempDir; + +#[test] +fn second_try_lock_on_same_file_blocks() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("client.lock"); + + let first = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .truncate(false) + .open(&path) + .unwrap(); + first.try_lock().expect("first lock must succeed"); + + let second = OpenOptions::new() + .read(true) + .write(true) + .open(&path) + .unwrap(); + match second.try_lock() { + Err(TryLockError::WouldBlock) => {} + Ok(()) => panic!("second try_lock unexpectedly succeeded"), + Err(e) => panic!("unexpected lock error: {e:?}"), + } + + // Dropping `first` releases the OS lock; the third attempt then + // succeeds — proves the lock truly scopes to the file handle. + drop(first); + let third = OpenOptions::new() + .read(true) + .write(true) + .open(&path) + .unwrap(); + third.try_lock().expect("after drop, re-lock must succeed"); +}