From b1652dc7d53e24196d3745f6196411c61ccd53fa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 5 Jun 2025 18:52:26 +0200 Subject: [PATCH] use File::lock to implement flock, and add a test for File::lock --- src/tools/miri/Cargo.lock | 1 - src/tools/miri/Cargo.toml | 7 -- src/tools/miri/src/lib.rs | 1 + src/tools/miri/src/shims/unix/fs.rs | 113 +++++--------------------- src/tools/miri/tests/pass/shims/fs.rs | 33 +++++++- 5 files changed, 50 insertions(+), 105 deletions(-) diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index 6f4bd3eab51a..192d4f444c2f 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -555,7 +555,6 @@ dependencies = [ "tempfile", "tikv-jemalloc-sys", "ui_test", - "windows-sys", ] [[package]] diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index e4d7abdb0f73..a314488bb253 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -40,13 +40,6 @@ libc = "0.2" libffi = "4.0.0" libloading = "0.8" -[target.'cfg(target_family = "windows")'.dependencies] -windows-sys = { version = "0.59", features = [ - "Win32_Foundation", - "Win32_System_IO", - "Win32_Storage_FileSystem", -] } - [dev-dependencies] ui_test = "0.29.1" colored = "2" diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 51ec19af52a0..dfefe2f4b052 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -16,6 +16,7 @@ #![feature(unqualified_local_imports)] #![feature(derive_coerce_pointee)] #![feature(arbitrary_self_types)] +#![feature(file_lock)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 31cb269059c1..0f7d453b296c 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -2,7 +2,8 @@ use std::borrow::Cow; use std::fs::{ - DirBuilder, File, FileType, OpenOptions, ReadDir, read_dir, remove_dir, remove_file, rename, + DirBuilder, File, FileType, OpenOptions, ReadDir, TryLockError, read_dir, remove_dir, + remove_file, rename, }; use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; @@ -89,103 +90,27 @@ impl UnixFileDescription for FileHandle { communicate_allowed: bool, op: FlockOp, ) -> InterpResult<'tcx, io::Result<()>> { - // cfg(bootstrap) - macro_rules! cfg_select_dispatch { - ($($tokens:tt)*) => { - #[cfg(bootstrap)] - cfg_match! { $($tokens)* } - - #[cfg(not(bootstrap))] - cfg_select! { $($tokens)* } - }; - } - assert!(communicate_allowed, "isolation should have prevented even opening a file"); - cfg_select_dispatch! { - all(target_family = "unix", not(target_os = "solaris")) => { - use std::os::fd::AsRawFd; - use FlockOp::*; - // We always use non-blocking call to prevent interpreter from being blocked - let (host_op, lock_nb) = match op { - SharedLock { nonblocking } => (libc::LOCK_SH | libc::LOCK_NB, nonblocking), - ExclusiveLock { nonblocking } => (libc::LOCK_EX | libc::LOCK_NB, nonblocking), - Unlock => (libc::LOCK_UN, false), - }; - - let fd = self.file.as_raw_fd(); - let ret = unsafe { libc::flock(fd, host_op) }; - let res = match ret { - 0 => Ok(()), - -1 => { - let err = io::Error::last_os_error(); - if !lock_nb && err.kind() == io::ErrorKind::WouldBlock { - throw_unsup_format!("blocking `flock` is not currently supported"); - } - Err(err) - } - ret => panic!("Unexpected return value from flock: {ret}"), - }; - interp_ok(res) + use FlockOp::*; + // We must not block the interpreter loop, so we always `try_lock`. + let (res, nonblocking) = match op { + SharedLock { nonblocking } => (self.file.try_lock_shared(), nonblocking), + ExclusiveLock { nonblocking } => (self.file.try_lock(), nonblocking), + Unlock => { + return interp_ok(self.file.unlock()); } - target_family = "windows" => { - use std::os::windows::io::AsRawHandle; + }; - use windows_sys::Win32::Foundation::{ - ERROR_IO_PENDING, ERROR_LOCK_VIOLATION, FALSE, TRUE, - }; - use windows_sys::Win32::Storage::FileSystem::{ - LOCKFILE_EXCLUSIVE_LOCK, LOCKFILE_FAIL_IMMEDIATELY, LockFileEx, UnlockFile, - }; - - let fh = self.file.as_raw_handle(); - - use FlockOp::*; - let (ret, lock_nb) = match op { - SharedLock { nonblocking } | ExclusiveLock { nonblocking } => { - // We always use non-blocking call to prevent interpreter from being blocked - let mut flags = LOCKFILE_FAIL_IMMEDIATELY; - if matches!(op, ExclusiveLock { .. }) { - flags |= LOCKFILE_EXCLUSIVE_LOCK; - } - let ret = unsafe { LockFileEx(fh, flags, 0, !0, !0, &mut std::mem::zeroed()) }; - (ret, nonblocking) - } - Unlock => { - let ret = unsafe { UnlockFile(fh, 0, 0, !0, !0) }; - (ret, false) - } - }; - - let res = match ret { - TRUE => Ok(()), - FALSE => { - let mut err = io::Error::last_os_error(); - // This only runs on Windows hosts so we can use `raw_os_error`. - // We have to be careful not to forward that error code to target code. - let code: u32 = err.raw_os_error().unwrap().try_into().unwrap(); - if matches!(code, ERROR_IO_PENDING | ERROR_LOCK_VIOLATION) { - if lock_nb { - // The io error mapping does not know about these error codes, - // so we translate it to `WouldBlock` manually. - let desc = format!("LockFileEx wouldblock error: {err}"); - err = io::Error::new(io::ErrorKind::WouldBlock, desc); - } else { - throw_unsup_format!("blocking `flock` is not currently supported"); - } - } - Err(err) - } - _ => panic!("Unexpected return value: {ret}"), - }; - interp_ok(res) - } - _ => { - let _ = op; - throw_unsup_format!( - "flock is supported only on UNIX (except Solaris) and Windows hosts" - ); - } + match res { + Ok(()) => interp_ok(Ok(())), + Err(TryLockError::Error(err)) => interp_ok(Err(err)), + Err(TryLockError::WouldBlock) => + if nonblocking { + interp_ok(Err(ErrorKind::WouldBlock.into())) + } else { + throw_unsup_format!("blocking `flock` is not currently supported"); + }, } } } diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index 87df43ca7e57..2f30827c9336 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -2,12 +2,12 @@ #![feature(io_error_more)] #![feature(io_error_uncategorized)] +#![feature(file_lock)] use std::collections::BTreeMap; use std::ffi::OsString; use std::fs::{ - File, OpenOptions, canonicalize, create_dir, read_dir, remove_dir, remove_dir_all, remove_file, - rename, + self, File, OpenOptions, create_dir, read_dir, remove_dir, remove_dir_all, remove_file, rename, }; use std::io::{Error, ErrorKind, IsTerminal, Read, Result, Seek, SeekFrom, Write}; use std::path::Path; @@ -33,6 +33,8 @@ fn main() { test_canonicalize(); #[cfg(unix)] test_pread_pwrite(); + #[cfg(not(any(target_os = "solaris", target_os = "illumos")))] + test_flock(); } } @@ -240,7 +242,7 @@ fn test_canonicalize() { let path = dir_path.join("test_file"); drop(File::create(&path).unwrap()); - let p = canonicalize(format!("{}/./test_file", dir_path.to_string_lossy())).unwrap(); + let p = fs::canonicalize(format!("{}/./test_file", dir_path.to_string_lossy())).unwrap(); assert_eq!(p.to_string_lossy().find("/./"), None); remove_dir_all(&dir_path).unwrap(); @@ -351,3 +353,28 @@ fn test_pread_pwrite() { f.read_exact(&mut buf1).unwrap(); assert_eq!(&buf1, b" m"); } + +// This function does seem to exist on Illumos but std does not expose it there. +#[cfg(not(any(target_os = "solaris", target_os = "illumos")))] +fn test_flock() { + let bytes = b"Hello, World!\n"; + let path = utils::prepare_with_content("miri_test_fs_flock.txt", bytes); + let file1 = OpenOptions::new().read(true).write(true).open(&path).unwrap(); + let file2 = OpenOptions::new().read(true).write(true).open(&path).unwrap(); + + // Test that we can apply many shared locks + file1.lock_shared().unwrap(); + file2.lock_shared().unwrap(); + // Test that shared lock prevents exclusive lock + assert!(matches!(file1.try_lock().unwrap_err(), fs::TryLockError::WouldBlock)); + // Unlock shared lock + file1.unlock().unwrap(); + file2.unlock().unwrap(); + // Take exclusive lock + file1.lock().unwrap(); + // Test that shared lock prevents exclusive and shared locks + assert!(matches!(file2.try_lock().unwrap_err(), fs::TryLockError::WouldBlock)); + assert!(matches!(file2.try_lock_shared().unwrap_err(), fs::TryLockError::WouldBlock)); + // Unlock exclusive lock + file1.unlock().unwrap(); +}