From d747b2f36259b0c38d2b6802ff6227344a39d77b Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Fri, 5 May 2023 18:54:33 -0700 Subject: [PATCH] lib: use advisory locks on Unix targets Allows automatic recovery when encountering stale lockfiles, and more efficient blocking rather than polling for fresh ones. The previous implementation is preserved for other platforms. --- Cargo.lock | 36 +++++++++++++-------- lib/Cargo.toml | 3 ++ lib/src/lock.rs | 61 ++++------------------------------ lib/src/lock/fallback.rs | 70 ++++++++++++++++++++++++++++++++++++++++ lib/src/lock/unix.rs | 57 ++++++++++++++++++++++++++++++++ 5 files changed, 158 insertions(+), 69 deletions(-) create mode 100644 lib/src/lock/fallback.rs create mode 100644 lib/src/lock/unix.rs diff --git a/Cargo.lock b/Cargo.lock index 5f1e23152..db56d46a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -557,13 +557,13 @@ dependencies = [ [[package]] name = "errno" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50d6a0976c999d473fe89ad888d5a284e55366d9dc9038b1ba2aa15128c4afa0" +checksum = "4bcfec3a70f97c962c307b2d2c56e358cf1d00b558d74262b5f929ee8cc7e73a" dependencies = [ "errno-dragonfly", "libc", - "windows-sys 0.45.0", + "windows-sys 0.48.0", ] [[package]] @@ -699,6 +699,12 @@ dependencies = [ "libc", ] +[[package]] +name = "hermit-abi" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fed44880c466736ef9a5c5b5facefb5ed0785676d0c02d612db14e54f0d84286" + [[package]] name = "hex" version = "0.4.3" @@ -765,12 +771,13 @@ dependencies = [ [[package]] name = "io-lifetimes" -version = "1.0.1" +version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7d367024b3f3414d8e01f437f704f41a9f64ab36f9067fa73e526ad4c763c87" +checksum = "9c66c74d2ae7e79a5a8f7ac924adbe38ee42a859c6539ad869eb51f0b52dc220" dependencies = [ + "hermit-abi 0.3.1", "libc", - "windows-sys 0.42.0", + "windows-sys 0.48.0", ] [[package]] @@ -887,6 +894,7 @@ dependencies = [ "rand", "rand_chacha", "regex", + "rustix 0.37.19", "serde_json", "smallvec", "tempfile", @@ -965,9 +973,9 @@ checksum = "8f9f08d8963a6c613f4b1a78f4f4a4dbfadf8e6545b2d72861731e4858b8b47f" [[package]] name = "linux-raw-sys" -version = "0.3.0" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd550e73688e6d578f0ac2119e32b797a327631a42f9433e59d02e139c8df60d" +checksum = "ece97ea872ece730aed82664c424eb4c8291e1ff2480247ccf7409044bc6479f" [[package]] name = "lock_api" @@ -1582,16 +1590,16 @@ dependencies = [ [[package]] name = "rustix" -version = "0.37.5" +version = "0.37.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e78cc525325c06b4a7ff02db283472f3c042b7ff0c391f96c6d5ac6f4f91b75" +checksum = "acf8729d8542766f1b2cf77eb034d52f40d375bb8b615d0b147089946e16613d" dependencies = [ "bitflags 1.3.2", - "errno 0.3.0", + "errno 0.3.1", "io-lifetimes", "libc", - "linux-raw-sys 0.3.0", - "windows-sys 0.45.0", + "linux-raw-sys 0.3.7", + "windows-sys 0.48.0", ] [[package]] @@ -1784,7 +1792,7 @@ dependencies = [ "cfg-if", "fastrand", "redox_syscall 0.3.5", - "rustix 0.37.5", + "rustix 0.37.19", "windows-sys 0.45.0", ] diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 16aec168d..c37df6271 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -47,6 +47,9 @@ tracing = "0.1.37" whoami = "1.4.0" zstd = "0.12.3" +[target.'cfg(unix)'.dependencies] +rustix = { version = "0.37.19", features = ["fs"] } + [dev-dependencies] assert_matches = "1.5.0" criterion = "0.4.0" diff --git a/lib/src/lock.rs b/lib/src/lock.rs index fd8f32af6..ea9251531 100644 --- a/lib/src/lock.rs +++ b/lib/src/lock.rs @@ -12,67 +12,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fs::{File, OpenOptions}; -use std::path::PathBuf; -use std::time::Duration; +#[cfg_attr(unix, path = "lock/unix.rs")] +#[cfg_attr(not(unix), path = "lock/fallback.rs")] +mod platform; -use backoff::{retry, ExponentialBackoff}; - -pub struct FileLock { - path: PathBuf, - _file: File, -} - -impl FileLock { - pub fn lock(path: PathBuf) -> FileLock { - let mut options = OpenOptions::new(); - options.create_new(true); - options.write(true); - let try_write_lock_file = || match options.open(&path) { - Ok(file) => Ok(FileLock { - path: path.clone(), - _file: file, - }), - Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { - Err(backoff::Error::Transient { - err, - retry_after: None, - }) - } - Err(err) if cfg!(windows) && err.kind() == std::io::ErrorKind::PermissionDenied => { - Err(backoff::Error::Transient { - err, - retry_after: None, - }) - } - Err(err) => Err(backoff::Error::Permanent(err)), - }; - let backoff = ExponentialBackoff { - initial_interval: Duration::from_millis(1), - max_elapsed_time: Some(Duration::from_secs(10)), - ..Default::default() - }; - match retry(backoff, try_write_lock_file) { - Err(err) => panic!( - "failed to create lock file {}: {}", - path.to_string_lossy(), - err - ), - Ok(file_lock) => file_lock, - } - } -} - -impl Drop for FileLock { - fn drop(&mut self) { - std::fs::remove_file(&self.path).expect("failed to delete lock file"); - } -} +pub use platform::FileLock; #[cfg(test)] mod tests { use std::cmp::max; + use std::fs::OpenOptions; use std::thread; + use std::time::Duration; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; diff --git a/lib/src/lock/fallback.rs b/lib/src/lock/fallback.rs new file mode 100644 index 000000000..56e9ac2af --- /dev/null +++ b/lib/src/lock/fallback.rs @@ -0,0 +1,70 @@ +// Copyright 2020 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::fs::{File, OpenOptions}; +use std::path::PathBuf; +use std::time::Duration; + +use backoff::{retry, ExponentialBackoff}; + +pub struct FileLock { + path: PathBuf, + _file: File, +} + +impl FileLock { + pub fn lock(path: PathBuf) -> FileLock { + let mut options = OpenOptions::new(); + options.create_new(true); + options.write(true); + let try_write_lock_file = || match options.open(&path) { + Ok(file) => Ok(FileLock { + path: path.clone(), + _file: file, + }), + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { + Err(backoff::Error::Transient { + err, + retry_after: None, + }) + } + Err(err) if cfg!(windows) && err.kind() == std::io::ErrorKind::PermissionDenied => { + Err(backoff::Error::Transient { + err, + retry_after: None, + }) + } + Err(err) => Err(backoff::Error::Permanent(err)), + }; + let backoff = ExponentialBackoff { + initial_interval: Duration::from_millis(1), + max_elapsed_time: Some(Duration::from_secs(10)), + ..Default::default() + }; + match retry(backoff, try_write_lock_file) { + Err(err) => panic!( + "failed to create lock file {}: {}", + path.to_string_lossy(), + err + ), + Ok(file_lock) => file_lock, + } + } +} + +impl Drop for FileLock { + fn drop(&mut self) { + std::fs::remove_file(&self.path).expect("failed to delete lock file"); + } +} diff --git a/lib/src/lock/unix.rs b/lib/src/lock/unix.rs new file mode 100644 index 000000000..6ed29278a --- /dev/null +++ b/lib/src/lock/unix.rs @@ -0,0 +1,57 @@ +// Copyright 2023 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::fs::File; +use std::path::PathBuf; + +use rustix::fs::FlockOperation; + +pub struct FileLock { + path: PathBuf, + file: File, +} + +impl FileLock { + pub fn lock(path: PathBuf) -> FileLock { + loop { + // Create lockfile, or open pre-existing one + let file = File::create(&path).expect("failed to open lockfile"); + // If the lock was already held, wait for it to be released + rustix::fs::flock(&file, FlockOperation::LockExclusive) + .expect("failed to lock lockfile"); + + let stat = rustix::fs::fstat(&file).expect("failed to stat lockfile"); + if stat.st_nlink == 0 { + // Lockfile was deleted, probably by the previous holder's `Drop` impl; create a + // new one so our ownership is visible, rather than hidden in an + // unlinked file. Not always necessary, since the previous + // holder might have exited abruptly. + continue; + } + + return Self { path, file }; + } + } +} + +impl Drop for FileLock { + fn drop(&mut self) { + // Removing the file isn't strictly necessary, but reduces confusion. + _ = std::fs::remove_file(&self.path); + // Unblock any processes that tried to acquire the lock while we held it. + // They're responsible for creating and locking a new lockfile, since we + // just deleted this one. + _ = rustix::fs::flock(&self.file, FlockOperation::Unlock); + } +}