Rust's Sneaky Deadlock With `if let` Blocks

Post by simonask » Wed, Nov 13, 2024, 8:07 AM UTC

sunshowers wrote: post #55

ajross wrote: post #48

Earlier quoted context omitted.

That's arguably true, but only insofar as it makes sense to talk about a mutex "owning" "data". There are many kinds of data that can't be owned by the language runtime like this (think about async/shared/DMA buffers, register blocks on foreign hardware, memory-mapped database files), but that still clearly need synchronization. Even things like coarse-grained locks taken at the subsystem level (e.g. to avoid having…

That is true, but I don't believe Rust imposes a cost here. The worst case degrades to a `Mutex ` and then whatever's being guarded being managed separately.

Post by kelnos » Wed, Nov 13, 2024, 8:17 AM UTC

NobodyNada wrote: post #34

anderskaseorg wrote: post #30

Earlier quoted context omitted.

Yes, allowing this to execute would be very unsound: let lock = RwLock::new(Box::new(111)); let r: &i32 = &**lock.read().unwrap(); // points to 111 *lock.write().unwrap() = Box::new(222); // allocates a new Box and deallocates 111 println!("{}", *r); // use after free

It can be done safely with an upgrade method that requires an owned read guard. The RwLock implementation provided by the parking_lot crate supports this safely: let lock = RwLock::new(Box::new(111)); let read = lock.upgradable_read(); let r: &i32 = &**read; // points to 111 *RwLockUpgradableReadGuard::upgrade(read) = Box::new(222); // error[E0505]: cannot move out of `read` because it is borrowed println!("{}", *r);

Post by GolDDranks » Wed, Nov 13, 2024, 8:21 AM UTC

the_duke wrote: post #56

anderskaseorg wrote: post #18

Clippy already has an error for this pattern with Mutex. It should be trivial to extend it to cover RwLock. error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock --> src/main.rs:5:5 | 5 | if let Some(num) = *map.lock().unwrap() { | ^ --- this Mutex will remain locked for the entire `if let`-block... | _____| | | 6 | | eprintln!("There's a number in there: {num}"); 7 | | } else { 8 |…

There are a lot of alternative lock implementations that are used all the time, with the most common ones probably being tokios RwLock/Mutex and parking_lot. That lint won't help for those.

Post by kelnos » Wed, Nov 13, 2024, 8:24 AM UTC

dathinab wrote: post #11

CJefferson wrote: post #8

Earlier quoted context omitted.

There are however many PRs about people ignoring the return values of pthread_mutex_lock(), or missing an unlock on one route through a function.

and nothing prevents you from explicitly dropping lock guards in rust either actually it's a common pattern in complicated multi threaded code or similar to require that

Post by SkiFire13 » Wed, Nov 13, 2024, 8:31 AM UTC

nlitened wrote: post #58

While I never dove deep into Rust, until now I have kinda naively expected for some reason that Rust's lifetimes and ownership model prevents these trivial deadlocks at compile time. Thinking about it now, there's no reason why it would. Still lots of footguns. Also, from my experience, acquiring and releasing a mutex multiple times within a single code path feels to me like a smelly, subtly faulty code. Are there le…