Skip to content

Commit 79e00b2

Browse files
committed
volatile: fix a race window in test case
There's a race window in observe_mutate() and we have observed it breaking the coverage test several times. So fix it by using Barrier. Signed-off-by: Liu Jiang <[email protected]>
1 parent 3e0ff7b commit 79e00b2

File tree

1 file changed

+9
-26
lines changed

1 file changed

+9
-26
lines changed

src/volatile_memory.rs

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,9 +1368,8 @@ mod tests {
13681368
use std::mem::size_of_val;
13691369
use std::path::Path;
13701370
use std::sync::atomic::{AtomicUsize, Ordering};
1371-
use std::sync::Arc;
1372-
use std::thread::{sleep, spawn};
1373-
use std::time::Duration;
1371+
use std::sync::{Arc, Barrier};
1372+
use std::thread::spawn;
13741373

13751374
use matches::assert_matches;
13761375
use vmm_sys_util::tempfile::TempFile;
@@ -1563,36 +1562,20 @@ mod tests {
15631562
let a = VecMem::new(1);
15641563
let a_clone = a.clone();
15651564
let v_ref = a.get_ref::<u8>(0).unwrap();
1565+
let barrier = Arc::new(Barrier::new(2));
1566+
let barrier1 = barrier.clone();
1567+
15661568
v_ref.store(99);
15671569
spawn(move || {
1568-
sleep(Duration::from_millis(10));
1570+
barrier1.wait();
15691571
let clone_v_ref = a_clone.get_ref::<u8>(0).unwrap();
15701572
clone_v_ref.store(0);
1573+
barrier1.wait();
15711574
});
15721575

1573-
// Technically this is a race condition but we have to observe the v_ref's value changing
1574-
// somehow and this helps to ensure the sleep actually happens before the store rather then
1575-
// being reordered by the compiler.
15761576
assert_eq!(v_ref.load(), 99);
1577-
1578-
// Granted we could have a machine that manages to perform this many volatile loads in the
1579-
// amount of time the spawned thread sleeps, but the most likely reason the retry limit will
1580-
// get reached is because v_ref.load() is not actually performing the required volatile read
1581-
// or v_ref.store() is not doing a volatile write. A timer based solution was avoided
1582-
// because that might use a syscall which could hint the optimizer to reload v_ref's pointer
1583-
// regardless of volatile status. Note that we use a longer retry duration for optimized
1584-
// builds.
1585-
#[cfg(debug_assertions)]
1586-
const RETRY_MAX: usize = 500_000_000;
1587-
#[cfg(not(debug_assertions))]
1588-
const RETRY_MAX: usize = 10_000_000_000;
1589-
1590-
let mut retry = 0;
1591-
while v_ref.load() == 99 && retry < RETRY_MAX {
1592-
retry += 1;
1593-
}
1594-
1595-
assert_ne!(retry, RETRY_MAX, "maximum retry exceeded");
1577+
barrier.wait();
1578+
barrier.wait();
15961579
assert_eq!(v_ref.load(), 0);
15971580
}
15981581

0 commit comments

Comments
 (0)