Skip to content

Commit e695451

Browse files
guillaume-dequennesonartech
guillaume-dequenne
authored and
sonartech
committed
SONARPY-2953 Rule S7490: Cancellation scopes should contain checkpoints (#283)
GitOrigin-RevId: c313d78d895a9348a728fe30ccbc2f339dfacba3
1 parent 80a63bb commit e695451

7 files changed

Lines changed: 435 additions & 0 deletions

File tree

‎python-checks/src/main/java/org/sonar/python/checks/CancellationScopeNoCheckpointCheck.java‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.Map;
22+
import javax.annotation.Nullable;
23+
import org.sonar.check.Rule;
24+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
25+
import org.sonar.plugins.python.api.SubscriptionContext;
26+
import org.sonar.plugins.python.api.tree.AwaitExpression;
27+
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
28+
import org.sonar.plugins.python.api.tree.CallExpression;
29+
import org.sonar.plugins.python.api.tree.Expression;
30+
import org.sonar.plugins.python.api.tree.ForStatement;
31+
import org.sonar.plugins.python.api.tree.Tree;
32+
import org.sonar.plugins.python.api.tree.WithItem;
33+
import org.sonar.plugins.python.api.tree.WithStatement;
34+
import org.sonar.plugins.python.api.tree.YieldExpression;
35+
import org.sonar.plugins.python.api.types.v2.TriBool;
36+
import org.sonar.python.types.v2.TypeCheckBuilder;
37+
import org.sonar.python.types.v2.TypeCheckMap;
38+
39+
@Rule(key = "S7490")
40+
public class CancellationScopeNoCheckpointCheck extends PythonSubscriptionCheck {
41+
42+
private static final String MESSAGE = "Add a checkpoint inside this cancellation scope.";
43+
private static final String SECONDARY_MESSAGE = "This checkpoint is not awaited.";
44+
45+
private static final String TRIO_CHECKPOINT = "trio.lowlevel.checkpoint";
46+
private static final Map<String, String> TYPE_WITH_FQN_MAP = Map.of(
47+
"trio.CancelScope", TRIO_CHECKPOINT,
48+
"trio.move_on_after", TRIO_CHECKPOINT,
49+
"trio.move_on_at", TRIO_CHECKPOINT
50+
);
51+
52+
private static final String ANYIO_CHECKPOINT = "anyio.lowlevel.checkpoint";
53+
private static final Map<String, String> TYPE_WITH_NAME_MAP = Map.of(
54+
"asyncio.timeout", "asyncio.sleep",
55+
"anyio.CancelScope", ANYIO_CHECKPOINT,
56+
"anyio.move_on_after", ANYIO_CHECKPOINT,
57+
"anyio.move_on_at", ANYIO_CHECKPOINT
58+
);
59+
60+
private TypeCheckMap<TypeCheckBuilder> typeCheckMap;
61+
62+
@Override
63+
public void initialize(Context context) {
64+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initializeTypeCheckers);
65+
context.registerSyntaxNodeConsumer(Tree.Kind.WITH_STMT, this::checkCancellationScope);
66+
}
67+
68+
private void initializeTypeCheckers(SubscriptionContext ctx) {
69+
typeCheckMap = new TypeCheckMap<>();
70+
71+
TYPE_WITH_FQN_MAP.forEach((typeName, checkpointFunction) ->
72+
typeCheckMap.put(
73+
ctx.typeChecker().typeCheckBuilder().isTypeWithFqn(typeName),
74+
ctx.typeChecker().typeCheckBuilder().isTypeWithFqn(checkpointFunction)
75+
)
76+
);
77+
78+
TYPE_WITH_NAME_MAP.forEach((typeName, checkpointFunction) ->
79+
typeCheckMap.put(
80+
ctx.typeChecker().typeCheckBuilder().isTypeWithName(typeName),
81+
ctx.typeChecker().typeCheckBuilder().isTypeWithName(checkpointFunction)
82+
)
83+
);
84+
}
85+
86+
private void checkCancellationScope(SubscriptionContext ctx) {
87+
WithStatement withStatement = (WithStatement) ctx.syntaxNode();
88+
89+
for (WithItem withItem : withStatement.withItems()) {
90+
Expression test = withItem.test();
91+
TypeCheckBuilder checkpointTypeChecker = getCheckpointTypeCheckerForScope(test);
92+
93+
if (checkpointTypeChecker != null) {
94+
CheckpointVisitor visitor = new CheckpointVisitor(checkpointTypeChecker);
95+
withStatement.statements().accept(visitor);
96+
97+
if (!visitor.hasCheckpoint()) {
98+
PreciseIssue issue = ctx.addIssue(test, MESSAGE);
99+
visitor.checkpointedTrees.forEach(t -> issue.secondary(t, SECONDARY_MESSAGE));
100+
}
101+
}
102+
}
103+
}
104+
105+
@Nullable
106+
private TypeCheckBuilder getCheckpointTypeCheckerForScope(Expression expression) {
107+
if (expression.is(Tree.Kind.CALL_EXPR)) {
108+
CallExpression callExpression = (CallExpression) expression;
109+
Expression callee = callExpression.callee();
110+
return typeCheckMap.getOptionalForType(callee.typeV2()).orElse(null);
111+
}
112+
return typeCheckMap.getOptionalForType(expression.typeV2()).orElse(null);
113+
}
114+
115+
private static class CheckpointVisitor extends BaseTreeVisitor {
116+
private final TypeCheckBuilder checkpointTypeChecker;
117+
private boolean checkpointFound = false;
118+
private List<Tree> checkpointedTrees = new ArrayList<>();
119+
120+
CheckpointVisitor(TypeCheckBuilder checkpointTypeChecker) {
121+
this.checkpointTypeChecker = checkpointTypeChecker;
122+
}
123+
124+
boolean hasCheckpoint() {
125+
return checkpointFound;
126+
}
127+
128+
@Override
129+
public void visitAwaitExpression(AwaitExpression awaitExpression) {
130+
checkpointFound = true;
131+
}
132+
133+
@Override
134+
public void visitYieldExpression(YieldExpression yieldExpression) {
135+
checkpointFound = true;
136+
}
137+
138+
@Override
139+
public void visitForStatement(ForStatement forStatement) {
140+
if (forStatement.isAsync()) {
141+
// async for loops implicitly create checkpoints at each iteration
142+
checkpointFound = true;
143+
}
144+
if (!checkpointFound) {
145+
super.visitForStatement(forStatement);
146+
}
147+
}
148+
149+
@Override
150+
public void visitCallExpression(CallExpression callExpression) {
151+
Expression callee = callExpression.callee();
152+
if (checkpointTypeChecker.check(callee.typeV2()) == TriBool.TRUE) {
153+
checkpointedTrees.add(callExpression);
154+
}
155+
super.visitCallExpression(callExpression);
156+
}
157+
158+
@Override
159+
protected void scan(@Nullable Tree tree) {
160+
if (!checkpointFound && tree != null) {
161+
tree.accept(this);
162+
}
163+
}
164+
}
165+
}

‎python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ public Stream<Class<?>> getChecks() {
133133
BreakContinueOutsideLoopCheck.class,
134134
BuiltinShadowingAssignmentCheck.class,
135135
BuiltinGenericsOverTypingModuleCheck.class,
136+
CancellationScopeNoCheckpointCheck.class,
136137
CaughtExceptionsCheck.class,
137138
ChangeMethodContractCheck.class,
138139
ChildAndParentExceptionCaughtCheck.class,

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S7490.html‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<p>This rule raises an issue when a cancellation scope (timeout context) is used without any checkpoints making the timeout functionality
2+
ineffective.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>When using asynchronous programming with libraries like <code>trio</code> or <code>anyio</code>, cancellation scopes (timeout contexts) are used to
5+
implement timeouts and cancellation. However, these mechanisms only work when there are checkpoints within the scope where cancellation can occur.
6+
Without any checkpoints, the timeout will never be triggered, making it ineffective.</p>
7+
<p>A checkpoint is a point in the code where cancellation can be detected and acted upon. Common checkpoints include:</p>
8+
<ul>
9+
<li> Explicit calls to the checkpoint method of the used framework </li>
10+
<li> <code>yield</code> statements </li>
11+
<li> <code>await</code> statements </li>
12+
</ul>
13+
<h3>What is the potential impact?</h3>
14+
<p>Without proper checkpoints in cancel scopes:</p>
15+
<ul>
16+
<li> Timeouts won’t work as expected </li>
17+
<li> Resources might be held longer than intended </li>
18+
<li> The application might become unresponsive or hang </li>
19+
<li> Cancellation operations won’t be honored </li>
20+
</ul>
21+
<h2>How to fix it in AsyncIO</h2>
22+
<p>There is no direct checkpoint method in <code>asyncio</code>, but you can use <code>await asyncio.sleep(0)</code> as a workaround.</p>
23+
<h3>Code examples</h3>
24+
<h4>Noncompliant code example</h4>
25+
<pre data-diff-id="3" data-diff-type="noncompliant">
26+
import asyncio
27+
28+
async def process_data(data):
29+
try:
30+
async with asyncio.timeout(1.0): # Noncompliant
31+
result = expensive_computation(data)
32+
return result
33+
except asyncio.TimeoutError:
34+
return None
35+
</pre>
36+
<h4>Compliant solution</h4>
37+
<pre data-diff-id="3" data-diff-type="compliant">
38+
import asyncio
39+
40+
async def process_data(data):
41+
try:
42+
async with asyncio.timeout(1.0): # Compliant
43+
result = expensive_computation(data)
44+
await asyncio.sleep(0)
45+
return result
46+
except asyncio.TimeoutError:
47+
return None
48+
</pre>
49+
<h2>How to fix it in Trio</h2>
50+
<h3>Code examples</h3>
51+
<h4>Noncompliant code example</h4>
52+
<pre data-diff-id="1" data-diff-type="noncompliant">
53+
import trio
54+
55+
async def process_data(data):
56+
async with trio.move_on_after(1.0): # Noncompliant
57+
result = expensive_computation(data)
58+
return result
59+
</pre>
60+
<h4>Compliant solution</h4>
61+
<pre data-diff-id="1" data-diff-type="compliant">
62+
import trio
63+
64+
async def process_data(data):
65+
async with trio.move_on_after(1.0): # Compliant
66+
result = expensive_computation(data)
67+
await trio.lowlevel.checkpoint()
68+
return result
69+
</pre>
70+
<h2>How to fix it in AnyIO</h2>
71+
<h3>Code examples</h3>
72+
<h4>Noncompliant code example</h4>
73+
<pre data-diff-id="2" data-diff-type="noncompliant">
74+
import anyio
75+
76+
async def process_data(data):
77+
async with anyio.move_on_after(1.0): # Noncompliant
78+
result = expensive_computation(data)
79+
return result
80+
</pre>
81+
<h4>Compliant solution</h4>
82+
<pre data-diff-id="2" data-diff-type="compliant">
83+
import anyio
84+
85+
async def process_data(data):
86+
async with anyio.move_on_after(1.0): # Compliant
87+
result = expensive_computation(data)
88+
await anyio.lowlevel.checkpoint()
89+
return result
90+
</pre>
91+
<h2>Resources</h2>
92+
<h3>Documentation</h3>
93+
<ul>
94+
<li> Trio documentation - <a href="https://trio.readthedocs.io/en/stable/reference-core.html#cancellation-and-timeouts">Cancellation and
95+
timeouts</a> </li>
96+
<li> AnyIO documentation - <a href="https://anyio.readthedocs.io/en/stable/cancellation.html#timeouts">Timeouts</a> </li>
97+
</ul>
98+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S7490.json‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"title": "Cancellation scopes should contain checkpoints",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"async",
11+
"asyncio",
12+
"trio",
13+
"anyio"
14+
],
15+
"defaultSeverity": "Major",
16+
"ruleSpecification": "RSPEC-7490",
17+
"sqKey": "S7490",
18+
"scope": "All",
19+
"quickfix": "unknown",
20+
"code": {
21+
"impacts": {
22+
"RELIABILITY": "HIGH"
23+
},
24+
"attribute": "CONVENTIONAL"
25+
}
26+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@
259259
"S7487",
260260
"S7488",
261261
"S7489",
262+
"S7490",
262263
"S7491",
263264
"S7492",
264265
"S7493",

‎python-checks/src/test/java/org/sonar/python/checks/CancellationScopeNoCheckpointCheckTest.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class CancellationScopeNoCheckpointCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/cancellationScopeNoCheckpoint.py", new CancellationScopeNoCheckpointCheck());
27+
}
28+
29+
}

0 commit comments

Comments
 (0)