Panic when accessing loom exectution state from spawned thread during unwinding

[e00E]

#[test]
#[should_panic]
fn foo() {
    struct S(loom::cell::UnsafeCell<()>);

    impl Drop for S {
        fn drop(&mut self) {
            self.0.get();
        }
    }

    loom::model(|| {
        let s = S(loom::cell::UnsafeCell::new(()));
        loom::thread::spawn(move || {
            drop(s);
        });
        panic!();
    });
}

The expected result of this test is that it passes because it is marked should_panic and explicitly panics.

The actual result is that the test aborts because a second panic occurs during unwinding of the first, explicit panic. The second panic is a bug.

running 1 test
thread 'foo' panicked at tests/foo.rs:17:9:
explicit panic
stack backtrace:
...
thread 'foo' panicked at /home/e/temp/loom/src/rt/scheduler.rs:128:13:
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`)?
stack backtrace:
...
loom::cell::unsafe_cell::UnsafeCell<T>::get
...
thread 'foo' panicked at library/core/src/panicking.rs:163:5:
panic in a destructor during cleanup
thread caused non-unwinding panic. aborting.

The bug does not occur in the following variations:

• There is no second thread. S is still dropped in the main thread during unwinding.
• S is created in the spawned thread instead of moving there.

I ran into this while working on https://github.com/tokio-rs/loom/issues/349 .

[Darksonn]

Please keep in mind that the contents of loom::model generally runs many times to try all different ways that the threads can be interleaved. It doesn't just run once.

Because of that, #[should_panic] tests are out of scope for loom.

You may be able to use catch_unwind to catch the panic inside the loom test, and then assert that a panic happened.

[e00E]

There are a bunch of should_panic tests in loom's own test suite. I ran into this issue through one of those tests. If should_panic isn't supported, then we should rewrite them.

[e00E]

To be more precise, I think you're saying that panicking across loom::model isn't supported. Could we document that?

[Darksonn]

Oh, I see, they are loom's own tests. That's a different problem then. It's a known problem that loom will double-panic in some situations. It sounds like the UnsafeCell::drop change is introducing more. That's okay. You can remove the offending test.