From a23dd0d1e6ddfe6624f1c59e9aefcb59e419610d Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 13 May 2020 02:43:23 +0100 Subject: [PATCH 1/3] Replace fcntl-based file lock with flock WSL1 does not support `fcntl`-based lock and will always report success, therefore creating a race condition when multiple rustc processes are modifying shared data such as `search-index.js`. WSL1 does however support `flock`. `flock` is supported by all unix-like platforms. The only caveat is that Linux 2.6.11 or earlier does not support `flock` on NFS mounts, but as the minimum supported Linux version is 2.6.18, it is not an issue. Fixes #72157 --- src/librustc_data_structures/flock.rs | 67 ++++++++------------------- 1 file changed, 19 insertions(+), 48 deletions(-) diff --git a/src/librustc_data_structures/flock.rs b/src/librustc_data_structures/flock.rs index 2a0139fa90d5a..d6fc552e66149 100644 --- a/src/librustc_data_structures/flock.rs +++ b/src/librustc_data_structures/flock.rs @@ -12,13 +12,12 @@ use std::path::Path; cfg_if! { if #[cfg(unix)] { - use std::ffi::{CString, OsStr}; - use std::mem; use std::os::unix::prelude::*; + use std::fs::{File, OpenOptions}; #[derive(Debug)] pub struct Lock { - fd: libc::c_int, + _file: File, } impl Lock { @@ -27,63 +26,35 @@ cfg_if! { create: bool, exclusive: bool) -> io::Result { - let os: &OsStr = p.as_ref(); - let buf = CString::new(os.as_bytes()).unwrap(); - let open_flags = if create { - libc::O_RDWR | libc::O_CREAT + let file = OpenOptions::new() + .read(true) + .write(true) + .create(create) + .mode(libc::S_IRWXU as u32) + .open(p)?; + + let mut operation = if exclusive { + libc::LOCK_EX } else { - libc::O_RDWR - }; - - let fd = unsafe { - libc::open(buf.as_ptr(), open_flags, - libc::S_IRWXU as libc::c_int) + libc::LOCK_SH }; - - if fd < 0 { - return Err(io::Error::last_os_error()); + if !wait { + operation |= libc::LOCK_NB } - let lock_type = if exclusive { - libc::F_WRLCK - } else { - libc::F_RDLCK - }; - - let mut flock: libc::flock = unsafe { mem::zeroed() }; - flock.l_type = lock_type as libc::c_short; - flock.l_whence = libc::SEEK_SET as libc::c_short; - flock.l_start = 0; - flock.l_len = 0; - - let cmd = if wait { libc::F_SETLKW } else { libc::F_SETLK }; - let ret = unsafe { - libc::fcntl(fd, cmd, &flock) - }; + let ret = unsafe { libc::flock(file.as_raw_fd(), operation) }; if ret == -1 { let err = io::Error::last_os_error(); - unsafe { libc::close(fd); } Err(err) } else { - Ok(Lock { fd }) + Ok(Lock { _file: file }) } } } - impl Drop for Lock { - fn drop(&mut self) { - let mut flock: libc::flock = unsafe { mem::zeroed() }; - flock.l_type = libc::F_UNLCK as libc::c_short; - flock.l_whence = libc::SEEK_SET as libc::c_short; - flock.l_start = 0; - flock.l_len = 0; - - unsafe { - libc::fcntl(self.fd, libc::F_SETLK, &flock); - libc::close(self.fd); - } - } - } + // Note that we don't need a Drop impl to execute `flock(fd, LOCK_UN)`. Lock acquired by + // `flock` is associated with the file descriptor and closing the file release it + // automatically. } else if #[cfg(windows)] { use std::mem; use std::os::windows::prelude::*; From 564ebbb0d19283894e87cd09333375aa0c84f8d9 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Wed, 20 May 2020 02:27:03 +0100 Subject: [PATCH 2/3] Use fcntl-based file lock for non-Linux unix --- src/librustc_data_structures/flock.rs | 77 +++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/src/librustc_data_structures/flock.rs b/src/librustc_data_structures/flock.rs index d6fc552e66149..655248e0f5221 100644 --- a/src/librustc_data_structures/flock.rs +++ b/src/librustc_data_structures/flock.rs @@ -7,13 +7,13 @@ #![allow(non_camel_case_types)] #![allow(nonstandard_style)] +use std::fs::{File, OpenOptions}; use std::io; use std::path::Path; cfg_if! { - if #[cfg(unix)] { + if #[cfg(target_os = "linux")] { use std::os::unix::prelude::*; - use std::fs::{File, OpenOptions}; #[derive(Debug)] pub struct Lock { @@ -27,11 +27,11 @@ cfg_if! { exclusive: bool) -> io::Result { let file = OpenOptions::new() - .read(true) - .write(true) - .create(create) - .mode(libc::S_IRWXU as u32) - .open(p)?; + .read(true) + .write(true) + .create(create) + .mode(libc::S_IRWXU as u32) + .open(p)?; let mut operation = if exclusive { libc::LOCK_EX @@ -44,8 +44,7 @@ cfg_if! { let ret = unsafe { libc::flock(file.as_raw_fd(), operation) }; if ret == -1 { - let err = io::Error::last_os_error(); - Err(err) + Err(io::Error::last_os_error()) } else { Ok(Lock { _file: file }) } @@ -55,10 +54,68 @@ cfg_if! { // Note that we don't need a Drop impl to execute `flock(fd, LOCK_UN)`. Lock acquired by // `flock` is associated with the file descriptor and closing the file release it // automatically. + } else if #[cfg(unix)] { + use std::mem; + use std::os::unix::prelude::*; + + #[derive(Debug)] + pub struct Lock { + file: File, + } + + impl Lock { + pub fn new(p: &Path, + wait: bool, + create: bool, + exclusive: bool) + -> io::Result { + let file = OpenOptions::new() + .read(true) + .write(true) + .create(create) + .mode(libc::S_IRWXU as u32) + .open(p)?; + + let lock_type = if exclusive { + libc::F_WRLCK + } else { + libc::F_RDLCK + }; + + let mut flock: libc::flock = unsafe { mem::zeroed() }; + flock.l_type = lock_type as libc::c_short; + flock.l_whence = libc::SEEK_SET as libc::c_short; + flock.l_start = 0; + flock.l_len = 0; + + let cmd = if wait { libc::F_SETLKW } else { libc::F_SETLK }; + let ret = unsafe { + libc::fcntl(file.as_raw_fd(), cmd, &flock) + }; + if ret == -1 { + Err(io::Error::last_os_error()) + } else { + Ok(Lock { file }) + } + } + } + + impl Drop for Lock { + fn drop(&mut self) { + let mut flock: libc::flock = unsafe { mem::zeroed() }; + flock.l_type = libc::F_UNLCK as libc::c_short; + flock.l_whence = libc::SEEK_SET as libc::c_short; + flock.l_start = 0; + flock.l_len = 0; + + unsafe { + libc::fcntl(self.file.as_raw_fd(), libc::F_SETLK, &flock); + } + } + } } else if #[cfg(windows)] { use std::mem; use std::os::windows::prelude::*; - use std::fs::{File, OpenOptions}; use winapi::um::minwinbase::{OVERLAPPED, LOCKFILE_FAIL_IMMEDIATELY, LOCKFILE_EXCLUSIVE_LOCK}; use winapi::um::fileapi::LockFileEx; From a05acbf36f5a1f49919253281fc9bf9465606070 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 21 May 2020 01:10:52 +0100 Subject: [PATCH 3/3] Comment flock usage on Linux --- src/librustc_data_structures/flock.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/librustc_data_structures/flock.rs b/src/librustc_data_structures/flock.rs index 655248e0f5221..9383be474fd5a 100644 --- a/src/librustc_data_structures/flock.rs +++ b/src/librustc_data_structures/flock.rs @@ -12,6 +12,11 @@ use std::io; use std::path::Path; cfg_if! { + // We use `flock` rather than `fcntl` on Linux, because WSL1 does not support + // `fcntl`-style advisory locks properly (rust-lang/rust#72157). + // + // For other Unix targets we still use `fcntl` because it's more portable than + // `flock`. if #[cfg(target_os = "linux")] { use std::os::unix::prelude::*;