watt: simplify lock acquisition; force single exclusive lock

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: I563f9989cf73ce99f227555a29c770916a6a6964

Author: NotAShelf

watt/lib.rs

@@ -1,7 +1,4 @@
-use std::{
-  env,
-  path::PathBuf,
-};
+use std::path::PathBuf;
 
 use anyhow::Context as _;
 use clap::Parser as _;
@@ -25,11 +22,6 @@ pub struct Cli {
   /// The daemon config path.
   #[arg(long, env = "WATT_CONFIG")]
   config: Option<PathBuf>,
-
-  /// Force running even if another instance is already running. Potentially
-  /// destructive.
-  #[arg(long)]
-  force: bool,
 }
 
 pub fn main() -> anyhow::Result<()> {
@@ -48,14 +40,13 @@ pub fn main() -> anyhow::Result<()> {
 
   log::info!("starting watt daemon");
 
-  let lock_path = env::var_os("XDG_RUNTIME_DIR")
-    .map_or_else(|| PathBuf::from("/run"), PathBuf::from)
-    .join("watt.pid");
-
-  let _lock =
-    lock::LockFile::acquire(&lock_path, cli.force).with_context(|| {
-      format!("failed to acquire pid lock at {}", lock_path.display())
-    })?;
+  let lock_path = PathBuf::from("/run/watt.lock");
+  let _lock = lock::LockFile::acquire(&lock_path).with_context(|| {
+    format!(
+      "failed to acquire exclusive lock at {}",
+      lock_path.display()
+    )
+  })?;
 
   system::run_daemon(config)
 }

watt/lock.rs

@@ -2,17 +2,15 @@ use std::{
   error::Error,
   fmt,
   fs::{
-    self,
     File,
     OpenOptions,
   },
-  io::Write,
   ops,
+  os::unix::fs::OpenOptionsExt,
   path::{
     Path,
     PathBuf,
   },
-  process,
 };
 
 #[cfg(unix)] use nix::fcntl::{
@@ -30,17 +28,17 @@ pub struct LockFile {
 
 #[derive(Debug)]
 pub struct LockFileError {
-  pub path: PathBuf,
-  pid:      u32,
+  pub path:    PathBuf,
+  pub message: Option<String>,
 }
 
 impl fmt::Display for LockFileError {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-    if self.pid == 0 {
-      write!(f, "failed to acquire lock at {}", self.path.display())
-    } else {
-      write!(f, "another watt daemon is running (PID: {})", self.pid)
+    write!(f, "failed to acquire lock on {}", self.path.display())?;
+    if let Some(msg) = &self.message {
+      write!(f, ": {}", msg)?;
     }
+    Ok(())
   }
 }
 
@@ -65,17 +63,13 @@ impl LockFile {
     &self.path
   }
 
-  pub fn acquire(
-    lock_path: &Path,
-    force: bool,
-  ) -> Result<Option<Self>, LockFileError> {
-    let pid = process::id();
-
+  pub fn acquire(lock_path: &Path) -> Result<Self, LockFileError> {
     #[allow(clippy::suspicious_open_options)]
     let file = OpenOptions::new()
       .create(true)
       .read(true)
       .write(true)
+      .mode(0o600)
       .open(lock_path)
       .map_err(|error| {
         log::error!(
@@ -84,96 +78,34 @@ impl LockFile {
           error
         );
         LockFileError {
-          path: lock_path.to_owned(),
-          pid:  0,
+          path:    lock_path.to_owned(),
+          message: Some(error.to_string()),
         }
       })?;
 
-    let mut lock = match Flock::lock(file, FlockArg::LockExclusiveNonblock) {
-      Ok(lock) => lock,
-      Err((_, nix::errno::Errno::EWOULDBLOCK)) => {
-        let Some(existing_pid) = Self::read_pid(lock_path) else {
-          if force {
-            log::warn!(
-              "could not determine PID of existing watt instance, starting \
-               anyway",
-            );
-            return Ok(None);
-          }
-
-          return Err(LockFileError {
-            path: lock_path.to_owned(),
-            pid:  0,
-          });
-        };
-
-        if force {
-          log::warn!(
-            "another watt instance is running (PID: {existing_pid}), starting \
-             anyway",
+    let lock = Flock::lock(file, FlockArg::LockExclusiveNonblock).map_err(
+      |(_, error)| {
+        let message = if error == nix::errno::Errno::EWOULDBLOCK {
+          log::error!(
+            "another watt instance is already running (lock held on {})",
+            lock_path.display()
           );
-          return Ok(None);
-        }
-
-        return Err(LockFileError {
-          path: lock_path.to_owned(),
-          pid:  existing_pid,
-        });
-      },
+          Some("another instance is running".to_string())
+        } else {
+          log::error!("failed to acquire lock: {}", error);
+          Some(error.to_string())
+        };
 
-      Err((_, error)) => {
-        log::error!("failed to acquire lock: {}", error);
-        return Err(LockFileError {
+        LockFileError {
           path: lock_path.to_owned(),
-          pid:  0,
-        });
+          message,
+        }
       },
-    };
-
-    if let Err(e) = lock.set_len(0) {
-      log::error!("failed to truncate lock file: {}", e);
-      return Err(LockFileError {
-        path: lock_path.to_owned(),
-        pid:  0,
-      });
-    }
-
-    if let Err(e) = lock.write_all(format!("{pid}\n").as_bytes()) {
-      log::error!("failed to write PID to lock file: {}", e);
-      return Err(LockFileError {
-        path: lock_path.to_owned(),
-        pid:  0,
-      });
-    }
+    )?;
 
-    if let Err(e) = lock.sync_all() {
-      log::error!("failed to sync lock file: {}", e);
-      return Err(LockFileError {
-        path: lock_path.to_owned(),
-        pid:  0,
-      });
-    }
-
-    Ok(Some(LockFile {
+    Ok(LockFile {
       lock,
       path: lock_path.to_owned(),
-    }))
-  }
-
-  fn read_pid(lock_path: &Path) -> Option<u32> {
-    match fs::read_to_string(lock_path) {
-      Ok(content) => content.trim().parse().ok(),
-      Err(_) => None,
-    }
-  }
-
-  pub fn release(&mut self) {
-    let _ = fs::remove_file(&self.path);
-  }
-}
-
-impl Drop for LockFile {
-  fn drop(&mut self) {
-    self.release();
+    })
   }
 }