Skip to content

Commit c869f8c

Browse files
L-ApplinL-Applin
authored
Spotbugs rule for blocking calls (#5494)
* Custom check impl * implementation of an illegal method call checkstyle module * switch to spotbugs/fndbugs * cleanup * add loadCredentials method * add Object.wait()
1 parent d93f433 commit c869f8c

File tree

5 files changed

+84
-0
lines changed

5 files changed

+84
-0
lines changed

‎.changes/next-release/feature-AWSSDKforJavav2-23280fd.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "feature",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Add new spotbugs rule to detect blocking call in the async codepath"
6+
}

‎build-tools/src/main/java/software/amazon/awssdk/buildtools/findbugs/DisallowMethodCall.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
*/
3232
public class DisallowMethodCall extends OpcodeStackDetector {
3333
private static final Set<Entry<String, String>> PROHIBITED_METHODS = new HashSet<>();
34+
private static final Set<Entry<String, String>> PROHIBITED_ASYNC_BLOCKING_METHODS = new HashSet<>();
3435
private final BugReporter bugReporter;
3536

3637
static {
@@ -44,6 +45,19 @@ public class DisallowMethodCall extends OpcodeStackDetector {
4445
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpRequest", "rawQueryParameters"));
4546
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullRequest", "rawQueryParameters"));
4647
PROHIBITED_METHODS.add(new SimpleEntry<>("software/amazon/awssdk/http/SdkHttpFullRequest$Builder", "rawQueryParameters"));
48+
49+
// Blocking calls in async code path
50+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>("java/lang/Thread", "sleep"));
51+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>("java/lang/Object", "wait"));
52+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>("java/util/concurrent/CompletableFuture", "join"));
53+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>("java/util/concurrent/CompletableFuture", "get"));
54+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>(
55+
"software/amazon/awssdk/utils/CompletableFutureUtils", "joinLikeSync"));
56+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>(
57+
"software/amazon/awssdk/regions/util/HttpResourcesUtils", "readResource"));
58+
PROHIBITED_ASYNC_BLOCKING_METHODS.add(new SimpleEntry<>(
59+
"software/amazon/awssdk/auth/credentials/internal/HttpCredentialsLoader", "loadCredentials"
60+
));
4761
}
4862

4963
public DisallowMethodCall(BugReporter bugReporter) {
@@ -68,10 +82,18 @@ private void handleMethodCall() {
6882
MethodDescriptor method = getMethodDescriptorOperand();
6983
SignatureParser signature = new SignatureParser(method.getSignature());
7084
Entry<String, String> calledMethod = new SimpleEntry<>(method.getSlashedClassName(), method.getName());
85+
7186
if (PROHIBITED_METHODS.contains(calledMethod) && signature.getNumParameters() == 0) {
7287
bugReporter.reportBug(new BugInstance(this, "SDK_BAD_METHOD_CALL", NORMAL_PRIORITY)
7388
.addClassAndMethod(this)
7489
.addSourceLine(this, getPC()));
90+
return;
91+
}
92+
93+
if (PROHIBITED_ASYNC_BLOCKING_METHODS.contains(calledMethod)) {
94+
bugReporter.reportBug(new BugInstance(this, "ASYNC_BLOCKING_CALL", NORMAL_PRIORITY)
95+
.addClassAndMethod(this)
96+
.addSourceLine(this, getPC()));
7597
}
7698
}
7799
}

‎build-tools/src/main/resources/findbugs.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,5 @@
1919

2020
<BugPattern abbrev="BM" type="SDK_BAD_METHOD_CALL" category="PERFORMANCE" />
2121
<BugPattern abbrev="BTB" type="BAD_TO_BUILDER" category="BUG" />
22+
<BugPattern abbrev="ABC" type="ASYNC_BLOCKING_CALL" category="BUG" />
2223
</FindbugsPlugin>

‎build-tools/src/main/resources/messages.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,16 @@
5151
<Details>Bad toBuilder implementation. See the SpotBugs logs for problem details.</Details>
5252
</BugPattern>
5353
<BugCode abbrev="BTB">Bad toBuilder implementation</BugCode>
54+
55+
<BugPattern type="ASYNC_BLOCKING_CALL">
56+
<ShortDescription>Blocking call in async code path</ShortDescription>
57+
<LongDescription>Blocking call in async code path. See the SpotBugs logs for problem details</LongDescription>
58+
<Details>
59+
The SDK disallow the usage of blocking calls like CompletableFuture.join() or Thread.sleep() in the
60+
asynchronous code path. If this is a legitimate use of a blocking call outside of the async code path or
61+
a false positive, this error can be suppressed by updating the spotbugs-suppression.xml file located at
62+
build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml
63+
</Details>
64+
</BugPattern>
65+
<BugCode abbrev="ABC">Blocking call in async code path</BugCode>
5466
</MessageCollection>

‎build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,49 @@
310310
<Bug pattern="SA_FIELD_SELF_ASSIGNMENT"/>
311311
</Match>
312312

313+
<!-- Suppress existing blocking call. -->
314+
<!-- Classes making calls to disallowed methods made outside of the async code path can be added here to be suppressed -->
315+
<!-- TODO: remove classes from the list once blocking calls have been removed from the SDK async code path -->
316+
<Match>
317+
<Or>
318+
<Class name="~software\.amazon\.awssdk\.core\.endpointdiscovery\.EndpointDiscoveryRefreshCache"/>
319+
<Class name="~software\.amazon\.awssdk\.core\.internal\.http\.async\.CombinedResponseAsyncHttpResponseHandler" />
320+
<Class name="~software\.amazon\.awssdk\.authcrt\.signer\.internal\.AwsCrt4aSigningAdapter" />
321+
<Class name="~software\.amazon\.awssdk\.http\.auth\.aws\.crt\.internal\.signer\.DefaultAwsCrtV4aHttpSigner" />
322+
<Class name="~software\.amazon\.awssdk\.http\.auth\.aws\.crt\.internal\.signer\.RollingSigner" />
323+
<Class name="~software\.amazon\.awssdk\.core\.internal\.http\.pipeline\.stages\.ApplyUserAgentStage" />
324+
<Class name="~software\.amazon\.awssdk\.core\.internal\.http\.pipeline\.stages\.SigningStage" />
325+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.AwsCredentialsProviderChain" />
326+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.CredentialUtils" />
327+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.TokenUtils" />
328+
<Class name="~software\.amazon\.awssdk\.auth\.signer\.AwsSignerExecutionAttribute" />
329+
<Class name="~software\.amazon\.awssdk\.auth\.token\.credentials\.SdkTokenProviderChain" />
330+
<Class name="~software\.amazon\.awssdk\.auth\.token\.signer\.SdkTokenExecutionAttribute" />
331+
<Class name="~software\.amazon\.awssdk\.awscore\.internal\.authcontext\.AwsCredentialsAuthorizationStrategy" />
332+
<Class name="~software\.amazon\.awssdk\.awscore\.internal\.authcontext\.TokenAuthorizationStrategy" />
333+
<Class name="~software\.amazon\.awssdk\.http\.nio\.netty\.internal\.AwaitCloseChannelPoolMap" />
334+
<Class name="~software\.amazon\.awssdk\.http\.nio\.netty\.internal\.ChannelAttributeKey" />
335+
<Class name="~software\.amazon\.awssdk\.transfer\.s3\.internal\.progress\.ResumeTransferProgress" />
336+
<Class name="~software\.amazon\.awssdk\.utils\.CompletableFutureUtils" />
337+
<Class name="~software\.amazon\.awssdk\.metrics\.publishers\.cloudwatch\.CloudWatchMetricPublisher" />
338+
<Class name="~software\.amazon\.awssdk\.http\.apache\.internal\.conn\.IdleConnectionReaper\$ReaperTask" />
339+
<Class name="~software\.amazon\.awssdk\.core\.internal\.retry\.RateLimitingTokenBucket" />
340+
<Class name="~software\.amazon\.awssdk\.core\.internal\.waiters\.WaiterExecutor" />
341+
<Class name="~software\.amazon\.awssdk\.regions\.internal\.util\.EC2MetadataUtils" />
342+
<Class name="~software\.amazon\.awssdk\.regions\.util\.HttpResourcesUtils" />
343+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.InstanceProfileCredentialsProvider" />
344+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.internal\.HttpCredentialsLoader" />
345+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.ContainerCredentialsProvider" />
346+
<Class name="~software\.amazon\.awssdk\.auth\.credentials\.InstanceProfileCredentialsProvider" />
347+
348+
<!-- test modules are allowed to make blocking call as parts of their testing -->
349+
<Class name="~.*testutils.*" />
350+
<Class name="~.*s3benchmarks.*" />
351+
352+
</Or>
353+
<Bug pattern="ASYNC_BLOCKING_CALL"/>
354+
</Match>
355+
313356
<!-- False positive -->
314357
<Match>
315358
<Class name="software.amazon.awssdk.v2migration.EnumCasingToV2$Visitor"/>

0 commit comments

Comments
 (0)