Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[new-lint] if let Some... else with locked mutex #5219

Closed
DevinR528 opened this issue Feb 23, 2020 · 3 comments · Fixed by #5332
Closed

[new-lint] if let Some... else with locked mutex #5219

DevinR528 opened this issue Feb 23, 2020 · 3 comments · Fixed by #5332
Labels
A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group

Comments

@DevinR528
Copy link
Contributor

DevinR528 commented Feb 23, 2020

I was working on a project and did something like this

let mutex: Mutex<Option<...>>;
if let Some(thing) = mutex.lock().unwrap() {
    do_thing();
} else {
    mutex.lock();
}

It just sat running and it took me a bit to realize the mutex is locked for the entire scope of the if and else. The suggestion could be something like

warning: locking a `Mutex` twice in the same scope will (fail/freeze/deadlock)
if let Some(thing) = mutex.lock().unwrap() {  <=== lock starts here
    do_thing();                               <=
} else {                                      <=
    mutex.lock();      <= second lock         <= 
}                                             <= lock is droped

or a suggestion to

warning: move lock out of `if let...else`
let locked = mutex.lock().unwrap();
if let Some(thing) = locked {
    do_thing();
} else {
    do_something_else(locked);
}  

I would be happy to start a PR if this sounds good to anyone else!

@flip1995 flip1995 added L-correctness Lint: Belongs in the correctness lint group A-lint Area: New lints labels Feb 23, 2020
@DevinR528
Copy link
Contributor Author

DevinR528 commented Feb 25, 2020

I should be using the latest nightly for testing a PR right? Is there a branch that is rustc 1.43.0-nightly current to start working from or is this a case where I should start with an update PR? @matthiaskrgr where did you start your PR from your rustc_pass_arg branch seems to be current with the removal of rustc_infer?

@flip1995
Copy link
Member

Clippy is compiled with the latest master commit of rustc, not with the latest nightly. (I think the 2020-02-25 nightly should work right now, though)

The setup-toolchain.sh script will set up the master toolchain for you.

bors added a commit that referenced this issue Apr 20, 2020
If let else mutex

changelog: Adds lint to catch incorrect use of `Mutex::lock` in `if let` expressions with lock calls in any of the blocks.

closes: #5219
@bors bors closed this as completed in 6ce05bf Apr 20, 2020
@viruscamp
Copy link

failed on

    let vec_mutex = Mutex::new(vec![2,3]);
    if let Some(num) = vec_mutex.lock().unwrap().pop() {
        if num == 2 {
            vec_mutex.lock().unwrap().push(4);
        }
        println!("got {num}");
    };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants