tokio-rs / loom

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

`JoinHandle.join()` triggers unexpected panic `assertion failed: state.notified` #249

Open faern opened 2 years ago

faern commented 2 years ago

I'm trying to add loom testing into my crate triggered. A simple test gives me errors I can't debug.

Test code:

use loom::thread;

#[test]
fn wait_and_trigger() {
    loom::model(|| {
        let (trigger, listener) = triggered::trigger();

        let thread_handle = thread::spawn(move || {
            //thread::yield_now();
            trigger.trigger();
        });

        //thread::yield_now();
        listener.wait();

        // Uncomment any of the `yield_now` calls or remove this join
        // to make the test pass.
        thread_handle.join().expect("Trigger thread panic");
    })
}

This test and a very slimmed down version of the library is available here: https://github.com/faern/triggered/blob/debug-loom-issue/src/lib.rs. Makes it easier to read both the test and the code it tests in a single file.

Results:

$ RUSTFLAGS="--cfg loom" cargo test
...
running 1 test
test wait_and_trigger ... FAILED

failures:

---- wait_and_trigger stdout ----
thread 'wait_and_trigger' panicked at 'assertion failed: state.notified', /home/faern/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.4/src/rt/notify.rs:120:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Inserting a thread::yield_now() call in some places makes the test succeed. Also, not caring about joining the thread also makes the test succeed. Both of these things makes me think the bug is in loom, not my library. Since if my library had a bug, loom should be able to find it even with those changes.

Expected results:

  1. It should not be a problem to join the thread in this case.
  2. The existence of a yield_now should not affect the test results.

Related

Possibly related to #177? I don't think I have non-determinism here? At least not in a similar way as described in that issue. But the panic seems to be in a similar place.

hawkw commented 2 years ago

I've also hit this, though I didn't have the time to put together a minimal reproduction --- thanks for that!

wyfo commented 1 year ago

@hawkw I also encountered this issue, but my minimal reproduction example is fairly minimal:

use loom::thread;
#[test]
fn loom_panic() {
    loom::model(|| {
        let main = thread::current();
        let other = thread::spawn(move || main.unpark());
        // thread::park();
        other.join().unwrap();
    })
}

Seems that loom cannot handle unparking if there is no parking, because uncommenting thread::park(); makes loom happy.