Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/cargo/util/flock.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if a test is required for this change and where it should go if required. I was thinking maybe in testsuite/concurrent.rs.

Either there or here in tests/testsuite/cache_lock.rs. We do have some tests around that but pretty brutal:

/// Helper to verify that acquiring the given mode would block.
///
/// Always call `thread_wait_timeout` on the result.
#[must_use]
fn verify_lock_would_block(mode: CacheLockMode) -> JoinHandle<()> {
let root = paths::root();
// Spawn a thread that will block on the lock.
let thread = std::thread::spawn(move || {
let gctx = GlobalContextBuilder::new().root(root).build();
let locker2 = CacheLocker::new();
let lock2 = locker2.lock(&gctx, mode).unwrap();
assert!(locker2.is_locked(mode));
drop(lock2);
});
// Verify that it blocked.
retry(100, || {
if let Ok(s) = std::fs::read_to_string(paths::root().join("shell.out")) {
if s.trim().starts_with("Blocking waiting for file lock on") {
return Some(());
} else {
eprintln!("unexpected output: {s}");
// Try again, it might have been partially written.
}
}
None
});
thread
}

// Wait for clean to finish.
let thread = std::thread::spawn(|| clean_child.wait_with_output().unwrap());
let output = thread_wait_timeout(100, thread);
assert!(output.status.success());
// Validate the output of the clean.
execs()
.with_stderr_data(str![[r#"
[BLOCKING] waiting for file lock on package cache mutation
[REMOVED] [FILE_NUM] files, [FILE_SIZE]B total
"#]])
.run_output(&output);

Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,13 @@ fn acquire(
if try_acquire(path, lock_try)? {
return Ok(());
}
let msg = format!("waiting for file lock on {}", msg);

let msg = if gctx.extra_verbose() {
format!("waiting for file lock on {} ({})", msg, path.display())
} else {
format!("waiting for file lock on {}", msg)
};

gctx.shell()
.status_with_color("Blocking", &msg, &style::NOTE)?;

Expand Down
70 changes: 69 additions & 1 deletion tests/testsuite/concurrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use cargo_test_support::install::assert_has_installed_exe;
use cargo_test_support::paths;
use cargo_test_support::registry::Package;
use cargo_test_support::str;
use cargo_test_support::{basic_manifest, execs, project, slow_cpu_multiplier};
use cargo_test_support::{basic_manifest, execs, project, retry, sleep_ms, slow_cpu_multiplier};

fn pkg(name: &str, vers: &str) {
Package::new(name, vers)
Expand Down Expand Up @@ -510,3 +510,71 @@ fn no_deadlock_with_git_dependencies() {
execs().run_output(&result);
}
}

#[cargo_test]
fn verbose_file_lock_blocking() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
edition = "2024"
"#,
)
.file("src/main.rs", "fn main() {}")
.file(
"build.rs",
r#"
fn main() {
let blocking = std::env::var("BLOCKING").unwrap_or_default();
if blocking == "1" {
std::fs::write("blocking", "").unwrap();
let path = std::path::Path::new("ready");
loop {
if path.exists() {
break;
} else {
std::thread::sleep(std::time::Duration::from_millis(100))
}
}
}
}
"#,
)
.build();

// start a build that will hold the lock on the build directory
let mut a = p
.cargo("check")
.build_command()
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.env("BLOCKING", "1")
.spawn()
.unwrap();

// wait for the build script to start
retry(100, || p.root().join("blocking").exists().then_some(()));

let blocked_p = p
.cargo("check -vv")
.build_command()
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.env("BLOCKING", "0")
.spawn()
.unwrap();

sleep_ms(500);

// release the lock on the build directory
std::fs::write(p.root().join("ready"), "").unwrap();

let blocked_p_otpt = blocked_p.wait_with_output().unwrap();

assert!(a.wait().unwrap().success());
execs()
.with_stderr_contains("[BLOCKING] waiting for file lock on build directory ([ROOT]/foo/target/debug/.cargo-lock)")
.run_output(&blocked_p_otpt);
}