Repository: rust
Issue: RwLock is `UnwindSafe` despite not poisoning on `read()`, can cause broken invariants
Labels: A-concurrency,T-libs-api,C-bug

`RwLock` only poisons on panic with `write()`, it explicitly does not poison on panic with `read()`. This makes sense for most types, but for types with interior mutability `read()` can expose that interior mutability and allow broken invariants to be witnessed without using `AssertUnwindSafe` or spawning threads.

If I understand this correctly, due to the lack of read poisoning, it really should only be `UnwindSafe where T: UnwindSafe` (and I believe `RefUnwindSafe where T: RefUnwindSafe`, though I'm not certain as I'm still struggling to properly conceptualize this.).

On a similar note, `RwLockReadGuard` is `UnwindSafe` and `RefUnwindSafe` without any conditions. So is `RwLockWriteGuard`, but that's the one that does poisoning so it's fine there. `RwLockReadGuard` should at least require `T: RefUnwindSafe` for it to be `UnwindSafe` and `RefUnwindSafe`. I believe that if `RwLock` is adjusted then `RwLockReadGuard` will pick it up automatically due to the auto trait rules, though in this case `RwLockWriteGuard` will need the manual implementations as it does poisoning.

---

I'm not sure if there's any quick go-to for testing unwind safety, but I wrote a simple type that maintains a logical invariant with interior mutability. Without `RwLock` I get the expected compiler error trying to pass a reference to it across a `catch_unwind` barrier. Wrapping it in `RwLock` gets rid of the compiler error without introducing poisoning, which then allows for observing a broken invariant. [Playground link](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8183062573ba2f99799543457b215752)

```rust
use std::cell::RefCell;

struct Foo {
    // Invariant: inner is always Some, it's only teporarily None during transformation.
    inner: RefCell<Option<String>>,
}

impl Foo {
    fn new() -> Self {
        Foo {
            inner: RefCell::new(Some("initial".to_owned())),
        }
    }

    fn transform(&self, f: impl FnOnce(String) -> String) {
        let inner = self.inner.borrow_mut().take().unwrap();
        *self.inner.borrow_mut() = Some(f(inner));
    }

    fn inner(&self) -> std::cell::Ref<str> {
        std::cell::Ref::map(self.inner.borrow(), |inner| {
            inner.as_deref().expect("broken invariant")
        })
    }
}

fn main() {
    let foo = Foo::new();
    
    // Comment out the next two lines to get an unwind safety error
    let foo = std::sync::RwLock::new(foo);
    let foo = foo.read().unwrap();
    
    dbg!(foo.inner());
    let result = std::panic::catch_unwind(|| {
        foo.transform(|_| panic!());
    });
    let _ = dbg!(result);
    dbg!(foo.inner());
}
```

In this code I'm passing the `RwLockReadGuard` across the `catch_unwind` barrier, but passing the `&RwLock` across and calling `.read().unwrap()` on each access produces the same results (meaning this can't just be fixed by changing `RwLockReadGuard`).

### Meta
Playground rust version: Stable channel, 1.55.0

### Additional context
I discovered this when I was trying to figure out how to make a type with interior mutability safe to pass to something that requires `RefUnwindSafe`. I thought "it has interior mutability, maybe I should just use a `RwLock` and only take reads on it", at which point I discovered that this doesn't add poisoning despite being `RefUnwindSafe`. In retrospect, reading can't add poisoning (given that there can be concurrent reads, and you can't poison an extant guard) so I really do need a mutex, but the fact that `RwLock` makes the compiler happy here is a problem.