Added Flock object for automatic unlock on drop.

changelog/2170.added.md

@@ -0,0 +1,2 @@
+Added newtype `Flock` to automatically unlock a held flock upon drop.
+Added `Flockable` trait to represent valid types for `Flock`.

changelog/2170.removed.md

@@ -0,0 +1 @@
+Made `fcntl::flock` private in favor of new Flock wrapper.

src/fcntl.rs

@@ -10,11 +10,17 @@ use libc::{self, c_int, c_uint, size_t, ssize_t};
 ))]
 use std::ffi::CStr;
 use std::ffi::OsString;
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+use std::mem::ManuallyDrop;
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+use std::ops::{Deref, DerefMut};
 #[cfg(not(target_os = "redox"))]
 use std::os::raw;
 use std::os::unix::ffi::OsStringExt;
 use std::os::unix::io::RawFd;
 // For splice and copy_file_range
+#[cfg(all(not(any(target_os = "redox", target_os = "solaris")), unix))]
+use std::os::unix::io::{AsRawFd, OwnedFd};
 #[cfg(any(
     target_os = "netbsd",
     apple_targets,
@@ -27,10 +33,7 @@ use std::path::PathBuf;
     target_os = "freebsd",
     target_os = "linux"
 ))]
-use std::{
-    os::unix::io::{AsFd, AsRawFd},
-    ptr,
-};
+use std::{os::unix::io::AsFd, ptr};
 
 #[cfg(feature = "fs")]
 use crate::{sys::stat::Mode, NixPath, Result};
@@ -596,39 +599,120 @@ pub fn fcntl(fd: RawFd, arg: FcntlArg) -> Result<c_int> {
     Errno::result(res)
 }
 
-// TODO: convert to libc_enum
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 #[non_exhaustive]
 pub enum FlockArg {
     LockShared,
     LockExclusive,
-    Unlock,
     LockSharedNonblock,
     LockExclusiveNonblock,
-    UnlockNonblock,
 }
 
+/// Represents valid types for flock.
+///
+/// # Safety
+/// Types implementing this must be `!Clone`.
 #[cfg(not(any(target_os = "redox", target_os = "solaris")))]
-pub fn flock(fd: RawFd, arg: FlockArg) -> Result<()> {
-    use self::FlockArg::*;
+pub unsafe trait Flockable: AsRawFd {}
 
-    let res = unsafe {
-        match arg {
-            LockShared => libc::flock(fd, libc::LOCK_SH),
-            LockExclusive => libc::flock(fd, libc::LOCK_EX),
-            Unlock => libc::flock(fd, libc::LOCK_UN),
-            LockSharedNonblock => {
-                libc::flock(fd, libc::LOCK_SH | libc::LOCK_NB)
-            }
-            LockExclusiveNonblock => {
-                libc::flock(fd, libc::LOCK_EX | libc::LOCK_NB)
-            }
-            UnlockNonblock => libc::flock(fd, libc::LOCK_UN | libc::LOCK_NB),
+/// Represents an owned flock, which unlocks on drop.
+///
+/// See flock(2) for details on locking semantics.
+// `ManuallyDrop` is necessary to circumvent move out of `Drop` type error.
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+#[derive(Debug)]
+pub struct Flock<T: Flockable>(ManuallyDrop<T>);
+
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+impl<T: Flockable> Drop for Flock<T> {
+    fn drop(&mut self) {
+        // Result is ignored because flock has no documented failure cases.
+        _ = unsafe { libc::flock(self.0.as_raw_fd(), libc::LOCK_UN) };
+    }
+}
+
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+impl<T: Flockable> Deref for Flock<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+impl<T: Flockable> DerefMut for Flock<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+impl<T: Flockable> Flock<T> {
+    /// Obtain a/an flock.
+    ///
+    /// # Example
+    /// ```
+    /// # use std::fs::File;
+    /// # use nix::fcntl::{Flock, FlockArg};
+    /// fn do_stuff(file: File) -> nix::Result<()> {
+    ///     let lock = match Flock::lock(file, FlockArg::LockExclusive) {
+    ///         Ok(l) => l,
+    ///         Err((_, e)) => return Err(e),
+    ///     };
+    ///
+    ///     // Do stuff
+    ///
+    ///     Ok(())
+    /// } // File is unlocked once `lock` goes out of scope.
+    pub fn lock(t: T, args: FlockArg) -> std::result::Result<Self, (T, Errno)> {
+        let flags = match args {
+            FlockArg::LockShared => libc::LOCK_SH,
+            FlockArg::LockExclusive => libc::LOCK_EX,
+            FlockArg::LockSharedNonblock => libc::LOCK_SH | libc::LOCK_NB,
+            FlockArg::LockExclusiveNonblock => libc::LOCK_EX | libc::LOCK_NB,
+        };
+        match unsafe { libc::flock(t.as_raw_fd(), flags) } {
+            0 => Ok(Self(ManuallyDrop::new(t))),
+            e => Err((t, Errno::from_i32(e))),
         }
-    };
+    }
 
-    Errno::result(res).map(drop)
+    /// Remove the lock and return the object wrapped within.
+    ///
+    /// # Example
+    /// ```
+    /// # use std::fs::File;
+    /// # use nix::fcntl::{Flock, FlockArg};
+    /// fn do_stuff(file: File) -> nix::Result<()> {
+    ///     let lock = match Flock::lock(file, FlockArg::LockExclusive) {
+    ///         Ok(l) => l,
+    ///         Err((_,e)) => return Err(e),
+    ///     };
+    ///
+    ///     // Do critical section
+    ///
+    ///     // Unlock
+    ///     let file = lock.unlock();
+    ///
+    ///     // Do anything else
+    ///
+    ///     Ok(())
+    /// }
+    pub fn unlock(mut self) -> T {
+        _ = unsafe { libc::flock(self.0.as_raw_fd(), libc::LOCK_UN) };
+
+        // Safety: `self.0` never used again.
+        unsafe { ManuallyDrop::take(&mut self.0) }
+    }
 }
+
+// Safety: `File` is not [std::clone::Clone].
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+unsafe impl Flockable for std::fs::File {}
+
+// Safety: `OwnedFd` is not [std::clone::Clone].
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+unsafe impl Flockable for OwnedFd {}
 }
 
 #[cfg(any(target_os = "android", target_os = "linux"))]

test/test_fcntl.rs

@@ -630,3 +630,57 @@ fn test_f_kinfo() {
     assert_ne!(res, -1);
     assert_eq!(path, tmp.path());
 }
+
+/// Test `Flock` and associated functions.
+///
+#[cfg(not(any(target_os = "redox", target_os = "solaris")))]
+mod test_flock {
+    use nix::fcntl::*;
+    use tempfile::NamedTempFile;
+
+    /// Verify that `Flock::lock()` correctly obtains a lock, and subsequently unlocks upon drop.
+    #[test]
+    fn verify_lock_and_drop() {
+        // Get 2 `File` handles to same underlying file.
+        let file1 = NamedTempFile::new().unwrap();
+        let file2 = file1.reopen().unwrap();
+        let file1 = file1.into_file();
+
+        // Lock first handle
+        let lock1 = Flock::lock(file1, FlockArg::LockExclusive).unwrap();
+
+        // Attempt to lock second handle
+        let file2 = match Flock::lock(file2, FlockArg::LockExclusiveNonblock) {
+            Ok(_) => panic!("Expected second exclusive lock to fail."),
+            Err((f, _)) => f,
+        };
+
+        // Drop first lock
+        std::mem::drop(lock1);
+
+        // Attempt to lock second handle again (but successfully)
+        if Flock::lock(file2, FlockArg::LockExclusiveNonblock).is_err() {
+            panic!("Expected locking to be successful.");
+        }
+    }
+
+    /// Verify that `Flock::unlock()` correctly obtains unlocks.
+    #[test]
+    fn verify_unlock() {
+        // Get 2 `File` handles to same underlying file.
+        let file1 = NamedTempFile::new().unwrap();
+        let file2 = file1.reopen().unwrap();
+        let file1 = file1.into_file();
+
+        // Lock first handle
+        let lock1 = Flock::lock(file1, FlockArg::LockExclusive).unwrap();
+
+        // Unlock and retain file so any erroneous flocks also remain present.
+        let _file1 = lock1.unlock();
+
+        // Attempt to lock second handle.
+        if Flock::lock(file2, FlockArg::LockExclusiveNonblock).is_err() {
+            panic!("Expected locking to be successful.");
+        }
+    }
+}