Skip to content

Commit 4606a19

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#139343 - cberner:filelock_wouldblock, r=workingjubilee
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 These methods are unstable under the "file_lock" feature. The related tracking issue is rust-lang#130999 and this PR changes the signatures as discussed by libs-api: rust-lang#130994 (comment)
2 parents baa9d74 + dd52495 commit 4606a19

File tree

9 files changed

+144
-62
lines changed

9 files changed

+144
-62
lines changed

‎std/src/fs.rs‎

Lines changed: 60 additions & 11 deletions
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,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,30 @@ 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 error::Error for TryLockError {}
373+
374+
#[unstable(feature = "file_lock", issue = "130994")]
375+
impl fmt::Debug for TryLockError {
376+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
377+
match self {
378+
TryLockError::Error(err) => err.fmt(f),
379+
TryLockError::WouldBlock => "WouldBlock".fmt(f),
380+
}
381+
}
382+
}
383+
384+
#[unstable(feature = "file_lock", issue = "130994")]
385+
impl fmt::Display for TryLockError {
386+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
387+
match self {
388+
TryLockError::Error(_) => "lock acquisition failed due to I/O error",
389+
TryLockError::WouldBlock => "lock acquisition failed because the operation would block",
390+
}
391+
.fmt(f)
392+
}
393+
}
394+
355395
impl File {
356396
/// Attempts to open a file in read-only mode.
357397
///
@@ -734,8 +774,8 @@ impl File {
734774

735775
/// Try to acquire an exclusive lock on the file.
736776
///
737-
/// Returns `Ok(false)` if a different lock is already held on this file (via another
738-
/// handle/descriptor).
777+
/// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
778+
/// (via another handle/descriptor).
739779
///
740780
/// This acquires an exclusive lock; no other file handle to this file may acquire another lock.
741781
///
@@ -777,23 +817,27 @@ impl File {
777817
///
778818
/// ```no_run
779819
/// #![feature(file_lock)]
780-
/// use std::fs::File;
820+
/// use std::fs::{File, TryLockError};
781821
///
782822
/// fn main() -> std::io::Result<()> {
783823
/// let f = File::create("foo.txt")?;
784-
/// f.try_lock()?;
824+
/// match f.try_lock() {
825+
/// Ok(_) => (),
826+
/// Err(TryLockError::WouldBlock) => (), // Lock not acquired
827+
/// Err(TryLockError::Error(err)) => return Err(err),
828+
/// }
785829
/// Ok(())
786830
/// }
787831
/// ```
788832
#[unstable(feature = "file_lock", issue = "130994")]
789-
pub fn try_lock(&self) -> io::Result<bool> {
833+
pub fn try_lock(&self) -> Result<(), TryLockError> {
790834
self.inner.try_lock()
791835
}
792836

793837
/// Try to acquire a shared (non-exclusive) lock on the file.
794838
///
795-
/// Returns `Ok(false)` if an exclusive lock is already held on this file (via another
796-
/// handle/descriptor).
839+
/// Returns `Err(TryLockError::WouldBlock)` if a different lock is already held on this file
840+
/// (via another handle/descriptor).
797841
///
798842
/// This acquires a shared lock; more than one file handle may hold a shared lock, but none may
799843
/// hold an exclusive lock at the same time.
@@ -834,16 +878,21 @@ impl File {
834878
///
835879
/// ```no_run
836880
/// #![feature(file_lock)]
837-
/// use std::fs::File;
881+
/// use std::fs::{File, TryLockError};
838882
///
839883
/// fn main() -> std::io::Result<()> {
840884
/// let f = File::open("foo.txt")?;
841-
/// f.try_lock_shared()?;
885+
/// match f.try_lock_shared() {
886+
/// Ok(_) => (),
887+
/// Err(TryLockError::WouldBlock) => (), // Lock not acquired
888+
/// Err(TryLockError::Error(err)) => return Err(err),
889+
/// }
890+
///
842891
/// Ok(())
843892
/// }
844893
/// ```
845894
#[unstable(feature = "file_lock", issue = "130994")]
846-
pub fn try_lock_shared(&self) -> io::Result<bool> {
895+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
847896
self.inner.try_lock_shared()
848897
}
849898

‎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
use crate::sys::fd::FileDesc;
1314
pub use crate::sys::fs::common::{copy, exists};
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
@@ -75,6 +75,7 @@ use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, st
7575

7676
use crate::ffi::{CStr, OsStr, OsString};
7777
use crate::fmt::{self, Write as _};
78+
use crate::fs::TryLockError;
7879
use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom};
7980
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd};
8081
use crate::os::unix::prelude::*;
@@ -1310,15 +1311,17 @@ impl File {
13101311
target_os = "netbsd",
13111312
target_vendor = "apple",
13121313
))]
1313-
pub fn try_lock(&self) -> io::Result<bool> {
1314+
pub fn try_lock(&self) -> Result<(), TryLockError> {
13141315
let result = cvt(unsafe { libc::flock(self.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) });
1315-
if let Err(ref err) = result {
1316+
if let Err(err) = result {
13161317
if err.kind() == io::ErrorKind::WouldBlock {
1317-
return Ok(false);
1318+
Err(TryLockError::WouldBlock)
1319+
} else {
1320+
Err(TryLockError::Error(err))
13181321
}
1322+
} else {
1323+
Ok(())
13191324
}
1320-
result?;
1321-
return Ok(true);
13221325
}
13231326

13241327
#[cfg(not(any(
@@ -1328,8 +1331,11 @@ impl File {
13281331
target_os = "netbsd",
13291332
target_vendor = "apple",
13301333
)))]
1331-
pub fn try_lock(&self) -> io::Result<bool> {
1332-
Err(io::const_error!(io::ErrorKind::Unsupported, "try_lock() not supported"))
1334+
pub fn try_lock(&self) -> Result<(), TryLockError> {
1335+
Err(TryLockError::Error(io::const_error!(
1336+
io::ErrorKind::Unsupported,
1337+
"try_lock() not supported"
1338+
)))
13331339
}
13341340

13351341
#[cfg(any(
@@ -1339,15 +1345,17 @@ impl File {
13391345
target_os = "netbsd",
13401346
target_vendor = "apple",
13411347
))]
1342-
pub fn try_lock_shared(&self) -> io::Result<bool> {
1348+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
13431349
let result = cvt(unsafe { libc::flock(self.as_raw_fd(), libc::LOCK_SH | libc::LOCK_NB) });
1344-
if let Err(ref err) = result {
1350+
if let Err(err) = result {
13451351
if err.kind() == io::ErrorKind::WouldBlock {
1346-
return Ok(false);
1352+
Err(TryLockError::WouldBlock)
1353+
} else {
1354+
Err(TryLockError::Error(err))
13471355
}
1356+
} else {
1357+
Ok(())
13481358
}
1349-
result?;
1350-
return Ok(true);
13511359
}
13521360

13531361
#[cfg(not(any(
@@ -1357,8 +1365,11 @@ impl File {
13571365
target_os = "netbsd",
13581366
target_vendor = "apple",
13591367
)))]
1360-
pub fn try_lock_shared(&self) -> io::Result<bool> {
1361-
Err(io::const_error!(io::ErrorKind::Unsupported, "try_lock_shared() not supported"))
1368+
pub fn try_lock_shared(&self) -> Result<(), TryLockError> {
1369+
Err(TryLockError::Error(io::const_error!(
1370+
io::ErrorKind::Unsupported,
1371+
"try_lock_shared() not supported"
1372+
)))
13621373
}
13631374

13641375
#[cfg(any(

0 commit comments

Comments
 (0)