forked from rust-lang/rust-clippy
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Lint for holding locks across await points
Fixes rust-lang#4226 This introduces the lint await_holding_lock. For async functions, we iterate over all types in generator_interior_types and look for types named MutexGuard, RwLockReadGuard, or RwLockWriteGuard. If we find one then we emit a lint.
- Loading branch information
Showing
6 changed files
with
189 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
use crate::utils::span_lint_and_note; | ||
use if_chain::if_chain; | ||
use rustc_hir::intravisit::FnKind; | ||
use rustc_hir::{Body, FnDecl, HirId, IsAsync}; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
use rustc_span::{Span, Symbol}; | ||
|
||
declare_clippy_lint! { | ||
/// **What it does:** Checks for calls to await while holding a MutexGuard. | ||
/// | ||
/// **Why is this bad?** This is almost certainly an error which can result | ||
/// in a deadlock because the reactor will invoke code not visible to the | ||
/// currently visible scope. | ||
/// | ||
/// **Known problems:** Detects only specifically named guard types: | ||
/// MutexGuard, RwLockReadGuard, and RwLockWriteGuard. | ||
/// | ||
/// **Example:** | ||
/// | ||
/// ```rust | ||
/// use std::sync::Mutex; | ||
/// | ||
/// async fn foo(x: &Mutex<u32>) { | ||
/// let guard = x.lock().unwrap(); | ||
/// *guard += 1; | ||
/// bar.await; | ||
/// } | ||
/// ``` | ||
/// Use instead: | ||
/// ```rust | ||
/// use std::sync::Mutex; | ||
/// | ||
/// async fn foo(x: &Mutex<u32>) { | ||
/// { | ||
/// let guard = x.lock().unwrap(); | ||
/// *guard += 1; | ||
/// } | ||
/// bar.await; | ||
/// } | ||
/// ``` | ||
pub AWAIT_HOLDING_LOCK, | ||
pedantic, | ||
"Inside an async function, holding a MutexGuard while calling await" | ||
} | ||
|
||
const MUTEX_GUARD_TYPES: [&str; 3] = ["MutexGuard", "RwLockReadGuard", "RwLockWriteGuard"]; | ||
|
||
declare_lint_pass!(AwaitHoldingLock => [AWAIT_HOLDING_LOCK]); | ||
|
||
impl LateLintPass<'_, '_> for AwaitHoldingLock { | ||
fn check_fn( | ||
&mut self, | ||
cx: &LateContext<'_, '_>, | ||
fn_kind: FnKind<'_>, | ||
_: &FnDecl<'_>, | ||
_: &Body<'_>, | ||
span: Span, | ||
_: HirId, | ||
) { | ||
if !is_async_fn(fn_kind) { | ||
return; | ||
} | ||
|
||
for ty_clause in &cx.tables.generator_interior_types { | ||
if_chain! { | ||
if let rustc_middle::ty::Adt(adt, _) = ty_clause.ty.kind; | ||
if let Some(&sym) = cx.get_def_path(adt.did).iter().last(); | ||
if is_symbol_mutex_guard(sym); | ||
then { | ||
span_lint_and_note( | ||
cx, | ||
AWAIT_HOLDING_LOCK, | ||
ty_clause.span, | ||
"this MutexGuard is held across an 'await' point", | ||
ty_clause.scope_span.unwrap_or(span), | ||
"these are all the await points this lock is held through" | ||
); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
fn is_async_fn(fn_kind: FnKind<'_>) -> bool { | ||
fn_kind.header().map_or(false, |h| match h.asyncness { | ||
IsAsync::Async => true, | ||
IsAsync::NotAsync => false, | ||
}) | ||
} | ||
|
||
fn is_symbol_mutex_guard(sym: Symbol) -> bool { | ||
let sym_str = sym.as_str(); | ||
for ty in &MUTEX_GUARD_TYPES { | ||
if sym_str == *ty { | ||
return true; | ||
} | ||
} | ||
false | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// edition:2018 | ||
#![warn(clippy::await_holding_lock)] | ||
|
||
use std::sync::Mutex; | ||
|
||
async fn bad(x: &Mutex<u32>) -> u32 { | ||
let guard = x.lock().unwrap(); | ||
baz().await | ||
} | ||
|
||
async fn good(x: &Mutex<u32>) -> u32 { | ||
{ | ||
let guard = x.lock().unwrap(); | ||
let y = *guard + 1; | ||
} | ||
baz().await; | ||
let guard = x.lock().unwrap(); | ||
47 | ||
} | ||
|
||
async fn baz() -> u32 { | ||
42 | ||
} | ||
|
||
async fn also_bad(x: &Mutex<u32>) -> u32 { | ||
let first = baz().await; | ||
|
||
let guard = x.lock().unwrap(); | ||
|
||
let second = baz().await; | ||
|
||
let third = baz().await; | ||
|
||
first + second + third | ||
} | ||
|
||
fn main() { | ||
let m = Mutex::new(100); | ||
good(&m); | ||
bad(&m); | ||
also_bad(&m); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
error: this MutexGuard is held across an 'await' point | ||
--> $DIR/await_holding_lock.rs:7:9 | ||
| | ||
LL | let guard = x.lock().unwrap(); | ||
| ^^^^^ | ||
| | ||
= note: `-D clippy::await-holding-lock` implied by `-D warnings` | ||
note: these are all the await points this lock is held through | ||
--> $DIR/await_holding_lock.rs:7:5 | ||
| | ||
LL | / let guard = x.lock().unwrap(); | ||
LL | | baz().await | ||
LL | | } | ||
| |_^ | ||
|
||
error: this MutexGuard is held across an 'await' point | ||
--> $DIR/await_holding_lock.rs:28:9 | ||
| | ||
LL | let guard = x.lock().unwrap(); | ||
| ^^^^^ | ||
| | ||
note: these are all the await points this lock is held through | ||
--> $DIR/await_holding_lock.rs:28:5 | ||
| | ||
LL | / let guard = x.lock().unwrap(); | ||
LL | | | ||
LL | | let second = baz().await; | ||
LL | | | ||
... | | ||
LL | | first + second + third | ||
LL | | } | ||
| |_^ | ||
|
||
error: aborting due to 2 previous errors | ||
|