Merge pull request #81 from rage/lock-fix

Lock fix

Author: Heliozoa

tmc-langs-framework/Cargo.toml

@@ -23,6 +23,9 @@ tempfile = "3"
 anyhow = "1"
 fd-lock = "2"
 
+[target.'cfg(windows)'.dependencies]
+winapi = "0.3"
+
 [dev-dependencies]
 tempfile = "3"
 mockall = "0.9"

tmc-langs-framework/src/error.rs

@@ -138,10 +138,15 @@ pub enum FileIo {
     NoFileName(PathBuf),
     #[error("Expected {0} to be a directory, but it was a file")]
     UnexpectedFile(PathBuf),
+    #[error("Expected {0} to be a file")]
+    UnexpectedNonFile(PathBuf),
 
     #[error("Directory walk error")]
     Walkdir(#[from] walkdir::Error),
 
+    #[error("Failed to lock {0}: not a file or directory")]
+    InvalidLockPath(PathBuf),
+
     // when there is no meaningful data that can be added to an error
     #[error("transparent")]
     Generic(#[from] std::io::Error),

tmc-langs-framework/src/file_util.rs

@@ -1,15 +1,21 @@
 //! Various utility functions, primarily wrapping the standard library's IO and filesystem functions
 
 use crate::error::FileIo;
-use fd_lock::{FdLock, FdLockGuard};
+use std::fs::{self, File};
 use std::io::{Read, Write};
 use std::path::Path;
-use std::{
-    fs::{self, File},
-    path::PathBuf,
-};
 use walkdir::WalkDir;
 
+#[cfg(unix)]
+mod lock_unix;
+#[cfg(unix)]
+pub use lock_unix::*;
+
+#[cfg(windows)]
+mod lock_windows;
+#[cfg(windows)]
+pub use lock_windows::*;
+
 pub fn temp_file() -> Result<File, FileIo> {
     tempfile::tempfile().map_err(FileIo::TempFile)
 }
@@ -167,64 +173,6 @@ pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(source: P, target: Q) -> Result<(),
     Ok(())
 }
 
-#[macro_export]
-macro_rules! lock {
-    ( $( $path: expr ),+ ) => {
-        $(
-            let path_buf: PathBuf = $path.into();
-            let mut fl = $crate::file_util::FileLock::new(path_buf)?;
-            let _lock = fl.lock()?;
-        )*
-    };
-}
-// macros always live at the top-level, re-export here
-pub use crate::lock;
-
-/// Wrapper for fd_lock::FdLock. Used to lock files/directories to prevent concurrent access
-/// from multiple instances of tmc-langs.
-// TODO: should this be in file_util or in the frontend (CLI)?
-pub struct FileLock {
-    path: PathBuf,
-    fd_lock: FdLock<File>,
-}
-
-impl FileLock {
-    pub fn new(path: PathBuf) -> Result<FileLock, FileIo> {
-        let file = open_file(&path)?;
-        Ok(Self {
-            path,
-            fd_lock: FdLock::new(file),
-        })
-    }
-
-    /// Blocks until the lock can be acquired.
-    pub fn lock(&mut self) -> Result<FileLockGuard, FileIo> {
-        log::debug!("locking {}", self.path.display());
-        let path = &self.path;
-        let fd_lock = &mut self.fd_lock;
-        let guard = fd_lock
-            .lock()
-            .map_err(|e| FileIo::FdLock(path.clone(), e))?;
-        log::debug!("locked {}", self.path.display());
-        Ok(FileLockGuard {
-            path,
-            _guard: guard,
-        })
-    }
-}
-
-#[derive(Debug)]
-pub struct FileLockGuard<'a> {
-    path: &'a Path,
-    _guard: FdLockGuard<'a, File>,
-}
-
-impl Drop for FileLockGuard<'_> {
-    fn drop(&mut self) {
-        log::debug!("unlocking {}", self.path.display());
-    }
-}
-
 #[cfg(test)]
 mod test {
     use super::*;

tmc-langs-framework/src/file_util/lock_unix.rs

@@ -0,0 +1,146 @@
+//! File locking utilities on Unix platforms.
+
+use crate::error::FileIo;
+use crate::file_util::*;
+use fd_lock::{FdLock, FdLockGuard};
+use std::fs::File;
+use std::path::PathBuf;
+
+#[macro_export]
+macro_rules! lock {
+    ( $( $path: expr ),+ ) => {
+        $(
+            let path_buf: PathBuf = $path.into();
+            let mut fl = $crate::file_util::FileLock::new(path_buf)?;
+            let _lock = fl.lock()?;
+        )*
+    };
+}
+// macros always live at the top-level, re-export here
+pub use crate::lock;
+
+/// Wrapper for fd_lock::FdLock. Used to lock files/directories to prevent concurrent access
+/// from multiple instances of tmc-langs.
+// TODO: should this be in file_util or in the frontend (CLI)?
+pub struct FileLock {
+    path: PathBuf,
+    fd_lock: FdLock<File>,
+}
+
+impl FileLock {
+    pub fn new(path: PathBuf) -> Result<FileLock, FileIo> {
+        let file = open_file(&path)?;
+        Ok(Self {
+            path,
+            fd_lock: FdLock::new(file),
+        })
+    }
+
+    /// Blocks until the lock can be acquired.
+    pub fn lock(&mut self) -> Result<FileLockGuard, FileIo> {
+        log::debug!("locking {}", self.path.display());
+        let path = &self.path;
+        let fd_lock = &mut self.fd_lock;
+        let guard = fd_lock
+            .lock()
+            .map_err(|e| FileIo::FdLock(path.clone(), e))?;
+        log::debug!("locked {}", self.path.display());
+        Ok(FileLockGuard {
+            path,
+            _guard: guard,
+        })
+    }
+}
+
+#[derive(Debug)]
+pub struct FileLockGuard<'a> {
+    path: &'a Path,
+    _guard: FdLockGuard<'a, File>,
+}
+
+impl Drop for FileLockGuard<'_> {
+    fn drop(&mut self) {
+        log::debug!("unlocking {}", self.path.display());
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use std::sync::Arc;
+    use std::sync::Mutex;
+    use tempfile::NamedTempFile;
+
+    fn init() {
+        use log::*;
+        use simple_logger::*;
+        let _ = SimpleLogger::new().with_level(LevelFilter::Debug).init();
+    }
+
+    #[test]
+    fn locks_file() {
+        init();
+
+        let temp = NamedTempFile::new().unwrap();
+        let temp_path = temp.path();
+        let mut lock = FileLock::new(temp_path.to_path_buf()).unwrap();
+        let mutex = Arc::new(Mutex::new(vec![]));
+
+        // take file lock and then refcell
+        let guard = lock.lock().unwrap();
+        let mut mguard = mutex.try_lock().unwrap();
+
+        let handle = {
+            let temp_path = temp_path.to_path_buf();
+            let mutex = mutex.clone();
+
+            std::thread::spawn(move || {
+                let mut lock = FileLock::new(temp_path).unwrap();
+                let _guard = lock.lock().unwrap();
+                mutex.try_lock().unwrap().push(1);
+            })
+        };
+
+        std::thread::sleep(std::time::Duration::from_millis(100));
+        mguard.push(1);
+
+        drop(mguard);
+        drop(guard);
+        handle.join().unwrap();
+    }
+
+    #[test]
+    fn locks_dir() {
+        init();
+
+        let temp = tempfile::tempdir().unwrap();
+        let temp_path = temp.path();
+        let mut lock = FileLock::new(temp_path.to_path_buf()).unwrap();
+        let mutex = Arc::new(Mutex::new(vec![]));
+
+        // take file lock and then refcell
+        let guard = lock.lock().unwrap();
+        let mut mguard = mutex.try_lock().unwrap();
+
+        let handle = {
+            let temp_path = temp_path.to_path_buf();
+            let refcell = mutex.clone();
+
+            std::thread::spawn(move || {
+                let mut lock = FileLock::new(temp_path).unwrap();
+                // block on file lock and use refcell
+                let _guard = lock.lock().unwrap();
+                refcell.try_lock().unwrap().push(1);
+            })
+        };
+
+        // wait for the other thread to actually lock
+        std::thread::sleep(std::time::Duration::from_millis(100));
+        mguard.push(1);
+
+        // drop mutex guard then file lock
+        drop(mguard);
+        drop(guard);
+        handle.join().unwrap();
+    }
+}