Change signature of File::try_lock and File::try_lock_shared

These methods now return Result<(), TryLockError> instead of
Result<bool, Error> to make their use less errorprone

Author: cberner

std/src/fs.rs

@@ -116,6 +116,22 @@ pub struct File {
     inner: fs_imp::File,
 }
 
+/// An enumeration of possible errors which can occur while trying to acquire a lock
+/// from the [`try_lock`] method and [`try_lock_shared`] method on a [`File`].
+///
+/// [`try_lock`]: File::try_lock
+/// [`try_lock_shared`]: File::try_lock_shared
+#[unstable(feature = "file_lock", issue = "130994")]
+pub enum TryLockError {
+    /// The lock could not be acquired due to an I/O error on the file. The standard library will
+    /// not return an [`ErrorKind::WouldBlock`] error inside [`TryLockError::Error`]
+    ///
+    /// [`ErrorKind::WouldBlock`]: io::ErrorKind::WouldBlock
+    Error(io::Error),
+    /// The lock could not be acquired at this time because it is held by another handle/process.
+    WouldBlock,
+}
+
 /// Metadata information about a file.
 ///
 /// This structure is returned from the [`metadata`] or
@@ -352,6 +368,27 @@ pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> io::Result
     inner(path.as_ref(), contents.as_ref())
 }
 
+#[unstable(feature = "file_lock", issue = "130994")]
+impl fmt::Debug for TryLockError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            TryLockError::Error(err) => err.fmt(f),
+            TryLockError::WouldBlock => "WouldBlock".fmt(f),
+        }
+    }
+}
+
+#[unstable(feature = "file_lock", issue = "130994")]
+impl fmt::Display for TryLockError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            TryLockError::Error(_) => "lock acquisition failed due to I/O error",
+            TryLockError::WouldBlock => "lock acquisition failed because the operation would block",
+        }
+        .fmt(f)
+    }
+}
+
 impl File {
     /// Attempts to open a file in read-only mode.
     ///
@@ -734,8 +771,8 @@ impl File {
 
     /// Try to acquire an exclusive lock on the file.
     ///
-    /// Returns `Ok(false)` if a different lock is already held on this file (via another
-    /// handle/descriptor).
+    /// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
+    /// (via another handle/descriptor).
     ///
     /// This acquires an exclusive lock; no other file handle to this file may acquire another lock.
     ///
@@ -777,23 +814,27 @@ impl File {
     ///
     /// ```no_run
     /// #![feature(file_lock)]
-    /// use std::fs::File;
+    /// use std::fs::{File, TryLockError};
     ///
     /// fn main() -> std::io::Result<()> {
     ///     let f = File::create("foo.txt")?;
-    ///     f.try_lock()?;
+    ///     match f.try_lock() {
+    ///         Ok(_) => (),
+    ///         Err(TryLockError::WouldBlock) => (), // Lock not acquired
+    ///         Err(TryLockError::Error(err)) => return Err(err),
+    ///     }
     ///     Ok(())
     /// }
     /// ```
     #[unstable(feature = "file_lock", issue = "130994")]
-    pub fn try_lock(&self) -> io::Result<bool> {
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
         self.inner.try_lock()
     }
 
     /// Try to acquire a shared (non-exclusive) lock on the file.
     ///
-    /// Returns `Ok(false)` if an exclusive lock is already held on this file (via another
-    /// handle/descriptor).
+    /// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
+    /// (via another handle/descriptor).
     ///
     /// This acquires a shared lock; more than one file handle may hold a shared lock, but none may
     /// hold an exclusive lock at the same time.
@@ -834,16 +875,21 @@ impl File {
     ///
     /// ```no_run
     /// #![feature(file_lock)]
-    /// use std::fs::File;
+    /// use std::fs::{File, TryLockError};
     ///
     /// fn main() -> std::io::Result<()> {
     ///     let f = File::open("foo.txt")?;
-    ///     f.try_lock_shared()?;
+    ///     match f.try_lock_shared() {
+    ///         Ok(_) => (),
+    ///         Err(TryLockError::WouldBlock) => (), // Lock not acquired
+    ///         Err(TryLockError::Error(err)) => return Err(err),
+    ///     }
+    ///
     ///     Ok(())
     /// }
     /// ```
     #[unstable(feature = "file_lock", issue = "130994")]
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
         self.inner.try_lock_shared()
     }
 

std/src/fs/tests.rs

@@ -1,6 +1,22 @@
 use rand::RngCore;
 
+#[cfg(any(
+    windows,
+    target_os = "freebsd",
+    target_os = "linux",
+    target_os = "netbsd",
+    target_vendor = "apple",
+))]
+use crate::assert_matches::assert_matches;
 use crate::char::MAX_LEN_UTF8;
+#[cfg(any(
+    windows,
+    target_os = "freebsd",
+    target_os = "linux",
+    target_os = "netbsd",
+    target_vendor = "apple",
+))]
+use crate::fs::TryLockError;
 use crate::fs::{self, File, FileTimes, OpenOptions};
 use crate::io::prelude::*;
 use crate::io::{BorrowedBuf, ErrorKind, SeekFrom};
@@ -223,8 +239,8 @@ fn file_lock_multiple_shared() {
     check!(f2.lock_shared());
     check!(f1.unlock());
     check!(f2.unlock());
-    assert!(check!(f1.try_lock_shared()));
-    assert!(check!(f2.try_lock_shared()));
+    check!(f1.try_lock_shared());
+    check!(f2.try_lock_shared());
 }
 
 #[test]
@@ -243,12 +259,12 @@ fn file_lock_blocking() {
 
     // Check that shared locks block exclusive locks
     check!(f1.lock_shared());
-    assert!(!check!(f2.try_lock()));
+    assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
     check!(f1.unlock());
 
     // Check that exclusive locks block shared locks
     check!(f1.lock());
-    assert!(!check!(f2.try_lock_shared()));
+    assert_matches!(f2.try_lock_shared(), Err(TryLockError::WouldBlock));
 }
 
 #[test]
@@ -267,9 +283,9 @@ fn file_lock_drop() {
 
     // Check that locks are released when the File is dropped
     check!(f1.lock_shared());
-    assert!(!check!(f2.try_lock()));
+    assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
     drop(f1);
-    assert!(check!(f2.try_lock()));
+    check!(f2.try_lock());
 }
 
 #[test]
@@ -288,10 +304,10 @@ fn file_lock_dup() {
 
     // Check that locks are not dropped if the File has been cloned
     check!(f1.lock_shared());
-    assert!(!check!(f2.try_lock()));
+    assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
     let cloned = check!(f1.try_clone());
     drop(f1);
-    assert!(!check!(f2.try_lock()));
+    assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
     drop(cloned)
 }
 
@@ -307,9 +323,9 @@ fn file_lock_double_unlock() {
     // Check that both are released by unlock()
     check!(f1.lock());
     check!(f1.lock_shared());
-    assert!(!check!(f2.try_lock()));
+    assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
     check!(f1.unlock());
-    assert!(check!(f2.try_lock()));
+    check!(f2.try_lock());
 }
 
 #[test]

std/src/sys/fs/hermit.rs

@@ -1,4 +1,5 @@
 use crate::ffi::{CStr, OsStr, OsString, c_char};
+use crate::fs::TryLockError;
 use crate::io::{self, BorrowedCursor, Error, ErrorKind, IoSlice, IoSliceMut, SeekFrom};
 use crate::os::hermit::ffi::OsStringExt;
 use crate::os::hermit::hermit_abi::{
@@ -12,7 +13,7 @@ use crate::sys::common::small_c_string::run_path_with_cstr;
 pub use crate::sys::fs::common::{copy, exists};
 use crate::sys::pal::fd::FileDesc;
 use crate::sys::time::SystemTime;
-use crate::sys::{cvt, unsupported};
+use crate::sys::{cvt, unsupported, unsupported_err};
 use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
 use crate::{fmt, mem};
 
@@ -366,12 +367,12 @@ impl File {
         unsupported()
     }
 
-    pub fn try_lock(&self) -> io::Result<bool> {
-        unsupported()
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
+        Err(TryLockError::Error(unsupported_err()))
     }
 
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
-        unsupported()
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
+        Err(TryLockError::Error(unsupported_err()))
     }
 
     pub fn unlock(&self) -> io::Result<()> {

std/src/sys/fs/solid.rs

@@ -2,6 +2,7 @@
 
 use crate::ffi::{CStr, CString, OsStr, OsString};
 use crate::fmt;
+use crate::fs::TryLockError;
 use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom};
 use crate::mem::MaybeUninit;
 use crate::os::raw::{c_int, c_short};
@@ -11,7 +12,7 @@ use crate::sync::Arc;
 pub use crate::sys::fs::common::exists;
 use crate::sys::pal::{abi, error};
 use crate::sys::time::SystemTime;
-use crate::sys::unsupported;
+use crate::sys::{unsupported, unsupported_err};
 use crate::sys_common::ignore_notfound;
 
 type CIntNotMinusOne = core::num::niche_types::NotAllOnes<c_int>;
@@ -352,12 +353,12 @@ impl File {
         unsupported()
     }
 
-    pub fn try_lock(&self) -> io::Result<bool> {
-        unsupported()
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
+        Err(TryLockError::Error(unsupported_err()))
     }
 
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
-        unsupported()
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
+        Err(TryLockError::Error(unsupported_err()))
     }
 
     pub fn unlock(&self) -> io::Result<()> {

std/src/sys/fs/uefi.rs

@@ -2,6 +2,7 @@ use r_efi::protocols::file;
 
 use crate::ffi::OsString;
 use crate::fmt;
+use crate::fs::TryLockError;
 use crate::hash::Hash;
 use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom};
 use crate::path::{Path, PathBuf};
@@ -227,11 +228,11 @@ impl File {
         self.0
     }
 
-    pub fn try_lock(&self) -> io::Result<bool> {
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
         self.0
     }
 
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
         self.0
     }
 

std/src/sys/fs/unix.rs

@@ -74,6 +74,7 @@ use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, st
 
 use crate::ffi::{CStr, OsStr, OsString};
 use crate::fmt::{self, Write as _};
+use crate::fs::TryLockError;
 use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom};
 use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd};
 use crate::os::unix::prelude::*;
@@ -1307,15 +1308,17 @@ impl File {
         target_os = "netbsd",
         target_vendor = "apple",
     ))]
-    pub fn try_lock(&self) -> io::Result<bool> {
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
         let result = cvt(unsafe { libc::flock(self.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) });
-        if let Err(ref err) = result {
+        if let Err(err) = result {
             if err.kind() == io::ErrorKind::WouldBlock {
-                return Ok(false);
+                Err(TryLockError::WouldBlock)
+            } else {
+                Err(TryLockError::Error(err))
             }
+        } else {
+            Ok(())
         }
-        result?;
-        return Ok(true);
     }
 
     #[cfg(not(any(
@@ -1325,8 +1328,11 @@ impl File {
         target_os = "netbsd",
         target_vendor = "apple",
     )))]
-    pub fn try_lock(&self) -> io::Result<bool> {
-        Err(io::const_error!(io::ErrorKind::Unsupported, "try_lock() not supported"))
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
+        Err(TryLockError::Error(io::const_error!(
+            io::ErrorKind::Unsupported,
+            "try_lock() not supported"
+        )))
     }
 
     #[cfg(any(
@@ -1336,15 +1342,17 @@ impl File {
         target_os = "netbsd",
         target_vendor = "apple",
     ))]
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
         let result = cvt(unsafe { libc::flock(self.as_raw_fd(), libc::LOCK_SH | libc::LOCK_NB) });
-        if let Err(ref err) = result {
+        if let Err(err) = result {
             if err.kind() == io::ErrorKind::WouldBlock {
-                return Ok(false);
+                Err(TryLockError::WouldBlock)
+            } else {
+                Err(TryLockError::Error(err))
             }
+        } else {
+            Ok(())
         }
-        result?;
-        return Ok(true);
     }
 
     #[cfg(not(any(
@@ -1354,8 +1362,11 @@ impl File {
         target_os = "netbsd",
         target_vendor = "apple",
     )))]
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
-        Err(io::const_error!(io::ErrorKind::Unsupported, "try_lock_shared() not supported"))
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
+        Err(TryLockError::Error(io::const_error!(
+            io::ErrorKind::Unsupported,
+            "try_lock_shared() not supported"
+        )))
     }
 
     #[cfg(any(

std/src/sys/fs/unsupported.rs

@@ -1,5 +1,6 @@
 use crate::ffi::OsString;
 use crate::fmt;
+use crate::fs::TryLockError;
 use crate::hash::{Hash, Hasher};
 use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom};
 use crate::path::{Path, PathBuf};
@@ -206,11 +207,11 @@ impl File {
         self.0
     }
 
-    pub fn try_lock(&self) -> io::Result<bool> {
+    pub fn try_lock(&self) -> Result<(), TryLockError> {
         self.0
     }
 
-    pub fn try_lock_shared(&self) -> io::Result<bool> {
+    pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
         self.0
     }