Skip to content

Commit a247689

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 6ca48ca commit a247689

File tree

9 files changed

+140
-61
lines changed

9 files changed

+140
-61
lines changed

‎std/src/fs.rs‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,22 @@ 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. The standard library will
127+
/// not return an [`ErrorKind::WouldBlock`] error inside [`TryLockError::Error`]
128+
///
129+
/// [`ErrorKind::WouldBlock`]: io::ErrorKind::WouldBlock
130+
Error(io::Error),
131+
/// The lock could not be acquired at this time because it is held by another handle/process.
132+
WouldBlock,
133+
}
134+
119135
/// Metadata information about a file.
120136
///
121137
/// 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
352368
inner(path.as_ref(), contents.as_ref())
353369
}
354370

371+
#[unstable(feature = "file_lock", issue = "130994")]
372+
impl fmt::Debug for TryLockError {
373+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
374+
match self {
375+
TryLockError::Error(err) => err.fmt(f),
376+
TryLockError::WouldBlock => "WouldBlock".fmt(f),
377+
}
378+
}
379+
}
380+
381+
#[unstable(feature = "file_lock", issue = "130994")]
382+
impl fmt::Display for TryLockError {
383+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
384+
match self {
385+
TryLockError::Error(_) => "lock acquisition failed due to I/O error",
386+
TryLockError::WouldBlock => "lock acquisition failed because the operation would block",
387+
}
388+
.fmt(f)
389+
}
390+
}
391+
355392
impl File {
356393
/// Attempts to open a file in read-only mode.
357394
///
@@ -734,8 +771,8 @@ impl File {
734771

735772
/// Try to acquire an exclusive lock on the file.
736773
///
737-
/// Returns `Ok(false)` if a different lock is already held on this file (via another
738-
/// handle/descriptor).
774+
/// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
775+
/// (via another handle/descriptor).
739776
///
740777
/// This acquires an exclusive lock; no other file handle to this file may acquire another lock.
741778
///
@@ -777,23 +814,27 @@ impl File {
777814
///
778815
/// ```no_run
779816
/// #![feature(file_lock)]
780-
/// use std::fs::File;
817+
/// use std::fs::{File, TryLockError};
781818
///
782819
/// fn main() -> std::io::Result<()> {
783820
/// let f = File::create("foo.txt")?;
784-
/// f.try_lock()?;
821+
/// match f.try_lock() {
822+
/// Ok(_) => (),
823+
/// Err(TryLockError::WouldBlock) => (), // Lock not acquired
824+
/// Err(TryLockError::Error(err)) => return Err(err),
825+
/// }
785826
/// Ok(())
786827
/// }
787828
/// ```
788829
#[unstable(feature = "file_lock", issue = "130994")]
789-
pub fn try_lock(&self) -> io::Result<bool> {
830+
pub fn try_lock(&self) -> Result<(), TryLockError> {
790831
self.inner.try_lock()
791832
}
792833

793834
/// Try to acquire a shared (non-exclusive) lock on the file.
794835
///
795-
/// Returns `Ok(false)` if an exclusive lock is already held on this file (via another
796-
/// handle/descriptor).
836+
/// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
837+
/// (via another handle/descriptor).
797838
///
798839
/// This acquires a shared lock; more than one file handle may hold a shared lock, but none may
799840
/// hold an exclusive lock at the same time.
@@ -834,16 +875,21 @@ impl File {
834875
///
835876
/// ```no_run
836877
/// #![feature(file_lock)]
837-
/// use std::fs::File;
878+
/// use std::fs::{File, TryLockError};
838879
///
839880
/// fn main() -> std::io::Result<()> {
840881
/// let f = File::open("foo.txt")?;
841-
/// f.try_lock_shared()?;
882+
/// match f.try_lock_shared() {
883+
/// Ok(_) => (),
884+
/// Err(TryLockError::WouldBlock) => (), // Lock not acquired
885+
/// Err(TryLockError::Error(err)) => return Err(err),
886+
/// }
887+
///
842888
/// Ok(())
843889
/// }
844890
/// ```
845891
#[unstable(feature = "file_lock", issue = "130994")]
846-
pub fn try_lock_shared(&self) -> io::Result<bool> {
892+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
847893
self.inner.try_lock_shared()
848894
}
849895

‎std/src/fs/tests.rs‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,22 @@
11
use rand::RngCore;
22

3+
#[cfg(any(
4+
windows,
5+
target_os = "freebsd",
6+
target_os = "linux",
7+
target_os = "netbsd",
8+
target_vendor = "apple",
9+
))]
10+
use crate::assert_matches::assert_matches;
311
use crate::char::MAX_LEN_UTF8;
12+
#[cfg(any(
13+
windows,
14+
target_os = "freebsd",
15+
target_os = "linux",
16+
target_os = "netbsd",
17+
target_vendor = "apple",
18+
))]
19+
use crate::fs::TryLockError;
420
use crate::fs::{self, File, FileTimes, OpenOptions};
521
use crate::io::prelude::*;
622
use crate::io::{BorrowedBuf, ErrorKind, SeekFrom};
@@ -223,8 +239,8 @@ fn file_lock_multiple_shared() {
223239
check!(f2.lock_shared());
224240
check!(f1.unlock());
225241
check!(f2.unlock());
226-
assert!(check!(f1.try_lock_shared()));
227-
assert!(check!(f2.try_lock_shared()));
242+
check!(f1.try_lock_shared());
243+
check!(f2.try_lock_shared());
228244
}
229245

230246
#[test]
@@ -243,12 +259,12 @@ fn file_lock_blocking() {
243259

244260
// Check that shared locks block exclusive locks
245261
check!(f1.lock_shared());
246-
assert!(!check!(f2.try_lock()));
262+
assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
247263
check!(f1.unlock());
248264

249265
// Check that exclusive locks block shared locks
250266
check!(f1.lock());
251-
assert!(!check!(f2.try_lock_shared()));
267+
assert_matches!(f2.try_lock_shared(), Err(TryLockError::WouldBlock));
252268
}
253269

254270
#[test]
@@ -267,9 +283,9 @@ fn file_lock_drop() {
267283

268284
// Check that locks are released when the File is dropped
269285
check!(f1.lock_shared());
270-
assert!(!check!(f2.try_lock()));
286+
assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
271287
drop(f1);
272-
assert!(check!(f2.try_lock()));
288+
check!(f2.try_lock());
273289
}
274290

275291
#[test]
@@ -288,10 +304,10 @@ fn file_lock_dup() {
288304

289305
// Check that locks are not dropped if the File has been cloned
290306
check!(f1.lock_shared());
291-
assert!(!check!(f2.try_lock()));
307+
assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
292308
let cloned = check!(f1.try_clone());
293309
drop(f1);
294-
assert!(!check!(f2.try_lock()));
310+
assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
295311
drop(cloned)
296312
}
297313

@@ -307,9 +323,9 @@ fn file_lock_double_unlock() {
307323
// Check that both are released by unlock()
308324
check!(f1.lock());
309325
check!(f1.lock_shared());
310-
assert!(!check!(f2.try_lock()));
326+
assert_matches!(f2.try_lock(), Err(TryLockError::WouldBlock));
311327
check!(f1.unlock());
312-
assert!(check!(f2.try_lock()));
328+
check!(f2.try_lock());
313329
}
314330

315331
#[test]

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

Lines changed: 6 additions & 5 deletions
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<()> {

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

Lines changed: 6 additions & 5 deletions
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<()> {

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

Lines changed: 3 additions & 2 deletions
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

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

Lines changed: 25 additions & 14 deletions
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(

‎std/src/sys/fs/unsupported.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::ffi::OsString;
22
use crate::fmt;
3+
use crate::fs::TryLockError;
34
use crate::hash::{Hash, Hasher};
45
use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut, SeekFrom};
56
use crate::path::{Path, PathBuf};
@@ -206,11 +207,11 @@ impl File {
206207
self.0
207208
}
208209

209-
pub fn try_lock(&self) -> io::Result<bool> {
210+
pub fn try_lock(&self) -> Result<(), TryLockError> {
210211
self.0
211212
}
212213

213-
pub fn try_lock_shared(&self) -> io::Result<bool> {
214+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
214215
self.0
215216
}
216217

0 commit comments

Comments
 (0)