Skip to content

Commit 78ab16d

Browse files
johann-beleites-sonarsourcejohann-beleites-sonarsource
committed
SONARJAVA-4832 Implement S6881
VirtualThreads should be used for tasks that include heavy blocking operations.
1 parent b02c0fe commit 78ab16d

8 files changed

Lines changed: 418 additions & 0 deletions

File tree

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S6881.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S6881",
3+
"hasTruePositives": true,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}

‎java-checks-test-sources/default/src/main/java/checks/BlockingOperationsInVirtualThreadsCheckSample.java‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
package checks;
2+
3+
import java.io.IOException;
4+
import java.net.HttpURLConnection;
5+
import java.util.concurrent.ExecutorService;
6+
import java.util.concurrent.Executors;
7+
8+
public class BlockingOperationsInVirtualThreadsCheckSample {
9+
HttpURLConnection con = (java.net.HttpURLConnection) new java.net.URL("http://localhost:64738").openConnection();
10+
11+
public BlockingOperationsInVirtualThreadsCheckSample() throws IOException {
12+
}
13+
14+
void newThread() throws IOException {
15+
16+
new Thread(() -> {
17+
try {
18+
con.getResponseMessage(); // Noncompliant [[sc=13;ec=31;secondary=-2]] {{Use virtual threads for heavy blocking operations.}}
19+
con.getResponseCode(); // Noncompliant
20+
con.disconnect(); // Compliant
21+
con.getInputStream(); // Compliant
22+
con.getRequestMethod(); // Compliant
23+
} catch (IOException e) {
24+
throw new RuntimeException(e);
25+
}
26+
});
27+
28+
Thread.ofPlatform().start(() -> {
29+
try {
30+
con.getResponseMessage(); // Noncompliant
31+
} catch (IOException e) {
32+
throw new RuntimeException(e);
33+
}
34+
});
35+
36+
Thread.ofPlatform().unstarted(() -> {
37+
try {
38+
con.getResponseMessage(); // Noncompliant
39+
} catch (IOException e) {
40+
throw new RuntimeException(e);
41+
}
42+
});
43+
44+
try {
45+
con.getResponseMessage(); // Compliant - not in a thread
46+
} catch (IOException e) {
47+
throw new RuntimeException(e);
48+
}
49+
50+
new Thread(new Runnable() {
51+
@Override
52+
public void run() {
53+
try {
54+
con.getResponseMessage(); // Noncompliant
55+
} catch (IOException e) {
56+
throw new RuntimeException(e);
57+
}
58+
}
59+
});
60+
61+
new Thread() {
62+
@Override
63+
public void run() {
64+
try {
65+
con.getResponseMessage(); // Noncompliant
66+
} catch (IOException e) {
67+
throw new RuntimeException(e);
68+
}
69+
}
70+
};
71+
}
72+
73+
void executor() {
74+
ExecutorService executor = Executors.newFixedThreadPool(100);
75+
executor.execute(() -> {
76+
try {
77+
con.getResponseMessage(); // Compliant FN - we don't support ExecutorService, as it is hard to determine if it uses virtual threads without data flow analysis
78+
} catch (IOException e) {
79+
throw new RuntimeException(e);
80+
}
81+
});
82+
}
83+
84+
private class NoncompliantThread extends Thread {
85+
@Override
86+
public void run() {
87+
try {
88+
con.getResponseMessage(); // Noncompliant
89+
} catch (IOException e) {
90+
throw new RuntimeException(e);
91+
}
92+
}
93+
}
94+
95+
private class CompliantRunnable implements Runnable {
96+
@Override
97+
public void run() {
98+
try {
99+
con.getResponseMessage(); // Compliant - not a thread
100+
} catch (IOException e) {
101+
throw new RuntimeException(e);
102+
}
103+
}
104+
}
105+
106+
void otherBlockingOperations() {
107+
new Thread(() -> {
108+
try {
109+
Thread.sleep(1000); // Noncompliant
110+
} catch (InterruptedException e) {
111+
throw new RuntimeException(e);
112+
}
113+
});
114+
}
115+
116+
void compliantVirtualThreads() {
117+
118+
Thread.ofVirtual().start(() -> {
119+
try {
120+
con.getResponseMessage(); // Compliant
121+
} catch (IOException e) {
122+
throw new RuntimeException(e);
123+
}
124+
});
125+
126+
Thread.startVirtualThread(() -> {
127+
try {
128+
con.getResponseMessage(); // Compliant
129+
} catch (IOException e) {
130+
throw new RuntimeException(e);
131+
}
132+
});
133+
134+
try (ExecutorService myExecutor = Executors.newVirtualThreadPerTaskExecutor()) {
135+
myExecutor.execute(() -> {
136+
try {
137+
con.getResponseMessage(); // Compliant
138+
} catch (IOException e) {
139+
throw new RuntimeException(e);
140+
}
141+
});
142+
}
143+
}
144+
}

‎java-checks/src/main/java/org/sonar/java/checks/BlockingOperationsInVirtualThreadsCheck.java‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2024 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 GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.checks;
21+
22+
import java.util.ArrayList;
23+
import java.util.List;
24+
import org.sonar.check.Rule;
25+
import org.sonar.java.model.ExpressionUtils;
26+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
27+
import org.sonar.plugins.java.api.JavaFileScannerContext;
28+
import org.sonar.plugins.java.api.JavaVersion;
29+
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
30+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
31+
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
32+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
33+
import org.sonar.plugins.java.api.tree.MethodTree;
34+
import org.sonar.plugins.java.api.tree.NewClassTree;
35+
import org.sonar.plugins.java.api.tree.Tree;
36+
37+
@Rule(key = "S6881")
38+
public class BlockingOperationsInVirtualThreadsCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
39+
40+
/*
41+
* #### Disclaimers ####
42+
*
43+
* This rule would benefit from a call graph analysis to determine which blocking operations are executed in a thread. For proper
44+
* reporting, it would be important to report issues with a flow, clearly showing the path from the blocking operation to the thread
45+
* creation. While we already have tools to find reachable methods (see TreeHelper#findReachableMethodsInSameFile), they do not provide
46+
* the necessary information to build a flow and hence a report would be quite confusing. For now, the rule is limited to only very basic
47+
* cases.
48+
*
49+
* On the other hand, this rule may falsely trigger on some cases where the blocking operation is not executed in the thread, but e.g.
50+
* returned within a lambda or object to be executed later. I.e. a thread creating a runnable for later execution.
51+
*/
52+
53+
private static final String JAVA_LANG_THREAD = "java.lang.Thread";
54+
55+
@Override
56+
public List<Tree.Kind> nodesToVisit() {
57+
return List.of(Tree.Kind.NEW_CLASS, Tree.Kind.METHOD_INVOCATION, Tree.Kind.METHOD);
58+
}
59+
60+
@Override
61+
public boolean isCompatibleWithJavaVersion(JavaVersion version) {
62+
return version.isJava21Compatible();
63+
}
64+
65+
// This rule currently only supports a limited set of blocking operations, focused on core Java HTTP requests.
66+
private static final MethodMatchers blockingOperations = MethodMatchers.or(
67+
MethodMatchers.create()
68+
.ofTypes("java.net.URLConnection", "java.net.HttpURLConnection")
69+
.names("getResponseCode", "getResponseMessage")
70+
.withAnyParameters()
71+
.build(),
72+
MethodMatchers.create()
73+
.ofTypes(JAVA_LANG_THREAD)
74+
.names("sleep")
75+
.addParametersMatcher("long")
76+
.build());
77+
78+
private static final MethodMatchers threadCreationConstructors = MethodMatchers.create()
79+
.ofSubTypes(JAVA_LANG_THREAD)
80+
.constructor()
81+
.addParametersMatcher("java.lang.Runnable")
82+
.build();
83+
84+
private static final MethodMatchers platformThreadMethods = MethodMatchers.create()
85+
.ofTypes(JAVA_LANG_THREAD + "$Builder$OfPlatform")
86+
.names("start", "unstarted")
87+
.addParametersMatcher("java.lang.Runnable")
88+
.build();
89+
90+
// For cases where a method is overwritten to define a thread's behavior (instead of e.g. passing a lambda to a Thread's constructor).
91+
private static final MethodMatchers overwritableThreadMethods = MethodMatchers.create()
92+
.ofSubTypes(JAVA_LANG_THREAD)
93+
.names("run")
94+
.addWithoutParametersMatcher()
95+
.build();
96+
97+
@Override
98+
public void visitNode(Tree tree) {
99+
switch (tree.kind()) {
100+
case NEW_CLASS -> onConstructorFound((NewClassTree) tree);
101+
case METHOD_INVOCATION -> onMethodInvocationFound((MethodInvocationTree) tree);
102+
case METHOD -> onMethodFound((MethodTree) tree);
103+
}
104+
}
105+
106+
private void onConstructorFound(NewClassTree newClassTree) {
107+
if (threadCreationConstructors.matches(newClassTree)) {
108+
analyzeForIssues(newClassTree.arguments().get(0), newClassTree.identifier());
109+
}
110+
}
111+
112+
private void onMethodInvocationFound(MethodInvocationTree mit) {
113+
if (platformThreadMethods.matches(mit)) {
114+
analyzeForIssues(mit.arguments().get(0), mit.methodSelect());
115+
}
116+
}
117+
118+
private void onMethodFound(MethodTree tree) {
119+
if (overwritableThreadMethods.matches(tree)) {
120+
analyzeForIssues(tree, tree.simpleName());
121+
}
122+
}
123+
124+
private void analyzeForIssues(Tree tree, Tree secondary) {
125+
var finder = new BlockingOperationFinder();
126+
tree.accept(finder);
127+
finder.collectedBlockingOperations.forEach(mit -> reportIssue(
128+
ExpressionUtils.methodName(mit),
129+
"Use virtual threads for heavy blocking operations.",
130+
List.of(new JavaFileScannerContext.Location("Containing thread", secondary)),
131+
null));
132+
}
133+
134+
private static class BlockingOperationFinder extends BaseTreeVisitor {
135+
final List<MethodInvocationTree> collectedBlockingOperations = new ArrayList<>();
136+
137+
@Override
138+
public void visitMethodInvocation(MethodInvocationTree mit) {
139+
if (blockingOperations.matches(mit)) {
140+
collectedBlockingOperations.add(mit);
141+
}
142+
}
143+
}
144+
}

‎java-checks/src/test/java/org/sonar/java/checks/BlockingOperationsInVirtualThreadsCheckTest.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2024 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 GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.checks;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.sonar.java.checks.verifier.CheckVerifier;
24+
import org.sonar.java.checks.verifier.TestUtils;
25+
26+
class BlockingOperationsInVirtualThreadsCheckTest {
27+
28+
@Test
29+
void test() {
30+
CheckVerifier.newVerifier()
31+
.onFile(TestUtils.mainCodeSourcesPath("checks/BlockingOperationsInVirtualThreadsCheckSample.java"))
32+
.withCheck(new BlockingOperationsInVirtualThreadsCheck())
33+
.withJavaVersion(21)
34+
.verifyIssues();
35+
}
36+
37+
@Test
38+
void test_no_issues_below_java_21() {
39+
CheckVerifier.newVerifier()
40+
.onFile(TestUtils.mainCodeSourcesPath("checks/BlockingOperationsInVirtualThreadsCheckSample.java"))
41+
.withCheck(new BlockingOperationsInVirtualThreadsCheck())
42+
.withJavaVersion(20)
43+
.verifyNoIssues();
44+
}
45+
}

‎sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckList.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.sonar.java.checks.BasicAuthCheck;
4949
import org.sonar.java.checks.BatchSQLStatementsCheck;
5050
import org.sonar.java.checks.BigDecimalDoubleConstructorCheck;
51+
import org.sonar.java.checks.BlockingOperationsInVirtualThreadsCheck;
5152
import org.sonar.java.checks.BluetoothLowPowerModeCheck;
5253
import org.sonar.java.checks.BooleanInversionCheck;
5354
import org.sonar.java.checks.BooleanLiteralCheck;
@@ -762,6 +763,7 @@ public final class CheckList {
762763
BigDecimalDoubleConstructorCheck.class,
763764
AndroidBiometricAuthWithoutCryptoCheck.class,
764765
BlindSerialVersionUidCheck.class,
766+
BlockingOperationsInVirtualThreadsCheck.class,
765767
BluetoothLowPowerModeCheck.class,
766768
BooleanInversionCheck.class,
767769
BooleanLiteralCheck.class,

0 commit comments

Comments
 (0)