Fix: Paar-Sperre ist jetzt prozessuebergreifend statt nur prozessintern
mount::lock::acquire() war ein rein prozessinterner Mutex<HashMap<...>>. smart-mount ist aber ein One-Shot-CLI (siehe reconcile-Moduldoku) - jeder Aufruf ist ein eigener Prozess mit eigener, leerer Registry. Ein manuelles `unmount --name X` waehrend ein gleichzeitig laufender systemd-Timer- `watch` fuer dasselbe Paar arbeitet, kollidierte dadurch ungebremst statt serialisiert zu werden. acquire() sperrt jetzt ueber eine `flock(2)`-Datei pro Paar (`std::fs::File::lock()`, seit Rust 1.89 Teil der Standardbibliothek) in einem root- bzw. XDG_RUNTIME_DIR-basierten Verzeichnis - damit blockieren sich zwei Prozesse fuer dasselbe Paar tatsaechlich gegenseitig. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+100
-21
@@ -1,31 +1,110 @@
|
|||||||
//! Pro-Paar-Mutex-Registry, damit ein manueller `mount --name X` nicht mit einem
|
//! Pro-Paar-Dateisperre, damit ein manueller `mount --name X` nicht mit einem gleichzeitig
|
||||||
//! gleichzeitig laufenden `watch` für dasselbe Paar kollidiert.
|
//! laufenden `watch` für dasselbe Paar kollidiert.
|
||||||
|
//!
|
||||||
|
//! **Warum eine Datei-basierte Sperre (`flock(2)`) statt eines simplen `Mutex`:** smart-mount
|
||||||
|
//! ist ein One-Shot-CLI (siehe [`crate::reconcile`]-Moduldoku) - jeder Aufruf ist ein eigener,
|
||||||
|
//! kurzlebiger Prozess. Ein rein prozessinterner `Mutex<HashMap<...>>` schützt daher NICHT vor
|
||||||
|
//! zwei gleichzeitig laufenden `smart-mount`-Prozessen (z. B. ein manuelles `unmount --name X`
|
||||||
|
//! während ein systemd-Timer-`watch` läuft): beide bekämen ihre eigene, leere Registry und
|
||||||
|
//! würden sich nie gegenseitig blockieren. Die Sperre muss daher außerhalb des Prozess-Speichers
|
||||||
|
//! liegen - hier über eine `flock`-Datei pro Paar (`std::fs::File::lock()`, seit Rust 1.89
|
||||||
|
//! Teil der Standardbibliothek, keine zusätzliche Abhängigkeit nötig).
|
||||||
//!
|
//!
|
||||||
//! Ersetzt den globalen `RwLock`+`Mutex` aus dem alten `src/filesystem/mount.rs` (v0.2.0):
|
//! Ersetzt den globalen `RwLock`+`Mutex` aus dem alten `src/filesystem/mount.rs` (v0.2.0):
|
||||||
//! dort war die Sperre prozessweit global, hier ist sie pro Laufwerkspaar - mehrere Paare
|
//! dort war die Sperre prozessweit global, hier ist sie pro Laufwerkspaar - mehrere Paare
|
||||||
//! können also parallel gemountet werden, ohne sich gegenseitig zu blockieren.
|
//! können also parallel gemountet werden, ohne sich gegenseitig zu blockieren.
|
||||||
|
|
||||||
use std::collections::HashMap;
|
use std::fs::{File, OpenOptions};
|
||||||
use std::sync::{Arc, Mutex, OnceLock};
|
use std::path::PathBuf;
|
||||||
|
|
||||||
use tokio::sync::{Mutex as AsyncMutex, OwnedMutexGuard};
|
use crate::error::{Error, Result};
|
||||||
|
|
||||||
type Registry = Mutex<HashMap<String, Arc<AsyncMutex<()>>>>;
|
/// Hält die Sperrdatei offen (und damit die `flock`-Sperre über den zugehörigen
|
||||||
|
/// Datei-Deskriptor), bis der Guard gedroppt wird - Schließen des Deskriptors gibt die Sperre
|
||||||
fn registry() -> &'static Registry {
|
/// implizit frei, ein explizites `unlock()` ist dafür nicht nötig.
|
||||||
static REGISTRY: OnceLock<Registry> = OnceLock::new();
|
pub struct PairLock {
|
||||||
REGISTRY.get_or_init(|| Mutex::new(HashMap::new()))
|
_file: File,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Sperrt ein Laufwerkspaar für die Dauer des zurückgegebenen Guards. `tokio::sync::Mutex`s
|
/// Verzeichnis für die Sperrdateien: `/run/smart-mount/locks` im Root-/System-Kontext (root ist
|
||||||
/// `lock_owned()` erlaubt einen Guard, der seine eigene `Arc`-Referenz hält - keine
|
/// dort ohnehin die einzige Partei, die Paare in diesem Kontext mountet), sonst
|
||||||
/// selbstreferenzielle Struktur/`unsafe` nötig.
|
/// `$XDG_RUNTIME_DIR` (per-Nutzer, von systemd `0700`-geschützt angelegt) mit Fallback auf das
|
||||||
pub async fn acquire(pair_id: &str) -> OwnedMutexGuard<()> {
|
/// System-Temp-Verzeichnis, falls `$XDG_RUNTIME_DIR` nicht gesetzt ist (z. B. ohne aktive
|
||||||
let mutex = {
|
/// Login-Session).
|
||||||
let mut reg = registry().lock().unwrap_or_else(|e| e.into_inner());
|
fn lock_dir() -> PathBuf {
|
||||||
reg.entry(pair_id.to_string())
|
if sudo_ctdra::is_run_as_root() {
|
||||||
.or_insert_with(|| Arc::new(AsyncMutex::new(())))
|
PathBuf::from("/run/smart-mount/locks")
|
||||||
.clone()
|
} else {
|
||||||
};
|
std::env::var_os("XDG_RUNTIME_DIR")
|
||||||
mutex.lock_owned().await
|
.map(PathBuf::from)
|
||||||
|
.unwrap_or_else(std::env::temp_dir)
|
||||||
|
.join("smart-mount-locks")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn acquire_blocking(pair_id: &str) -> Result<PairLock> {
|
||||||
|
let dir = lock_dir();
|
||||||
|
std::fs::create_dir_all(&dir).map_err(|e| Error::io(&dir, e))?;
|
||||||
|
|
||||||
|
let path = dir.join(format!("{pair_id}.lock"));
|
||||||
|
// `truncate(false)`: der Dateiinhalt ist irrelevant, nur der Deskriptor/die `flock`-Sperre
|
||||||
|
// darauf zählt - ein Zurücksetzen auf leer bei jedem Aufruf wäre unnötig.
|
||||||
|
let file = OpenOptions::new()
|
||||||
|
.create(true)
|
||||||
|
.truncate(false)
|
||||||
|
.write(true)
|
||||||
|
.open(&path)
|
||||||
|
.map_err(|e| Error::io(&path, e))?;
|
||||||
|
|
||||||
|
// Blockiert, bis die Sperre frei wird - `flock(2)` kennt keinen Async-Mechanismus, daher
|
||||||
|
// läuft dieser gesamte Aufruf über `spawn_blocking` (siehe [`acquire`]) auf einem
|
||||||
|
// Blocking-Thread statt einem Tokio-Worker-Thread.
|
||||||
|
file.lock().map_err(|e| Error::io(&path, e))?;
|
||||||
|
Ok(PairLock { _file: file })
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Sperrt ein Laufwerkspaar prozessübergreifend für die Dauer des zurückgegebenen Guards.
|
||||||
|
pub async fn acquire(pair_id: &str) -> Result<PairLock> {
|
||||||
|
let pair_id = pair_id.to_string();
|
||||||
|
tokio::task::spawn_blocking(move || acquire_blocking(&pair_id))
|
||||||
|
.await
|
||||||
|
.map_err(|e| Error::Other(format!("lock task panicked: {e}")))?
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn acquire_returns_a_guard_and_releases_on_drop() {
|
||||||
|
let id = format!("test-lock-{}", uuid::Uuid::new_v4());
|
||||||
|
{
|
||||||
|
let _guard = acquire(&id).await.expect("first acquire");
|
||||||
|
}
|
||||||
|
// Sollte nicht blockieren: der obige Guard wurde bereits gedroppt.
|
||||||
|
let _guard2 = acquire(&id).await.expect("second acquire after drop");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_second_concurrent_acquire_waits_for_the_first_to_be_dropped() {
|
||||||
|
let id = format!("test-lock-{}", uuid::Uuid::new_v4());
|
||||||
|
let guard = acquire(&id).await.expect("first acquire");
|
||||||
|
|
||||||
|
let id2 = id.clone();
|
||||||
|
let handle = tokio::spawn(async move { acquire(&id2).await });
|
||||||
|
|
||||||
|
// Kurz warten, damit der spawnte Task realistischerweise Zeit hatte, in `acquire` zu
|
||||||
|
// blockieren, bevor die erste Sperre freigegeben wird.
|
||||||
|
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
|
||||||
|
assert!(
|
||||||
|
!handle.is_finished(),
|
||||||
|
"second acquire must block while the first guard is held"
|
||||||
|
);
|
||||||
|
|
||||||
|
drop(guard);
|
||||||
|
handle
|
||||||
|
.await
|
||||||
|
.expect("task")
|
||||||
|
.expect("second acquire after release");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -74,7 +74,7 @@ async fn reconcile_pair_inner(
|
|||||||
settings: &GlobalSettings,
|
settings: &GlobalSettings,
|
||||||
creds: &CredentialStore,
|
creds: &CredentialStore,
|
||||||
) -> Result<Action> {
|
) -> Result<Action> {
|
||||||
let _guard = lock::acquire(&pair.id).await;
|
let _guard = lock::acquire(&pair.id).await?;
|
||||||
|
|
||||||
let local_reachable = check_local_reachable(&pair.local, settings);
|
let local_reachable = check_local_reachable(&pair.local, settings);
|
||||||
let active = target::active_side(pair);
|
let active = target::active_side(pair);
|
||||||
@@ -167,7 +167,7 @@ async fn mount_side(
|
|||||||
/// ein leeres, ausgehängtes Backing-Verzeichnis) - der nächste `mount`/`watch`-Lauf räumt das
|
/// ein leeres, ausgehängtes Backing-Verzeichnis) - der nächste `mount`/`watch`-Lauf räumt das
|
||||||
/// beim erneuten Aktivieren automatisch auf.
|
/// beim erneuten Aktivieren automatisch auf.
|
||||||
pub async fn unmount_pair(pair: &DrivePair, settings: &GlobalSettings) -> Result<Action> {
|
pub async fn unmount_pair(pair: &DrivePair, settings: &GlobalSettings) -> Result<Action> {
|
||||||
let _guard = lock::acquire(&pair.id).await;
|
let _guard = lock::acquire(&pair.id).await?;
|
||||||
|
|
||||||
let Some(side) = target::active_side(pair) else {
|
let Some(side) = target::active_side(pair) else {
|
||||||
return Ok(Action::NoOp);
|
return Ok(Action::NoOp);
|
||||||
|
|||||||
Reference in New Issue
Block a user