Skip to content

Commit 823b39d

Browse files
cbernercberner
committed
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
1 parent 70dab5a commit 823b39d

File tree

9 files changed

+135
-61
lines changed

9 files changed

+135
-61
lines changed

‎library/std/src/fs.rs

+56-9
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@
2121
mod tests;
2222

2323
use crate::ffi::OsString;
24-
use crate::fmt;
2524
use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write};
2625
use crate::path::{Path, PathBuf};
2726
use crate::sealed::Sealed;
2827
use crate::sync::Arc;
2928
use crate::sys::fs as fs_imp;
3029
use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
3130
use crate::time::SystemTime;
31+
use crate::{error, fmt};
3232

3333
/// An object providing access to an open file on the filesystem.
3434
///
@@ -116,6 +116,19 @@ pub struct File {
116116
inner: fs_imp::File,
117117
}
118118

119+
/// An enumeration of possible errors which can occur while trying to acquire a lock
120+
/// from the [`try_lock`] method and [`try_lock_shared`] method on a [`File`].
121+
///
122+
/// [`try_lock`]: File::try_lock
123+
/// [`try_lock_shared`]: File::try_lock_shared
124+
#[unstable(feature = "file_lock", issue = "130994")]
125+
pub enum TryLockError {
126+
/// The lock could not be acquired due to an I/O error on the file.
127+
Error(io::Error),
128+
/// The lock could not be acquired at this time because the operation would block.
129+
WouldBlock,
130+
}
131+
119132
/// Metadata information about a file.
120133
///
121134
/// This structure is returned from the [`metadata`] or
@@ -352,6 +365,40 @@ pub fn write<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> io::Result
352365
inner(path.as_ref(), contents.as_ref())
353366
}
354367

368+
#[unstable(feature = "file_lock", issue = "130994")]
369+
impl fmt::Debug for TryLockError {
370+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
371+
match self {
372+
TryLockError::Error(err) => err.fmt(f),
373+
TryLockError::WouldBlock => "WouldBlock".fmt(f),
374+
}
375+
}
376+
}
377+
378+
#[unstable(feature = "file_lock", issue = "130994")]
379+
impl fmt::Display for TryLockError {
380+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
381+
match self {
382+
TryLockError::Error(_) => "lock acquisition failed due to I/O error",
383+
TryLockError::WouldBlock => "lock acquisition failed because the operation would block",
384+
}
385+
.fmt(f)
386+
}
387+
}
388+
389+
#[unstable(feature = "file_lock", issue = "130994")]
390+
impl error::Error for TryLockError {}
391+
392+
#[unstable(feature = "file_lock", issue = "130994")]
393+
impl From<TryLockError> for io::Error {
394+
fn from(err: TryLockError) -> io::Error {
395+
match err {
396+
TryLockError::Error(err) => err,
397+
TryLockError::WouldBlock => io::ErrorKind::WouldBlock.into(),
398+
}
399+
}
400+
}
401+
355402
impl File {
356403
/// Attempts to open a file in read-only mode.
357404
///
@@ -734,8 +781,8 @@ impl File {
734781

735782
/// Try to acquire an exclusive lock on the file.
736783
///
737-
/// Returns `Ok(false)` if a different lock is already held on this file (via another
738-
/// handle/descriptor).
784+
/// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
785+
/// (via another handle/descriptor).
739786
///
740787
/// This acquires an exclusive lock; no other file handle to this file may acquire another lock.
741788
///
@@ -781,19 +828,19 @@ impl File {
781828
///
782829
/// fn main() -> std::io::Result<()> {
783830
/// let f = File::create("foo.txt")?;
784-
/// f.try_lock()?;
831+
/// f.try_lock()?; // raises WouldBlock if the lock cannot be acquired
785832
/// Ok(())
786833
/// }
787834
/// ```
788835
#[unstable(feature = "file_lock", issue = "130994")]
789-
pub fn try_lock(&self) -> io::Result<bool> {
836+
pub fn try_lock(&self) -> Result<(), TryLockError> {
790837
self.inner.try_lock()
791838
}
792839

793840
/// Try to acquire a shared (non-exclusive) lock on the file.
794841
///
795-
/// Returns `Ok(false)` if an exclusive lock is already held on this file (via another
796-
/// handle/descriptor).
842+
/// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
843+
/// (via another handle/descriptor).
797844
///
798845
/// This acquires a shared lock; more than one file handle may hold a shared lock, but none may
799846
/// hold an exclusive lock at the same time.
@@ -838,12 +885,12 @@ impl File {
838885
///
839886
/// fn main() -> std::io::Result<()> {
840887
/// let f = File::open("foo.txt")?;
841-
/// f.try_lock_shared()?;
888+
/// f.try_lock_shared()?; // raises WouldBlock if the lock cannot be acquired
842889
/// Ok(())
843890
/// }
844891
/// ```
845892
#[unstable(feature = "file_lock", issue = "130994")]
846-
pub fn try_lock_shared(&self) -> io::Result<bool> {
893+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
847894
self.inner.try_lock_shared()
848895
}
849896

‎library/std/src/fs/tests.rs

+21-11
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use rand::RngCore;
22

33
use crate::char::MAX_LEN_UTF8;
4-
use crate::fs::{self, File, FileTimes, OpenOptions};
4+
use crate::fs::{self, File, FileTimes, OpenOptions, TryLockError};
55
use crate::io::prelude::*;
66
use crate::io::{BorrowedBuf, ErrorKind, SeekFrom};
77
use crate::mem::MaybeUninit;
@@ -28,6 +28,16 @@ macro_rules! check {
2828
};
2929
}
3030

31+
macro_rules! check_would_block {
32+
($e:expr) => {
33+
match $e {
34+
Ok(_) => panic!("{} acquired lock when it should have failed", stringify!($e)),
35+
Err(TryLockError::WouldBlock) => (),
36+
Err(e) => panic!("{} failed with: {e}", stringify!($e)),
37+
}
38+
};
39+
}
40+
3141
#[cfg(windows)]
3242
macro_rules! error {
3343
($e:expr, $s:expr) => {
@@ -223,8 +233,8 @@ fn file_lock_multiple_shared() {
223233
check!(f2.lock_shared());
224234
check!(f1.unlock());
225235
check!(f2.unlock());
226-
assert!(check!(f1.try_lock_shared()));
227-
assert!(check!(f2.try_lock_shared()));
236+
check!(f1.try_lock_shared());
237+
check!(f2.try_lock_shared());
228238
}
229239

230240
#[test]
@@ -243,12 +253,12 @@ fn file_lock_blocking() {
243253

244254
// Check that shared locks block exclusive locks
245255
check!(f1.lock_shared());
246-
assert!(!check!(f2.try_lock()));
256+
check_would_block!(f2.try_lock());
247257
check!(f1.unlock());
248258

249259
// Check that exclusive locks block shared locks
250260
check!(f1.lock());
251-
assert!(!check!(f2.try_lock_shared()));
261+
check_would_block!(f2.try_lock_shared());
252262
}
253263

254264
#[test]
@@ -267,9 +277,9 @@ fn file_lock_drop() {
267277

268278
// Check that locks are released when the File is dropped
269279
check!(f1.lock_shared());
270-
assert!(!check!(f2.try_lock()));
280+
check_would_block!(f2.try_lock());
271281
drop(f1);
272-
assert!(check!(f2.try_lock()));
282+
check!(f2.try_lock());
273283
}
274284

275285
#[test]
@@ -288,10 +298,10 @@ fn file_lock_dup() {
288298

289299
// Check that locks are not dropped if the File has been cloned
290300
check!(f1.lock_shared());
291-
assert!(!check!(f2.try_lock()));
301+
check_would_block!(f2.try_lock());
292302
let cloned = check!(f1.try_clone());
293303
drop(f1);
294-
assert!(!check!(f2.try_lock()));
304+
check_would_block!(f2.try_lock());
295305
drop(cloned)
296306
}
297307

@@ -307,9 +317,9 @@ fn file_lock_double_unlock() {
307317
// Check that both are released by unlock()
308318
check!(f1.lock());
309319
check!(f1.lock_shared());
310-
assert!(!check!(f2.try_lock()));
320+
check_would_block!(f2.try_lock());
311321
check!(f1.unlock());
312-
assert!(check!(f2.try_lock()));
322+
check!(f2.try_lock());
313323
}
314324

315325
#[test]

‎library/std/src/sys/fs/hermit.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::ffi::{CStr, OsStr, OsString, c_char};
2+
use crate::fs::TryLockError;
23
use crate::io::{self, BorrowedCursor, Error, ErrorKind, IoSlice, IoSliceMut, SeekFrom};
34
use crate::os::hermit::ffi::OsStringExt;
45
use crate::os::hermit::hermit_abi::{
@@ -12,7 +13,7 @@ use crate::sys::common::small_c_string::run_path_with_cstr;
1213
pub use crate::sys::fs::common::{copy, exists};
1314
use crate::sys::pal::fd::FileDesc;
1415
use crate::sys::time::SystemTime;
15-
use crate::sys::{cvt, unsupported};
16+
use crate::sys::{cvt, unsupported, unsupported_err};
1617
use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
1718
use crate::{fmt, mem};
1819

@@ -366,12 +367,12 @@ impl File {
366367
unsupported()
367368
}
368369

369-
pub fn try_lock(&self) -> io::Result<bool> {
370-
unsupported()
370+
pub fn try_lock(&self) -> Result<(), TryLockError> {
371+
Err(TryLockError::Error(unsupported_err()))
371372
}
372373

373-
pub fn try_lock_shared(&self) -> io::Result<bool> {
374-
unsupported()
374+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
375+
Err(TryLockError::Error(unsupported_err()))
375376
}
376377

377378
pub fn unlock(&self) -> io::Result<()> {

‎library/std/src/sys/fs/solid.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use crate::ffi::{CStr, CString, OsStr, OsString};
44
use crate::fmt;
5+
use crate::fs::TryLockError;
56
use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom};
67
use crate::mem::MaybeUninit;
78
use crate::os::raw::{c_int, c_short};
@@ -11,7 +12,7 @@ use crate::sync::Arc;
1112
pub use crate::sys::fs::common::exists;
1213
use crate::sys::pal::{abi, error};
1314
use crate::sys::time::SystemTime;
14-
use crate::sys::unsupported;
15+
use crate::sys::{unsupported, unsupported_err};
1516
use crate::sys_common::ignore_notfound;
1617

1718
type CIntNotMinusOne = core::num::niche_types::NotAllOnes<c_int>;
@@ -352,12 +353,12 @@ impl File {
352353
unsupported()
353354
}
354355

355-
pub fn try_lock(&self) -> io::Result<bool> {
356-
unsupported()
356+
pub fn try_lock(&self) -> Result<(), TryLockError> {
357+
Err(TryLockError::Error(unsupported_err()))
357358
}
358359

359-
pub fn try_lock_shared(&self) -> io::Result<bool> {
360-
unsupported()
360+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
361+
Err(TryLockError::Error(unsupported_err()))
361362
}
362363

363364
pub fn unlock(&self) -> io::Result<()> {

‎library/std/src/sys/fs/uefi.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use r_efi::protocols::file;
22

33
use crate::ffi::OsString;
44
use crate::fmt;
5+
use crate::fs::TryLockError;
56
use crate::hash::Hash;
67
use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom};
78
use crate::path::{Path, PathBuf};
@@ -227,11 +228,11 @@ impl File {
227228
self.0
228229
}
229230

230-
pub fn try_lock(&self) -> io::Result<bool> {
231+
pub fn try_lock(&self) -> Result<(), TryLockError> {
231232
self.0
232233
}
233234

234-
pub fn try_lock_shared(&self) -> io::Result<bool> {
235+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
235236
self.0
236237
}
237238

‎library/std/src/sys/fs/unix.rs

+25-14
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, st
7474

7575
use crate::ffi::{CStr, OsStr, OsString};
7676
use crate::fmt::{self, Write as _};
77+
use crate::fs::TryLockError;
7778
use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom};
7879
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd};
7980
use crate::os::unix::prelude::*;
@@ -1307,15 +1308,17 @@ impl File {
13071308
target_os = "netbsd",
13081309
target_vendor = "apple",
13091310
))]
1310-
pub fn try_lock(&self) -> io::Result<bool> {
1311+
pub fn try_lock(&self) -> Result<(), TryLockError> {
13111312
let result = cvt(unsafe { libc::flock(self.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) });
1312-
if let Err(ref err) = result {
1313+
if let Err(err) = result {
13131314
if err.kind() == io::ErrorKind::WouldBlock {
1314-
return Ok(false);
1315+
Err(TryLockError::WouldBlock)
1316+
} else {
1317+
Err(TryLockError::Error(err))
13151318
}
1319+
} else {
1320+
Ok(())
13161321
}
1317-
result?;
1318-
return Ok(true);
13191322
}
13201323

13211324
#[cfg(not(any(
@@ -1325,8 +1328,11 @@ impl File {
13251328
target_os = "netbsd",
13261329
target_vendor = "apple",
13271330
)))]
1328-
pub fn try_lock(&self) -> io::Result<bool> {
1329-
Err(io::const_error!(io::ErrorKind::Unsupported, "try_lock() not supported"))
1331+
pub fn try_lock(&self) -> Result<(), TryLockError> {
1332+
Err(TryLockError::Error(io::const_error!(
1333+
io::ErrorKind::Unsupported,
1334+
"try_lock() not supported"
1335+
)))
13301336
}
13311337

13321338
#[cfg(any(
@@ -1336,15 +1342,17 @@ impl File {
13361342
target_os = "netbsd",
13371343
target_vendor = "apple",
13381344
))]
1339-
pub fn try_lock_shared(&self) -> io::Result<bool> {
1345+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
13401346
let result = cvt(unsafe { libc::flock(self.as_raw_fd(), libc::LOCK_SH | libc::LOCK_NB) });
1341-
if let Err(ref err) = result {
1347+
if let Err(err) = result {
13421348
if err.kind() == io::ErrorKind::WouldBlock {
1343-
return Ok(false);
1349+
Err(TryLockError::WouldBlock)
1350+
} else {
1351+
Err(TryLockError::Error(err))
13441352
}
1353+
} else {
1354+
Ok(())
13451355
}
1346-
result?;
1347-
return Ok(true);
13481356
}
13491357

13501358
#[cfg(not(any(
@@ -1354,8 +1362,11 @@ impl File {
13541362
target_os = "netbsd",
13551363
target_vendor = "apple",
13561364
)))]
1357-
pub fn try_lock_shared(&self) -> io::Result<bool> {
1358-
Err(io::const_error!(io::ErrorKind::Unsupported, "try_lock_shared() not supported"))
1365+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
1366+
Err(TryLockError::Error(io::const_error!(
1367+
io::ErrorKind::Unsupported,
1368+
"try_lock_shared() not supported"
1369+
)))
13591370
}
13601371

13611372
#[cfg(any(

0 commit comments

Comments
 (0)