tokio-rs / loom

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

Internal Panic: Index Out of Bounds #290

Open jswrenn opened 1 year ago

jswrenn commented 1 year ago

The following program:

use loom::cell::Cell;

thread_local! {
    static ACTIVE_FRAME: Cell<()> = Cell::new(());
}

fn main() {
    loom::model(|| {
        let handle_a = loom::thread::spawn(|| {
            let _ = ACTIVE_FRAME.with(Cell::get);
        });
        let handle_b = loom::thread::spawn(|| ());
        handle_a.join().unwrap();
        handle_b.join().unwrap();
    });
}

...panics with:

thread 'main' panicked at 'index out of bounds: the len is 2 but the index is 2', /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/object.rs:286:25
stack backtrace:
   0: rust_begin_unwind
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/std/src/panicking.rs:556:5
   1: core::panicking::panic_fmt
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/core/src/panicking.rs:142:14
   2: core::panicking::panic_bounds_check
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/core/src/panicking.rs:84:5
   3: <usize as core::slice::index::SliceIndex<[T]>>::index_mut
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/core/src/slice/index.rs:259:14
   4: core::slice::index::<impl core::ops::index::IndexMut<I> for [T]>::index_mut
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/alloc/src/vec/mod.rs:2640:9
   5: <alloc::vec::Vec<T,A> as core::ops::index::IndexMut<I>>::index_mut
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/alloc/src/vec/mod.rs:2640:9
   6: loom::rt::object::Ref<T>::get_mut
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/object.rs:286:25
   7: loom::rt::cell::Cell::start_read::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/cell.rs:58:25
   8: loom::rt::synchronize::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/mod.rs:141:9
   9: loom::rt::scheduler::Scheduler::with_execution::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/scheduler.rs:48:34
  10: loom::rt::scheduler::Scheduler::with_state::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/scheduler.rs:130:28
  11: scoped_tls::ScopedKey<T>::with
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:171:13
  12: loom::rt::scheduler::Scheduler::with_state
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/scheduler.rs:130:9
  13: loom::rt::scheduler::Scheduler::with_execution
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/scheduler.rs:48:9
  14: loom::rt::execution
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/mod.rs:171:5
  15: loom::rt::synchronize
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/mod.rs:138:5
  16: loom::rt::cell::Cell::start_read
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/cell.rs:57:9
  17: loom::cell::unsafe_cell::UnsafeCell<T>::with
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/cell/unsafe_cell.rs:134:24
  18: loom::cell::cell::Cell<T>::get
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/cell/cell.rs:59:9
  19: core::ops::function::FnOnce::call_once
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/core/src/ops/function.rs:251:5
  20: std::thread::local::LocalKey<T>::try_with
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/std/src/thread/local.rs:446:16
  21: std::thread::local::LocalKey<T>::with
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/std/src/thread/local.rs:422:9
  22: loom_issue::main::{{closure}}::{{closure}}
             at ./src/main.rs:10:21
  23: loom::thread::spawn_internal::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/thread.rs:161:47
  24: loom::rt::spawn::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/mod.rs:76:9
  25: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/core/src/ops/function.rs:251:5
  26: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/alloc/src/boxed.rs:1938:9
  27: loom::rt::scheduler::spawn_threads::{{closure}}::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/loom-0.5.6/src/rt/scheduler.rs:149:21
  28: generator::gen_impl::GeneratorImpl<A,T>::init_code::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/generator-0.7.1/src/gen_impl.rs:344:21
  29: generator::stack::StackBox<F>::call_once
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/generator-0.7.1/src/stack/mod.rs:139:13
  30: generator::stack::Func::call_once
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/generator-0.7.1/src/stack/mod.rs:121:9
  31: generator::gen_impl::gen_init::{{closure}}
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/generator-0.7.1/src/gen_impl.rs:560:9
  32: core::ops::function::FnOnce::call_once
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/core/src/ops/function.rs:251:5
  33: std::panicking::try::do_call
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/std/src/panicking.rs:464:40
  34: ___rust_try
  35: std::panicking::try
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/std/src/panicking.rs:428:19
  36: std::panic::catch_unwind
             at /rustc/ce7f0f1aa0f02c45cad0749e63f3086234b1f422/library/std/src/panic.rs:137:14
  37: generator::gen_impl::catch_unwind_filter
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/generator-0.7.1/src/gen_impl.rs:551:5
  38: generator::gen_impl::gen_init
             at /Users/jswrenn/.cargo/registry/src/github.com-1ecc6299db9ec823/generator-0.7.1/src/gen_impl.rs:578:25

The issue does not occur if loom::thread_local! (but unfortunately it took me several days to realize that loom even provided a mocked version of thread_local! and that I might want to use it).

carllerche commented 1 year ago

The issue is the loom Cell is being reused across loom runs. Fundamentally, all loom objects must be created in each iteration of the model. This is why switching to a loom thread-local fixes it.

The best we can do is catch this and panic w/ a better error message. Off the top of my head, each iteration of the model could get a unique identifier. Loom objects could then include the iteration identifier and panic if the object's identifier doesn't match the runtime's identifier.

Is this something you want to look into doing?

alexkeizer commented 1 year ago

This seems like an easy enough issue for a new contributor to pick up. I would like to give it a try.

From a cursory glance at the source code, I figured I could try the following:

Does this seem like a reasonable approach?

alexkeizer commented 1 year ago

Actually, the Execution object already has an id, which is already unique for each iteration (it is modified in Execution::step()), so we need only add code to the objects to track this existing id.