-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement asyncio-dangling-task to track asyncio.create_task calls
- Loading branch information
1 parent
08e9d12
commit 47838db
Showing
10 changed files
with
159 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,52 @@ | ||
import asyncio | ||
|
||
|
||
# Error | ||
def f(): | ||
asyncio.create_task(coordinator.ws_connect()) # Error | ||
|
||
|
||
# OK | ||
def f(): | ||
background_tasks = set() | ||
|
||
for i in range(10): | ||
task = asyncio.create_task(some_coro(param=i)) | ||
|
||
# Add task to the set. This creates a strong reference. | ||
background_tasks.add(task) | ||
|
||
# To prevent keeping references to finished tasks forever, | ||
# make each task remove its own reference from the set after | ||
# completion: | ||
task.add_done_callback(background_tasks.discard) | ||
|
||
|
||
# OK | ||
def f(): | ||
ctx.task = asyncio.create_task(make_request()) | ||
|
||
|
||
# OK | ||
def f(): | ||
tasks.append(asyncio.create_task(self._populate_collection(coll, coll_info))) | ||
|
||
|
||
# OK | ||
def f(): | ||
asyncio.wait([asyncio.create_task(client.close()) for client in clients.values()]) | ||
|
||
|
||
# OK | ||
def f(): | ||
tasks = [asyncio.create_task(task) for task in tasks] | ||
|
||
|
||
# Ok (false negative) | ||
def f(): | ||
task = asyncio.create_task(coordinator.ws_connect()) | ||
|
||
|
||
# Ok (potential false negative) | ||
def f(): | ||
do_nothing_with_the_task(asyncio.create_task(coordinator.ws_connect())) |
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
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,78 @@ | ||
use rustpython_parser::ast::{Expr, ExprKind}; | ||
|
||
use ruff_macros::{define_violation, derive_message_formats}; | ||
|
||
use crate::ast::types::{CallPath, Range}; | ||
use crate::registry::Diagnostic; | ||
use crate::violation::Violation; | ||
|
||
define_violation!( | ||
/// ## What it does | ||
/// Checks for `asyncio.create_task` calls that do not store a reference | ||
/// to the returned result. | ||
/// | ||
/// ## Why is this bad? | ||
/// Per the `asyncio` documentation, the event loop only retains a weak | ||
/// reference to tasks. If the task returned by `asyncio.create_task` is | ||
/// not stored in a variable, or a collection, or otherwise referenced, it | ||
/// may be garbage collected at any time. This can lead to unexpected and | ||
/// inconsistent behavior, as your tasks may or may not run to completion. | ||
/// | ||
/// ## Example | ||
/// ```python | ||
/// import asyncio | ||
/// | ||
/// for i in range(10): | ||
/// # This creates a weak reference to the task, which may be garbage | ||
/// # collected at any time. | ||
/// asyncio.create_task(some_coro(param=i)) | ||
/// ``` | ||
/// | ||
/// Use instead: | ||
/// ```python | ||
/// import asyncio | ||
/// | ||
/// background_tasks = set() | ||
/// | ||
/// for i in range(10): | ||
/// task = asyncio.create_task(some_coro(param=i)) | ||
/// | ||
/// # Add task to the set. This creates a strong reference. | ||
/// background_tasks.add(task) | ||
/// | ||
/// # To prevent keeping references to finished tasks forever, | ||
/// # make each task remove its own reference from the set after | ||
/// # completion: | ||
/// task.add_done_callback(background_tasks.discard) | ||
/// ``` | ||
/// | ||
/// ## References | ||
/// * [_The Heisenbug lurking in your async code_](https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/) | ||
/// * [`asyncio.create_task`](https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task) | ||
pub struct AsyncioDanglingTask; | ||
); | ||
|
||
impl Violation for AsyncioDanglingTask { | ||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
format!("`asyncio.create_task` call without a reference to the returned task") | ||
} | ||
} | ||
|
||
/// RUF006 | ||
pub fn asyncio_dangling_task<'a, F>(expr: &'a Expr, resolve_call_path: F) -> Option<Diagnostic> | ||
where | ||
F: FnOnce(&'a Expr) -> Option<CallPath<'a>>, | ||
{ | ||
if let ExprKind::Call { func, .. } = &expr.node { | ||
if resolve_call_path(func).map_or(false, |call_path| { | ||
call_path.as_slice() == ["asyncio", "create_task"] | ||
}) { | ||
return Some(Diagnostic::new( | ||
AsyncioDanglingTask, | ||
Range::from_located(expr), | ||
)); | ||
} | ||
} | ||
None | ||
} |
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
15 changes: 15 additions & 0 deletions
15
crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF006_RUF006.py.snap
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,15 @@ | ||
--- | ||
source: crates/ruff/src/rules/ruff/mod.rs | ||
expression: diagnostics | ||
--- | ||
- kind: | ||
AsyncioDanglingTask: ~ | ||
location: | ||
row: 6 | ||
column: 4 | ||
end_location: | ||
row: 6 | ||
column: 49 | ||
fix: ~ | ||
parent: ~ | ||
|
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 |
---|---|---|
|
@@ -1864,6 +1864,7 @@ | |
"RUF003", | ||
"RUF004", | ||
"RUF005", | ||
"RUF006", | ||
"RUF1", | ||
"RUF10", | ||
"RUF100", | ||
|