diff --git a/src/rt/scheduler.rs b/src/rt/scheduler.rs index a9fed80..4d6fbdd 100644 --- a/src/rt/scheduler.rs +++ b/src/rt/scheduler.rs @@ -127,7 +127,41 @@ impl Scheduler { panic!("cannot access Loom execution state from outside a Loom model. \ are you accessing a Loom synchronization primitive from outside a Loom test (a call to `model` or `check`)?") } - STATE.with(|state| f(&mut state.borrow_mut())) + STATE.with(|state| { + // When unwinding after a panic, `STATE` is unset before `Scheduler` is dropped. + // However, `Scheduler::queued_spawn` could hold loom objects which would try to use + // `STATE` when they are dropped. Because of that, we need to clear `queued_spawn` + // while STATE is still set. Since `STATE` has an exclusive reference (&mut) to + // `Scheduler::queued_spawn`, we also need to use `STATE` ourselves for accessing them, + // but drop our `RefMut` before the dropping of `queued_spawn` happens. + // + // To implement this, we exploit the fact that the struct fields of `DropGuard` + // are dropped in declaration order, and move `queued_spawn`in `DropGuard::drop` + // to the second struct field of `DropGuard` (replacing it with an empty `VecDeque`). + // Then, the destructor first drops the `RefMut` (thereby allowing `STATE` to be + // borrowed again), and then the former `queued_spawn` value (possibly accessing `STATE`). + struct DropGuard<'a, 'b>( + std::cell::RefMut<'a, State<'b>>, + VecDeque>, + ); + impl<'a, 'b> Drop for DropGuard<'a, 'b> { + fn drop(&mut self) { + if std::thread::panicking() { + self.1 = std::mem::take(self.0.queued_spawn); + } + } + } + impl<'a, 'b> DropGuard<'a, 'b> { + fn run(&mut self, f: F) -> R + where + F: FnOnce(&mut State<'b>) -> R, + { + f(&mut self.0) + } + } + let mut guard = DropGuard(state.borrow_mut(), Default::default()); + guard.run(f) + }) } } diff --git a/tests/arc.rs b/tests/arc.rs index 62cf0d1..1bf8e47 100644 --- a/tests/arc.rs +++ b/tests/arc.rs @@ -128,3 +128,26 @@ fn try_unwrap_multithreaded() { let _ = Arc::try_unwrap(num).unwrap(); }); } + +/// Test that there is no double panic +/// when a model with an Arc in an unspawned thread panics. +#[test] +#[should_panic(expected = "loom should not panic inside another panic")] +fn access_on_drop_during_panic_in_unspawned_thread() { + use loom::sync::Arc; + use std::panic::catch_unwind; + + let result = catch_unwind(|| { + loom::model(move || { + let arc = Arc::new(()); + thread::spawn(move || { + let _arc = arc; + }); + panic!(); + }); + }); + + // propagate the panic from the spawned thread + // to the main thread. + result.expect("loom should not panic inside another panic"); +}