[New Lint Request] If possible, suggest releasing sync locks early

[c410-f3r]

Problem

Code that heavily depends on synchronous primitives like Mutex can suffer unnecessary resource contention if the lock is only released at the end of its scope through Drop but could in fact be released early.

fn example_1() {
  let locked = some_sync_resource.lock();
  let owned_rslt = locked.do_stuff_with_resource();
  // Only `owned_rslt` is needed but `locked` is still held
  do_heavy_computation_that_takes_time(owned_rslt);
}

fn example_2() {
  let locked = some_sync_resource.lock();
  let reference = locked.get_inner_resource_ref();
  let _ = reference.do_stuff();
  let _ = reference.do_other_things();
  // From this point forward, `locked` is not needed anymore
  let _ = compute_foo();
  let _ = compute_bar();
  let _ = compute_baz();
}

Request

A new lint called sync_lock_drop that asks for an explicit "inline" expression or an explicit drop call. The above snippets would then be rewritten as follows:

fn example_1() {
  let owned_rslt = some_sync_resource.lock().do_stuff_with_resource();
  do_heavy_computation_that_takes_time(owned_rslt);
}

fn example_2() {
  let locked = some_sync_resource.lock();
  let reference = locked.get_inner_resource_ref();
  let _ = reference.do_stuff();
  let _ = reference.do_other_things();
  drop(locked);
  let _ = compute_foo();
  let _ = compute_bar();
  let _ = compute_baz();
}

Implementation

It is trivial to detect synchronous primitives of the standard library but third-party implementations are tricky to detect. What should be done here? Hard code a list of well-known crates or allow users to provide a list of custom types?

[flip1995]

To implement this, the has_significant_drop attribute, that can be attached to types could be used, like the clippy::significant_drop_in_scrutinee lint does.

However, I marked it as E-hard because keeping track of references build from the locked variables. So checking for the last use of locked should be easy. But checking that reference (or other bindings) aren't used anymore might be hard.

[c410-f3r]

@rustbot claim

[Thomasdezeeuw]

I've hit a false positive, or at least a bad suggestion, with this lint using clippy 0.1.72 (f4b80ca 2023-06-30).

The related code: https://github.com/Thomasdezeeuw/heph/blob/8725fe9ed871f135b7ab87b5969eb1213121a242/inbox/src/lib.rs#L523-L529

For which Clippy suggests:

error: temporary with significant `Drop` can be early dropped
   --> inbox/src/lib.rs:523:21
    |
522 |           if let Some(waker) = self.registered_waker.take() {
    |  ___________________________________________________________-
523 | |             let mut sender_wakers = self.channel.sender_wakers.lock().unwrap();
    | |                     ^^^^^^^^^^^^^
524 | |             let idx = sender_wakers.iter().position(|w| w.will_wake(&waker));
525 | |             if let Some(idx) = idx {
...   |
529 | |             }
530 | |         }
    | |_________- temporary `sender_wakers` is currently being dropped at the end of its contained scope
    |
    = note: this might lead to unnecessary resource contention
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#significant_drop_tightening
    = note: `-D clippy::significant-drop-tightening` implied by `-D clippy::nursery`
help: merge the temporary construction with its single usage
    |
523 ~
524 +             let idx = self.channel.sender_wakers.lock().unwrap().position(|w| w.will_wake(&waker));
    |
help: remove separated single usage
    |
524 -             let idx = sender_wakers.iter().position(|w| w.will_wake(&waker));
524 +
    |

It doesn't seem to realize that sender_wakers is used twice, once to iterate over (line 524) and then to mutate it (line 526).

[c410-f3r]

At least for f4b80ca, it seems that the Rust project didn't catch up with the latest changes of this lint (https://github.com/rust-lang/rust/blob/f4b80cacf93ca216c75f6ae12f4b9dec19eba42f/src/tools/clippy/clippy_lints/src/significant_drop_tightening.rs).

Can you create another issue if the same behavior continues after the next Clippy update?