Deadlock with Rwlock and Thread with version 1.62 and 1.63

[vireshk]

I tried this code:

use std::sync::{Arc, RwLock};
use std::thread::spawn;

fn main() {
    let count = 2;
    let data = Arc::new(RwLock::new(vec![Some(true); count]));

    for i in 0..count {
        let data = data.clone();

        Some(spawn(move || {
            data.write().unwrap()[i] = None;
        }));
    }

    // This hangs a lot, since 1.62.0 (1.61 works fine)
    while data.read().unwrap()[0].is_some()
        || data.read().unwrap()[1].is_some()
    {}

    // This always passes
    while data.read().unwrap()[0].is_some() {}
    while data.read().unwrap()[1].is_some() {}

    dbg!();
}

I expected to see this happen: print [src/main.rs:25]

Instead, this happened: but it hangs

Meta

rustc --version --verbose:

rustc 1.63.0 (4b91a6ea7 2022-08-08)
binary: rustc
commit-hash: 4b91a6ea7258a947e59c6522cd5898e7c0a6a88f
commit-date: 2022-08-08
host: x86_64-unknown-linux-gnu
release: 1.63.0
LLVM version: 14.0.5

[m-ou-se]

That's because your first while loop attempts to acquire two read locks at the same time, which can result in a deadlock. There is no guarantee that you can always acquire a second read lock if you already have one. Many RwLock implementations block new readers when there's a writer waiting.

(It's perhaps a bit surprising that || keeps the temporaries on the left side alive while evaluating the right one, since it only needs the bool result.)

[vireshk]

I was expecting here that the first lock gets dropped as soon as we reach || and so the second one will be a fresh attempt.

[m-ou-se]

That's a very reasonable expectation, but unfortunately not what happens. Temporaries stay around until the end of the whole expression. It sometimes results in confusing situations with lock guards, just like in your snippet. It'd be nice if we can do something about this, but it's very tricky to change the language rules without breaking any existing code, or accidentally making the rules more confusing. Perhaps we just need a lint (in Clippy, or Rustc) that warns about confusing situations like this.

[m-ou-se]

FWIW, the 'right' way to write that while loop would be something like this, by only locking once:

    while {
        let d = data.read().unwrap();
        d[0].is_some() || d[1].is_some()
    } {
        …
    }

[vireshk]

From a common programmer's point of view, looks like something is broken somewhere since the compiler didn't complain at all. But I do understand what you are saying @m-ou-se . Thanks for quick response, really appreciate it.

Lets see if someone can come up with a solution to this.

[vireshk]

    while {
        let d = data.read().unwrap();
        d[0].is_some() || d[1].is_some()
    } {

Hmm, I wasn't aware that we can write while's statement this way, like body i.e.. Interesting.

[Noratrieb]

Closing this as not a bug. If you think the drop order of || statements is confusing, feel free to discuss that further on zulip or internals.rust-lang.org (or by opening a new issue).