feat(lock): add locking for concurrent process safety

Cargo.toml

@@ -37,7 +37,7 @@ image = { version = "0.25.9", default-features = false, features = ["png"] }
 landlock = "0.4.4"
 libsqlite3-sys = { version = ">=0.30.1,<0.36.0", features = [ "bundled" ]}
 miette = { version = "7.6.0", features = ["fancy"] }
-nix = { version = "0.30.1", features = ["ioctl", "term", "user"] }
+nix = { version = "0.30.1", features = ["fs", "ioctl", "term", "user"] }
 percent-encoding = "2.3.2"
 rayon = "1.11.0"
 regex = { version = "1.12.2", default-features = false, features = [

crates/soar-cli/src/install.rs

@@ -7,6 +7,7 @@ use std::{
         atomic::{AtomicU64, Ordering},
         Arc, Mutex,
     },
+    time::Duration,
 };
 
 use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
@@ -25,13 +26,14 @@ use soar_core::{
     SoarResult,
 };
 use soar_db::repository::{
-    core::{CoreRepository, SortDirection},
+    core::{CoreRepository, InstalledPackageWithPortable, SortDirection},
     metadata::MetadataRepository,
 };
 use soar_dl::types::Progress;
 use soar_package::integrate_package;
 use soar_utils::{
     hash::{calculate_checksum, hash_string},
+    lock::FileLock,
     pattern::apply_sig_variants,
 };
 use tabled::{
@@ -1038,11 +1040,14 @@ async fn spawn_installation_task(
 
         match result {
             Ok((install_dir, symlinks)) => {
-                installed_indices
-                    .lock()
-                    .unwrap()
-                    .insert(idx, (install_dir, symlinks));
-                installed_count.fetch_add(1, Ordering::Relaxed);
+                // Only count as installed if actually installed (not skipped)
+                if !install_dir.as_os_str().is_empty() {
+                    installed_indices
+                        .lock()
+                        .unwrap()
+                        .insert(idx, (install_dir, symlinks));
+                    installed_count.fetch_add(1, Ordering::Relaxed);
+                }
                 total_pb.inc(1);
 
                 let _ = remove_old_versions(&target.package, &core_db, false);
@@ -1085,6 +1090,59 @@ pub async fn install_single_package(
         target.package.repo_name,
         target.package.version
     );
+
+    let mut lock_attempts = 0;
+    let _package_lock = loop {
+        match FileLock::try_acquire(&target.package.pkg_name) {
+            Ok(Some(lock)) => break Ok(lock),
+            Ok(None) => {
+                lock_attempts += 1;
+                if lock_attempts == 1 {
+                    info!(
+                        "Waiting for lock on '{}' (another process is installing)...",
+                        target.package.pkg_name
+                    );
+                }
+                tokio::time::sleep(Duration::from_millis(500)).await;
+            }
+            Err(err) => {
+                break Err(err);
+            }
+        }
+    }
+    .map_err(|e| SoarError::Custom(format!("Failed to acquire package lock: {}", e)))?;
+
+    debug!(
+        "acquired lock for '{}' after {} attempts",
+        target.package.pkg_name, lock_attempts
+    );
+
+    // Re-check if package is already installed after acquiring lock
+    let freshly_installed: Option<InstalledPackageWithPortable> = core_db
+        .with_conn(|conn| {
+            CoreRepository::list_filtered(
+                conn,
+                Some(&target.package.repo_name),
+                Some(&target.package.pkg_name),
+                Some(&target.package.pkg_id),
+                Some(&target.package.version),
+                Some(true),
+                None,
+                None,
+                Some(SortDirection::Asc),
+            )
+        })?
+        .into_iter()
+        .find(|ip| ip.is_installed);
+
+    if let Some(ref pkg) = freshly_installed {
+        info!(
+            "{}#{}:{} ({}) is already installed - skipping",
+            pkg.pkg_name, pkg.pkg_id, pkg.repo_name, pkg.version
+        );
+        return Ok((PathBuf::new(), Vec::new()));
+    }
+
     let bin_dir = get_config().get_bin_path()?;
 
     let dir_suffix: String = target

crates/soar-utils/src/error.rs

@@ -31,6 +31,24 @@ pub enum HashError {
     },
 }
 
+/// Errors that can occur when working with locks.
+#[derive(Debug, Diagnostic, Error)]
+pub enum LockError {
+    #[error(transparent)]
+    #[diagnostic(
+        code(soar_utils::lock::io),
+        help("Check if you have write permissions to the lock directory")
+    )]
+    Io(#[from] std::io::Error),
+
+    #[error("Failed to acquire lock for '{0}'")]
+    #[diagnostic(
+        code(soar_utils::lock::acquire_failed),
+        help("Check if the lock directory exists and you have write permissions")
+    )]
+    AcquireFailed(String),
+}
+
 /// Error type for path operations.
 #[derive(Error, Diagnostic, Debug)]
 pub enum PathError {
@@ -360,6 +378,10 @@ pub enum UtilsError {
     #[diagnostic(transparent)]
     Bytes(#[from] BytesError),
 
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    Lock(#[from] LockError),
+
     #[error(transparent)]
     #[diagnostic(transparent)]
     Path(#[from] PathError),
@@ -376,6 +398,7 @@ pub enum UtilsError {
 pub type BytesResult<T> = std::result::Result<T, BytesError>;
 pub type FileSystemResult<T> = std::result::Result<T, FileSystemError>;
 pub type HashResult<T> = std::result::Result<T, HashError>;
+pub type LockResult<T> = std::result::Result<T, LockError>;
 pub type PathResult<T> = std::result::Result<T, PathError>;
 pub type UtilsResult<T> = std::result::Result<T, UtilsError>;
 

crates/soar-utils/src/fs.rs

@@ -40,7 +40,7 @@ pub fn safe_remove<P: AsRef<Path>>(path: P) -> FileSystemResult<()> {
             return Err(FileSystemError::RemoveFile {
                 path: path.to_path_buf(),
                 source: e,
-            })
+            });
         }
     };
 

crates/soar-utils/src/lib.rs

@@ -2,6 +2,7 @@ pub mod bytes;
 pub mod error;
 pub mod fs;
 pub mod hash;
+pub mod lock;
 pub mod path;
 pub mod pattern;
 pub mod system;

crates/soar-utils/src/lock.rs

@@ -0,0 +1,193 @@
+//! File-based locking mechanism for preventing concurrent operations.
+//!
+//! This module provides a simple file-based lock using a `.lock` file to ensure
+//! that only one process can operate on a specific resource at a time.
+
+use std::{
+    fs::{self, File, OpenOptions},
+    path::{Path, PathBuf},
+};
+
+use crate::error::{LockError, LockResult};
+
+/// A file-based lock using `flock`.
+///
+/// The lock is automatically released when `FileLock` is dropped.
+pub struct FileLock {
+    _file: nix::fcntl::Flock<File>,
+    path: PathBuf,
+}
+
+impl FileLock {
+    /// Get the default lock directory for soar.
+    ///
+    /// Uses `$XDG_RUNTIME_DIR/soar/locks` or falls back to `/tmp/soar-locks`.
+    fn lock_dir() -> LockResult<PathBuf> {
+        let xdg_runtime = std::env::var("XDG_RUNTIME_DIR").ok();
+        let base = if let Some(ref runtime) = xdg_runtime {
+            PathBuf::from(runtime)
+        } else {
+            std::env::temp_dir()
+        };
+
+        let lock_dir = base.join("soar").join("locks");
+
+        if !lock_dir.exists() {
+            fs::create_dir_all(&lock_dir)?;
+        }
+
+        Ok(lock_dir)
+    }
+
+    /// Generate a lock file path for a package.
+    fn lock_path(name: &str) -> LockResult<PathBuf> {
+        let lock_dir = Self::lock_dir()?;
+
+        // Sanitize the package name to ensure a valid filename
+        let sanitize = |s: &str| {
+            s.chars()
+                .map(|c| {
+                    if c.is_alphanumeric() || c == '-' || c == '_' || c == '.' {
+                        c
+                    } else {
+                        '_'
+                    }
+                })
+                .collect::<String>()
+        };
+
+        let filename = format!("{}.lock", sanitize(name));
+        Ok(lock_dir.join(filename))
+    }
+
+    /// Acquire an exclusive lock on a package.
+    ///
+    /// This will block until the lock can be acquired.
+    ///
+    /// # Arguments
+    ///
+    /// * `name` - Package name
+    ///
+    /// # Returns
+    ///
+    /// Returns a `FileLock` that will automatically release the lock when dropped.
+    pub fn acquire(name: &str) -> LockResult<Self> {
+        let lock_path = Self::lock_path(name)?;
+
+        let file = OpenOptions::new()
+            .write(true)
+            .create(true)
+            .truncate(false)
+            .open(&lock_path)?;
+
+        let file = nix::fcntl::Flock::lock(file, nix::fcntl::FlockArg::LockExclusive).map_err(
+            |(_, err)| LockError::AcquireFailed(format!("{}: {}", lock_path.display(), err)),
+        )?;
+
+        Ok(FileLock {
+            path: lock_path,
+            _file: file,
+        })
+    }
+
+    /// Try to acquire an exclusive lock without blocking.
+    ///
+    /// Returns `None` if the lock is already held by another process.
+    ///
+    /// # Arguments
+    ///
+    /// * `name` - Package name
+    pub fn try_acquire(name: &str) -> LockResult<Option<Self>> {
+        let lock_path = Self::lock_path(name)?;
+
+        let file = OpenOptions::new()
+            .write(true)
+            .create(true)
+            .truncate(false)
+            .open(&lock_path)?;
+
+        match nix::fcntl::Flock::lock(file, nix::fcntl::FlockArg::LockExclusiveNonblock) {
+            Ok(file) => {
+                Ok(Some(FileLock {
+                    path: lock_path,
+                    _file: file,
+                }))
+            }
+            Err((_, err)) => {
+                if matches!(err, nix::errno::Errno::EWOULDBLOCK) {
+                    return Ok(None);
+                }
+                Err(LockError::AcquireFailed(format!(
+                    "{}: {}",
+                    lock_path.display(),
+                    err
+                )))
+            }
+        }
+    }
+
+    /// Get the path to the lock file.
+    pub fn path(&self) -> &Path {
+        &self.path
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::{thread, time::Duration};
+
+    use super::*;
+
+    #[test]
+    fn test_lock_path_generation() {
+        let path = FileLock::lock_path("test-pkg").unwrap();
+        assert!(path.to_string_lossy().ends_with("test-pkg.lock"));
+    }
+
+    #[test]
+    fn test_lock_sanitization() {
+        let path = FileLock::lock_path("test/pkg").unwrap();
+        assert!(path.to_string_lossy().contains("test_pkg"));
+    }
+
+    #[test]
+    fn test_exclusive_lock() {
+        let lock1 = FileLock::acquire("test-exclusive").unwrap();
+
+        let lock2 = FileLock::try_acquire("test-exclusive").unwrap();
+        assert!(lock2.is_none(), "Should not be able to acquire lock");
+
+        drop(lock1);
+
+        let lock3 = FileLock::try_acquire("test-exclusive").unwrap();
+        assert!(
+            lock3.is_some(),
+            "Should be able to acquire lock after release"
+        );
+    }
+
+    #[test]
+    fn test_concurrent_locks_different_packages() {
+        let lock1 = FileLock::acquire("pkg-a").unwrap();
+        let lock2 = FileLock::acquire("pkg-b").unwrap();
+
+        assert!(lock1.path() != lock2.path());
+    }
+
+    #[test]
+    fn test_lock_blocks_until_released() {
+        let lock1 = FileLock::acquire("test-block").unwrap();
+        let path = lock1.path().to_path_buf();
+
+        let handle = thread::spawn(move || {
+            let lock2 = FileLock::acquire("test-block").unwrap();
+            assert_eq!(lock2.path(), &path);
+        });
+
+        thread::sleep(Duration::from_millis(100));
+
+        drop(lock1);
+
+        handle.join().unwrap();
+    }
+}