Merge pull request #4378 from RalfJung/flock

use File::lock to implement flock, and add a test for File::lock

Author: RalfJung

src/tools/miri/Cargo.lock

@@ -555,7 +555,6 @@ dependencies = [
  "tempfile",
  "tikv-jemalloc-sys",
  "ui_test",
- "windows-sys",
 ]
 
 [[package]]

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"

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,

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");
+                },
         }
     }
 }

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();
+}