Make the in-flight claim test wait for the waiter to park
a_second_claim_waits_for_the_first_to_be_released failed on CI: the second claim came back Some. The waiter thread signalled the main thread before calling claim, so the main thread could drop the first guard before the waiter reached the lock. The path was free by then, and the waiter claimed it outright. The registry now keeps a test-only count of threads parked in claim, bumped under the lock just before the condvar wait. The test spins until that count is one before releasing. The release needs the same lock, so it can only reach a waiter that is already waiting. Passed 500 runs in a row.
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -592,6 +592,10 @@ static IN_FLIGHT: std::sync::LazyLock<InFlight> = std::sync::LazyLock::new(InFli
|
|||||||
pub(super) struct InFlight {
|
pub(super) struct InFlight {
|
||||||
busy: std::sync::Mutex<std::collections::HashSet<String>>,
|
busy: std::sync::Mutex<std::collections::HashSet<String>>,
|
||||||
freed: std::sync::Condvar,
|
freed: std::sync::Condvar,
|
||||||
|
/// Threads parked in [`InFlight::claim`], counted under the lock so a
|
||||||
|
/// test can release the holder only once a waiter is really waiting.
|
||||||
|
#[cfg(test)]
|
||||||
|
waiting: std::sync::atomic::AtomicUsize,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl InFlight {
|
impl InFlight {
|
||||||
@@ -609,6 +613,9 @@ impl InFlight {
|
|||||||
path: path.to_string(),
|
path: path.to_string(),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
#[cfg(test)]
|
||||||
|
self.waiting
|
||||||
|
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
|
||||||
while busy.contains(path) {
|
while busy.contains(path) {
|
||||||
busy = self.freed.wait(busy).unwrap_or_else(|e| e.into_inner());
|
busy = self.freed.wait(busy).unwrap_or_else(|e| e.into_inner());
|
||||||
}
|
}
|
||||||
@@ -901,17 +908,16 @@ mod tests {
|
|||||||
let first = registry.claim("shoot/one.CR2");
|
let first = registry.claim("shoot/one.CR2");
|
||||||
assert!(first.is_some(), "an unclaimed path is claimed outright");
|
assert!(first.is_some(), "an unclaimed path is claimed outright");
|
||||||
|
|
||||||
let (tx, rx) = std::sync::mpsc::channel();
|
|
||||||
let waiter = {
|
let waiter = {
|
||||||
let registry = registry.clone();
|
let registry = registry.clone();
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || registry.claim("shoot/one.CR2").is_some())
|
||||||
tx.send(()).unwrap();
|
|
||||||
registry.claim("shoot/one.CR2").is_some()
|
|
||||||
})
|
|
||||||
};
|
};
|
||||||
rx.recv().unwrap();
|
// Wait until the second claim is parked on the condvar. The count is
|
||||||
// The waiter is blocked on the first claim. Not provable without a
|
// bumped under the lock just before the wait, and the release below
|
||||||
// sleep, but a release that reaches it proves the wait ended there.
|
// needs that lock, so it cannot reach the waiter any earlier.
|
||||||
|
while registry.waiting.load(std::sync::atomic::Ordering::SeqCst) == 0 {
|
||||||
|
std::thread::yield_now();
|
||||||
|
}
|
||||||
assert!(
|
assert!(
|
||||||
!waiter.is_finished(),
|
!waiter.is_finished(),
|
||||||
"the second claim must not return while the first is held"
|
"the second claim must not return while the first is held"
|
||||||
|
|||||||
Reference in New Issue
Block a user