Using `mutex.lock()?` risks deadlocks due to `PoisonError` holding the mutex guard

When using methods like Mutex::lock in functions that return a type like Box<dyn Error>, using lock()? can lead to the PoisonError, which contains the lock guard, being held for longer than anticipated.

This is a little bit of a footgun and some basic code search (thanks @bjorn3!) shows that people are actually writing lock()? in their code (note that many of these are File::lock or other unrelated methods).

Also applies to RwLock::write and RwLock::read.

Demo of a deadlock from a PoisonError returned as Box<dyn Error>:

use std::{error::Error, sync::Mutex};

fn main() {
    // poison the mutex
    std::panic::catch_unwind(|| {
        let _lock = MUTEX.lock();
        panic!();
    })
    .unwrap_err();

    let _result = wat(); // lock is contained in PoisonError
    _ = wat(); // deadlock
}

static MUTEX: Mutex<i32> = Mutex::new(1);

fn wat() -> Result<(), Box<dyn Error>> {
    *MUTEX.lock()? += 1;
    Ok(())
}

See also zulip.

imo there should be a lint against any code that does anything nontrivial to a PoisonError containing a lock guard, but I'm not sure what that would look like.

@rustbot label C-discussion
cc #149359

Start by reading the Mutex::lock, RwLock::write, and RwLock::read behavior described in the issue, then review the linked Zulip discussion and related issue #149359. The desired lint behavior and scope are not decided yet, so completion would require an agreed design before implementation.