Skip to content

Commit ffdc40f

Browse files
committed
bootstrap: Switch from fd-lock to native locking in std
In the process, fix a race condition, by never truncating or writing to the file unless we currently hold the lock.
1 parent 3507a74 commit ffdc40f

File tree

3 files changed

+21
-38
lines changed

3 files changed

+21
-38
lines changed

src/bootstrap/Cargo.lock

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ dependencies = [
4848
"clap",
4949
"clap_complete",
5050
"cmake",
51-
"fd-lock",
5251
"home",
5352
"ignore",
5453
"insta",
@@ -268,17 +267,6 @@ version = "2.3.0"
268267
source = "registry+https://github.com/rust-lang/crates.io-index"
269268
checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be"
270269

271-
[[package]]
272-
name = "fd-lock"
273-
version = "4.0.4"
274-
source = "registry+https://github.com/rust-lang/crates.io-index"
275-
checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78"
276-
dependencies = [
277-
"cfg-if",
278-
"rustix",
279-
"windows-sys 0.59.0",
280-
]
281-
282270
[[package]]
283271
name = "filetime"
284272
version = "0.2.25"

src/bootstrap/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ cmake = "=0.1.54"
3838
build_helper = { path = "../build_helper" }
3939
clap = { version = "4.4", default-features = false, features = ["std", "usage", "help", "derive", "error-context"] }
4040
clap_complete = "4.4"
41-
fd-lock = "4.0"
4241
home = "0.5"
4342
ignore = "0.4"
4443
libc = "0.2"

src/bootstrap/src/bin/main.rs

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
//! parent directory, and otherwise documentation can be found throughout the `build`
66
//! directory in each respective module.
77
8-
use std::fs::{self, OpenOptions};
9-
use std::io::{self, BufRead, BufReader, IsTerminal, Write};
8+
use std::fs::{self, OpenOptions, TryLockError};
9+
use std::io::{self, BufRead, BufReader, IsTerminal, Read, Write};
1010
use std::path::Path;
1111
use std::str::FromStr;
1212
use std::time::Instant;
@@ -39,38 +39,34 @@ fn main() {
3939
let config = Config::parse(flags);
4040

4141
let mut build_lock;
42-
let _build_lock_guard;
4342

4443
if !config.bypass_bootstrap_lock {
4544
// Display PID of process holding the lock
4645
// PID will be stored in a lock file
4746
let lock_path = config.out.join("lock");
48-
let pid = fs::read_to_string(&lock_path);
49-
50-
build_lock = fd_lock::RwLock::new(t!(fs::OpenOptions::new()
47+
build_lock = t!(fs::OpenOptions::new()
48+
.read(true)
5149
.write(true)
52-
.truncate(true)
5350
.create(true)
54-
.open(&lock_path)));
55-
_build_lock_guard = match build_lock.try_write() {
56-
Ok(mut lock) => {
57-
t!(lock.write(process::id().to_string().as_ref()));
58-
lock
51+
.truncate(false)
52+
.open(&lock_path));
53+
t!(build_lock.try_lock().or_else(|e| {
54+
if let TryLockError::Error(e) = e {
55+
return Err(e);
5956
}
60-
err => {
61-
drop(err);
62-
// #135972: We can reach this point when the lock has been taken,
63-
// but the locker has not yet written its PID to the file
64-
if let Some(pid) = pid.ok().filter(|pid| !pid.is_empty()) {
65-
println!("WARNING: build directory locked by process {pid}, waiting for lock");
66-
} else {
67-
println!("WARNING: build directory locked, waiting for lock");
68-
}
69-
let mut lock = t!(build_lock.write());
70-
t!(lock.write(process::id().to_string().as_ref()));
71-
lock
57+
let mut pid = String::new();
58+
t!(build_lock.read_to_string(&mut pid));
59+
// #135972: We can reach this point when the lock has been taken,
60+
// but the locker has not yet written its PID to the file
61+
if !pid.is_empty() {
62+
println!("WARNING: build directory locked by process {pid}, waiting for lock");
63+
} else {
64+
println!("WARNING: build directory locked, waiting for lock");
7265
}
73-
};
66+
build_lock.lock()
67+
}));
68+
t!(build_lock.set_len(0));
69+
t!(build_lock.write_all(process::id().to_string().as_bytes()));
7470
}
7571

7672
// check_version warnings are not printed during setup, or during CI

0 commit comments

Comments
 (0)