drop memory mutex, and use associated type instead of enum polymorphy

database/Cargo.toml

@@ -14,7 +14,6 @@ serde_derive = "1.0"
 serde_json = "1.0"
 time = "0.1"
 tempfile = "3.0"
-parking_lot = "0.7"
 url = "1.6"
 hex = "0.3"
 base64 = "0.10"

database/src/fs.rs

@@ -12,7 +12,7 @@ use pathdiff::diff_paths;
 
 use {Database, Query};
 use types::{Email, Fingerprint, KeyID};
-use sync::{MutexGuard, FlockMutex};
+use sync::FlockMutexGuard;
 use Result;
 
 use tempfile::NamedTempFile;
@@ -20,8 +20,6 @@ use tempfile::NamedTempFile;
 use openpgp::TPK;
 
 pub struct Filesystem {
-    update_lock: FlockMutex,
-
     tmp_dir: PathBuf,
 
     keys_internal_dir: PathBuf,
@@ -111,7 +109,6 @@ impl Filesystem {
         info!("keys_external_dir: '{}'", keys_external_dir.display());
         info!("tmp_dir: '{}'", tmp_dir.display());
         Ok(Filesystem {
-            update_lock: FlockMutex::new(&keys_internal_dir)?,
             keys_internal_dir,
             keys_external_dir,
             tmp_dir,
@@ -358,8 +355,10 @@ fn symlink(symlink_content: &Path, symlink_name: &Path) -> Result<()> {
 }
 
 impl Database for Filesystem {
-    fn lock(&self) -> MutexGuard<()> {
+    type MutexGuard = FlockMutexGuard;
-        self.update_lock.lock().into()
+
+    fn lock(&self) -> Result<Self::MutexGuard> {
+        FlockMutexGuard::lock(&self.keys_internal_dir)
     }
 
     fn write_to_temp(&self, content: &[u8]) -> Result<NamedTempFile> {

database/src/lib.rs

@@ -12,7 +12,6 @@ use failure::Fallible as Result;
 extern crate fs2;
 extern crate idna;
 #[macro_use] extern crate log;
-extern crate parking_lot;
 extern crate pathdiff;
 extern crate rand;
 extern crate serde;
@@ -39,7 +38,6 @@ pub mod types;
 use types::{Email, Fingerprint, KeyID};
 
 pub mod sync;
-use sync::MutexGuard;
 
 mod fs;
 pub use self::fs::Filesystem as KeyDatabase;
@@ -77,11 +75,13 @@ impl FromStr for Query {
 }
 
 pub trait Database: Sync + Send {
+    type MutexGuard;
+
     /// Lock the DB for a complex update.
     ///
     /// All basic write operations are atomic so we don't need to lock
     /// read operations to ensure that we return something sane.
-    fn lock(&self) -> MutexGuard<()>;
+    fn lock(&self) -> Result<Self::MutexGuard>;
 
     /// Queries the database using Fingerprint, KeyID, or
     /// email-address.

database/src/sync.rs

@@ -3,41 +3,20 @@ use std::io;
 use std::path::Path;
 
 use fs2::FileExt;
-use parking_lot;
 
-pub enum MutexGuard<'a, T> {
+use Result;
-    ParkingLot(parking_lot::MutexGuard<'a, T>),
-    Flock(FlockMutexGuard<'a>),
-}
-
-impl<'a, T> From<parking_lot::MutexGuard<'a, T>> for MutexGuard<'a, T> {
-    fn from(g: parking_lot::MutexGuard<'a, T>) -> Self {
-        MutexGuard::ParkingLot(g)
-    }
-}
-
-impl<'a> From<FlockMutexGuard<'a>> for MutexGuard<'a, ()> {
-    fn from(g: FlockMutexGuard<'a>) -> Self {
-        MutexGuard::Flock(g)
-    }
-}
 
 /// A minimalistic flock-based mutex.
 ///
 /// This just barely implements enough what we need from a mutex.
-pub struct FlockMutex {
+pub struct FlockMutexGuard {
-    f: File,
+    file: File,
 }
 
-impl FlockMutex {
+impl FlockMutexGuard {
-    pub fn new<P: AsRef<Path>>(p: P) -> io::Result<Self> {
+    pub fn lock(path: impl AsRef<Path>) -> Result<Self> {
-        Ok(Self {
+        let file = File::open(path)?;
-            f: File::open(p)?
+        while let Err(e) = file.lock_exclusive() {
-        })
-    }
-
-    pub fn lock(&self) -> FlockMutexGuard {
-        while let Err(e) = self.f.lock_exclusive() {
             // According to flock(2), possible errors returned are:
             //
             //   EBADF  fd is not an open file descriptor.
@@ -64,22 +43,61 @@ impl FlockMutex {
             // by retrying.
             assert_eq!(e.kind(), io::ErrorKind::Interrupted);
         }
-
+        Ok(Self { file })
-        FlockMutexGuard {
-            m: &self,
-        }
     }
 }
 
-pub struct FlockMutexGuard<'a> {
+impl Drop for FlockMutexGuard {
-    m: &'a FlockMutex,
-}
-
-impl<'a> Drop for FlockMutexGuard<'a> {
     fn drop(&mut self) {
-        while let Err(e) = self.m.f.unlock() {
+        while let Err(e) = self.file.unlock() {
             // See above.
             assert_eq!(e.kind(), io::ErrorKind::Interrupted);
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use tempfile::{NamedTempFile, TempDir};
+
+    #[test]
+    fn flock_dir() {
+        let tempdir = TempDir::new().unwrap();
+        let file = tempdir.path();
+
+        assert!(File::open(file).unwrap().try_lock_exclusive().is_ok());
+        let _lock = FlockMutexGuard::lock(file).unwrap();
+        assert!(File::open(file).unwrap().try_lock_exclusive().is_err());
+        assert!(File::open(file).unwrap().try_lock_shared().is_err());
+    }
+
+    #[test]
+    fn flock_file() {
+        let tempfile = NamedTempFile::new().unwrap();
+        let file = tempfile.path();
+
+        assert!(File::open(file).unwrap().try_lock_exclusive().is_ok());
+        let _lock = FlockMutexGuard::lock(file).unwrap();
+        assert!(File::open(file).unwrap().try_lock_exclusive().is_err());
+        assert!(File::open(file).unwrap().try_lock_shared().is_err());
+    }
+
+    #[test]
+    fn flock_drop() {
+        let tempfile = NamedTempFile::new().unwrap();
+        let file = tempfile.path();
+
+        assert!(File::open(file).unwrap().try_lock_exclusive().is_ok());
+        {
+            let _lock = FlockMutexGuard::lock(file).unwrap();
+            assert!(File::open(file).unwrap().try_lock_exclusive().is_err());
+        }
+        assert!(File::open(file).unwrap().try_lock_exclusive().is_ok());
+    }
+
+    #[test]
+    fn flock_nonexistent() {
+        FlockMutexGuard::lock("nonexistent").is_err();
+    }
+}