rust-lang / rust-lang/rust-clippy
Example code in `significant_drop_in_scrutinee` documentation gives bad advice
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 13.5k
- Forks
- 2.2k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 32
Description
The documentation for the significant_drop_in_scrutinee lint currently suggests that code like this...
let mutex = Mutex::new(State {});
match mutex.lock().unwrap().foo() {
true => {
mutex.lock().unwrap().bar(); // Deadlock!
}
false => {}
};
ought to be rewritten like this...
let mutex = Mutex::new(State {});
let is_foo = mutex.lock().unwrap().foo(); // lock taken and then released
match is_foo {
true => {
mutex.lock().unwrap().bar(); // lock taken again
}
false => {}
};
This is very likely to have fixed the deadlock by introducing a TOCTOU race. The result of the foo predicate is probably only reliable for as long as the code that called foo continues to hold the lock -- meaning that, by the time the call to bar happens, it might not be appropriate to call bar anymore! Even if this isn't the case, locking a mutex is expensive, so dropping a lock only to reclaim it again almost immediately is almost always the wrong choice for performance's sake.
A better suggestion would be to make the guard's lifetime explicit:
let mutex = Mutex::new(State {});
{
let state = mutex.lock().unwrap();
match state.foo() {
true => state.bar(),
false => {}
}
} // lock released here
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked significant_drop_in_scrutinee documentation entry and review the two mutex examples and their explanation. Update the guidance to demonstrate an explicit guard lifetime rather than dropping and immediately reacquiring the lock; done means the example no longer recommends a potentially unsafe or unnecessarily expensive pattern.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- documentation
- Issue type
- Documentation
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100