]>
Commit | Line | Data |
---|---|---|
5e7ed085 | 1 | use clippy_utils::diagnostics::span_lint_and_then; |
cdc7bbd5 | 2 | use clippy_utils::{match_def_path, paths}; |
04454e1e | 3 | use rustc_data_structures::fx::FxHashMap; |
f20569fa | 4 | use rustc_hir::def_id::DefId; |
04454e1e | 5 | use rustc_hir::{def::Res, AsyncGeneratorKind, Body, BodyId, GeneratorKind}; |
f20569fa XL |
6 | use rustc_lint::{LateContext, LateLintPass}; |
7 | use rustc_middle::ty::GeneratorInteriorTypeCause; | |
04454e1e | 8 | use rustc_session::{declare_tool_lint, impl_lint_pass}; |
f20569fa XL |
9 | use rustc_span::Span; |
10 | ||
04454e1e FG |
11 | use crate::utils::conf::DisallowedType; |
12 | ||
f20569fa | 13 | declare_clippy_lint! { |
94222f64 | 14 | /// ### What it does |
5e7ed085 | 15 | /// Checks for calls to await while holding a non-async-aware MutexGuard. |
f20569fa | 16 | /// |
94222f64 XL |
17 | /// ### Why is this bad? |
18 | /// The Mutex types found in std::sync and parking_lot | |
f20569fa XL |
19 | /// are not designed to operate in an async context across await points. |
20 | /// | |
c295e0f8 | 21 | /// There are two potential solutions. One is to use an async-aware Mutex |
f20569fa XL |
22 | /// type. Many asynchronous foundation crates provide such a Mutex type. The |
23 | /// other solution is to ensure the mutex is unlocked before calling await, | |
24 | /// either by introducing a scope or an explicit call to Drop::drop. | |
25 | /// | |
94222f64 | 26 | /// ### Known problems |
5e7ed085 FG |
27 | /// Will report false positive for explicitly dropped guards |
28 | /// ([#6446](https://github.com/rust-lang/rust-clippy/issues/6446)). A workaround for this is | |
29 | /// to wrap the `.lock()` call in a block instead of explicitly dropping the guard. | |
f20569fa | 30 | /// |
94222f64 | 31 | /// ### Example |
5e7ed085 FG |
32 | /// ```rust |
33 | /// # use std::sync::Mutex; | |
34 | /// # async fn baz() {} | |
f20569fa | 35 | /// async fn foo(x: &Mutex<u32>) { |
5e7ed085 | 36 | /// let mut guard = x.lock().unwrap(); |
f20569fa | 37 | /// *guard += 1; |
5e7ed085 FG |
38 | /// baz().await; |
39 | /// } | |
40 | /// | |
41 | /// async fn bar(x: &Mutex<u32>) { | |
42 | /// let mut guard = x.lock().unwrap(); | |
43 | /// *guard += 1; | |
44 | /// drop(guard); // explicit drop | |
45 | /// baz().await; | |
f20569fa XL |
46 | /// } |
47 | /// ``` | |
48 | /// | |
49 | /// Use instead: | |
5e7ed085 FG |
50 | /// ```rust |
51 | /// # use std::sync::Mutex; | |
52 | /// # async fn baz() {} | |
f20569fa XL |
53 | /// async fn foo(x: &Mutex<u32>) { |
54 | /// { | |
5e7ed085 | 55 | /// let mut guard = x.lock().unwrap(); |
f20569fa XL |
56 | /// *guard += 1; |
57 | /// } | |
5e7ed085 FG |
58 | /// baz().await; |
59 | /// } | |
60 | /// | |
61 | /// async fn bar(x: &Mutex<u32>) { | |
62 | /// { | |
63 | /// let mut guard = x.lock().unwrap(); | |
64 | /// *guard += 1; | |
65 | /// } // guard dropped here at end of scope | |
66 | /// baz().await; | |
f20569fa XL |
67 | /// } |
68 | /// ``` | |
a2a8927a | 69 | #[clippy::version = "1.45.0"] |
f20569fa | 70 | pub AWAIT_HOLDING_LOCK, |
5e7ed085 FG |
71 | suspicious, |
72 | "inside an async function, holding a `MutexGuard` while calling `await`" | |
f20569fa XL |
73 | } |
74 | ||
75 | declare_clippy_lint! { | |
94222f64 | 76 | /// ### What it does |
5e7ed085 | 77 | /// Checks for calls to await while holding a `RefCell` `Ref` or `RefMut`. |
f20569fa | 78 | /// |
94222f64 XL |
79 | /// ### Why is this bad? |
80 | /// `RefCell` refs only check for exclusive mutable access | |
f20569fa XL |
81 | /// at runtime. Holding onto a `RefCell` ref across an `await` suspension point |
82 | /// risks panics from a mutable ref shared while other refs are outstanding. | |
83 | /// | |
94222f64 | 84 | /// ### Known problems |
5e7ed085 FG |
85 | /// Will report false positive for explicitly dropped refs |
86 | /// ([#6353](https://github.com/rust-lang/rust-clippy/issues/6353)). A workaround for this is | |
87 | /// to wrap the `.borrow[_mut]()` call in a block instead of explicitly dropping the ref. | |
f20569fa | 88 | /// |
94222f64 | 89 | /// ### Example |
5e7ed085 FG |
90 | /// ```rust |
91 | /// # use std::cell::RefCell; | |
92 | /// # async fn baz() {} | |
f20569fa XL |
93 | /// async fn foo(x: &RefCell<u32>) { |
94 | /// let mut y = x.borrow_mut(); | |
95 | /// *y += 1; | |
5e7ed085 FG |
96 | /// baz().await; |
97 | /// } | |
98 | /// | |
99 | /// async fn bar(x: &RefCell<u32>) { | |
100 | /// let mut y = x.borrow_mut(); | |
101 | /// *y += 1; | |
102 | /// drop(y); // explicit drop | |
103 | /// baz().await; | |
f20569fa XL |
104 | /// } |
105 | /// ``` | |
106 | /// | |
107 | /// Use instead: | |
5e7ed085 FG |
108 | /// ```rust |
109 | /// # use std::cell::RefCell; | |
110 | /// # async fn baz() {} | |
f20569fa XL |
111 | /// async fn foo(x: &RefCell<u32>) { |
112 | /// { | |
113 | /// let mut y = x.borrow_mut(); | |
114 | /// *y += 1; | |
115 | /// } | |
5e7ed085 FG |
116 | /// baz().await; |
117 | /// } | |
118 | /// | |
119 | /// async fn bar(x: &RefCell<u32>) { | |
120 | /// { | |
121 | /// let mut y = x.borrow_mut(); | |
122 | /// *y += 1; | |
123 | /// } // y dropped here at end of scope | |
124 | /// baz().await; | |
f20569fa XL |
125 | /// } |
126 | /// ``` | |
a2a8927a | 127 | #[clippy::version = "1.49.0"] |
f20569fa | 128 | pub AWAIT_HOLDING_REFCELL_REF, |
5e7ed085 FG |
129 | suspicious, |
130 | "inside an async function, holding a `RefCell` ref while calling `await`" | |
f20569fa XL |
131 | } |
132 | ||
04454e1e FG |
133 | declare_clippy_lint! { |
134 | /// ### What it does | |
135 | /// Allows users to configure types which should not be held across `await` | |
136 | /// suspension points. | |
137 | /// | |
138 | /// ### Why is this bad? | |
139 | /// There are some types which are perfectly "safe" to be used concurrently | |
140 | /// from a memory access perspective but will cause bugs at runtime if they | |
141 | /// are held in such a way. | |
142 | /// | |
04454e1e FG |
143 | /// ### Example |
144 | /// | |
145 | /// ```toml | |
146 | /// await-holding-invalid-types = [ | |
147 | /// # You can specify a type name | |
148 | /// "CustomLockType", | |
149 | /// # You can (optionally) specify a reason | |
150 | /// { path = "OtherCustomLockType", reason = "Relies on a thread local" } | |
151 | /// ] | |
152 | /// ``` | |
153 | /// | |
154 | /// ```rust | |
155 | /// # async fn baz() {} | |
156 | /// struct CustomLockType; | |
157 | /// struct OtherCustomLockType; | |
158 | /// async fn foo() { | |
159 | /// let _x = CustomLockType; | |
160 | /// let _y = OtherCustomLockType; | |
161 | /// baz().await; // Lint violation | |
162 | /// } | |
163 | /// ``` | |
064997fb | 164 | #[clippy::version = "1.62.0"] |
04454e1e FG |
165 | pub AWAIT_HOLDING_INVALID_TYPE, |
166 | suspicious, | |
167 | "holding a type across an await point which is not allowed to be held as per the configuration" | |
168 | } | |
169 | ||
170 | impl_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF, AWAIT_HOLDING_INVALID_TYPE]); | |
171 | ||
172 | #[derive(Debug)] | |
173 | pub struct AwaitHolding { | |
174 | conf_invalid_types: Vec<DisallowedType>, | |
175 | def_ids: FxHashMap<DefId, DisallowedType>, | |
176 | } | |
177 | ||
178 | impl AwaitHolding { | |
179 | pub(crate) fn new(conf_invalid_types: Vec<DisallowedType>) -> Self { | |
180 | Self { | |
181 | conf_invalid_types, | |
182 | def_ids: FxHashMap::default(), | |
183 | } | |
184 | } | |
185 | } | |
f20569fa XL |
186 | |
187 | impl LateLintPass<'_> for AwaitHolding { | |
04454e1e FG |
188 | fn check_crate(&mut self, cx: &LateContext<'_>) { |
189 | for conf in &self.conf_invalid_types { | |
190 | let path = match conf { | |
191 | DisallowedType::Simple(path) | DisallowedType::WithReason { path, .. } => path, | |
192 | }; | |
193 | let segs: Vec<_> = path.split("::").collect(); | |
194 | if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs) { | |
195 | self.def_ids.insert(id, conf.clone()); | |
196 | } | |
197 | } | |
198 | } | |
199 | ||
f20569fa XL |
200 | fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) { |
201 | use AsyncGeneratorKind::{Block, Closure, Fn}; | |
202 | if let Some(GeneratorKind::Async(Block | Closure | Fn)) = body.generator_kind { | |
203 | let body_id = BodyId { | |
204 | hir_id: body.value.hir_id, | |
205 | }; | |
206 | let typeck_results = cx.tcx.typeck_body(body_id); | |
04454e1e | 207 | self.check_interior_types( |
f20569fa | 208 | cx, |
cdc7bbd5 | 209 | typeck_results.generator_interior_types.as_ref().skip_binder(), |
f20569fa XL |
210 | body.value.span, |
211 | ); | |
212 | } | |
213 | } | |
214 | } | |
215 | ||
04454e1e FG |
216 | impl AwaitHolding { |
217 | fn check_interior_types(&self, cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorTypeCause<'_>], span: Span) { | |
218 | for ty_cause in ty_causes { | |
219 | if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind() { | |
220 | if is_mutex_guard(cx, adt.did()) { | |
221 | span_lint_and_then( | |
222 | cx, | |
223 | AWAIT_HOLDING_LOCK, | |
224 | ty_cause.span, | |
225 | "this `MutexGuard` is held across an `await` point", | |
226 | |diag| { | |
227 | diag.help( | |
228 | "consider using an async-aware `Mutex` type or ensuring the \ | |
5e7ed085 | 229 | `MutexGuard` is dropped before calling await", |
04454e1e FG |
230 | ); |
231 | diag.span_note( | |
232 | ty_cause.scope_span.unwrap_or(span), | |
233 | "these are all the `await` points this lock is held through", | |
234 | ); | |
235 | }, | |
236 | ); | |
237 | } else if is_refcell_ref(cx, adt.did()) { | |
238 | span_lint_and_then( | |
239 | cx, | |
240 | AWAIT_HOLDING_REFCELL_REF, | |
241 | ty_cause.span, | |
242 | "this `RefCell` reference is held across an `await` point", | |
243 | |diag| { | |
244 | diag.help("ensure the reference is dropped before calling `await`"); | |
245 | diag.span_note( | |
246 | ty_cause.scope_span.unwrap_or(span), | |
247 | "these are all the `await` points this reference is held through", | |
248 | ); | |
249 | }, | |
250 | ); | |
251 | } else if let Some(disallowed) = self.def_ids.get(&adt.did()) { | |
252 | emit_invalid_type(cx, ty_cause.span, disallowed); | |
253 | } | |
f20569fa XL |
254 | } |
255 | } | |
256 | } | |
257 | } | |
258 | ||
04454e1e FG |
259 | fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedType) { |
260 | let (type_name, reason) = match disallowed { | |
261 | DisallowedType::Simple(path) => (path, &None), | |
262 | DisallowedType::WithReason { path, reason } => (path, reason), | |
263 | }; | |
264 | ||
265 | span_lint_and_then( | |
266 | cx, | |
267 | AWAIT_HOLDING_INVALID_TYPE, | |
268 | span, | |
269 | &format!("`{type_name}` may not be held across an `await` point per `clippy.toml`",), | |
270 | |diag| { | |
271 | if let Some(reason) = reason { | |
272 | diag.note(reason.clone()); | |
273 | } | |
274 | }, | |
275 | ); | |
276 | } | |
277 | ||
f20569fa XL |
278 | fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool { |
279 | match_def_path(cx, def_id, &paths::MUTEX_GUARD) | |
280 | || match_def_path(cx, def_id, &paths::RWLOCK_READ_GUARD) | |
281 | || match_def_path(cx, def_id, &paths::RWLOCK_WRITE_GUARD) | |
282 | || match_def_path(cx, def_id, &paths::PARKING_LOT_MUTEX_GUARD) | |
283 | || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_READ_GUARD) | |
284 | || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_WRITE_GUARD) | |
285 | } | |
286 | ||
287 | fn is_refcell_ref(cx: &LateContext<'_>, def_id: DefId) -> bool { | |
288 | match_def_path(cx, def_id, &paths::REFCELL_REF) || match_def_path(cx, def_id, &paths::REFCELL_REFMUT) | |
289 | } |