tokio-rs / loom

Concurrency permutation testing tool for Rust.
MIT License
2.09k stars 110 forks source link

Possibly missing interleavings when adding a redundant `load` operation #166

Open zyla opened 4 years ago

zyla commented 4 years ago

Hello! I believe I found a case where Loom skips some possible executions of a program. Here's a test case I prepared:

use loom::sync::atomic::{AtomicUsize, Ordering::*};
use std::hash::Hash;
use std::sync::Arc;

pub struct Entry {
    key: AtomicUsize,
    value: AtomicUsize,
}

#[test]
fn test_outcomes_should_be_the_same() {
    assert_eq!(run_example(false), run_example(true));
}

fn run_example(with_extra_load: bool) -> Vec<(usize, usize)> {
    println!("run_example(with_extra_load={})", with_extra_load);
    collect_all_outcomes(move || {
        let entry = Arc::new(Entry {
            key: AtomicUsize::new(0),
            value: AtomicUsize::new(0),
        });
        let entry1 = entry.clone();
        let entry2 = entry.clone();

        let t1 = loom::thread::spawn(move || {
            entry1.key.store(1, SeqCst);
            entry1.value.store(1, SeqCst);
            entry1.value.store(0, SeqCst);

            if with_extra_load {
                entry1.key.load(SeqCst);
            }

            entry1.key.store(0, SeqCst);
        });

        let t2 = loom::thread::spawn(move || loop {
            let value = entry2.value.load(SeqCst);
            let key = entry2.key.load(SeqCst);
            return (value, key);
        });

        t1.join().unwrap();
        t2.join().unwrap()
    })
}

/// Run all interleavings of the given function using Loom and return the sorted list of all
/// observed outcomes.
fn collect_all_outcomes<A: Hash + Ord + Send + 'static>(
    f: impl Fn() -> A + Sync + Send + 'static,
) -> Vec<A> {
    use std::collections::HashSet;
    use std::sync::Mutex;
    let result_set: Arc<Mutex<HashSet<A>>> = Arc::new(Mutex::new(HashSet::new()));
    let result_set_2 = result_set.clone();
    loom::model(move || {
        let result = f();
        result_set.lock().unwrap().insert(result);
    });
    let mut results = result_set_2.lock().unwrap().drain().collect::<Vec<_>>();
    results.sort();
    results
}

The test runs the example in two versions: one with an extra entry1.key.load(SeqCst); call, and one without. The set of possible outcomes should be the same, since the load shouldn't affect the semantics of the program (we don't use its value, and all the other accesses are also SeqCst, so it doesn't constrain ordering further).

However, when I run it, the test fails:

---- test_outcomes_should_be_the_same stdout ----
run_example(with_extra_load=false)
Completed in 240 iterations
run_example(with_extra_load=true)
Completed in 162 iterations
thread 'test_outcomes_should_be_the_same' panicked at 'assertion failed: `(left == right)`
  left: `[(0, 0), (0, 1), (1, 0), (1, 1)]`,
 right: `[(0, 0), (0, 1), (1, 1)]`', src/lib.rs:12:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Note that the test with an additional operation has fewer iterations. It misses the outcome (value=1, key=1), which could be generated for example by the following interleaving of the two threads:

t1:   entry1.key.store(1, SeqCst);
t1:   entry1.value.store(1, SeqCst);
t2:   let value = entry2.value.load(SeqCst);  // 1
t2:   let key = entry2.key.load(SeqCst);      // 1
t1:   entry1.value.store(0, SeqCst);
t1:   entry1.key.load(SeqCst);                // 1
t1:   entry1.key.store(0, SeqCst);
t2:   return (value, key);                    // (1, 1)

This is the smallest example I could come up with - minimal tweaks (for example changing the order of reading key and value) seem to make the problem go away.

Tested on Loom 0.3.5.