Skip to content

Commit a4f52c1

Browse files
PatrikScullyPatrikScully
committed
Add more logic
1 parent 23b7c77 commit a4f52c1

23 files changed

+699
-102
lines changed

‎CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
99
### Fixed
1010
- Do not report BC_UNCONFIRMED_CAST for Java 21's type switches when the switch instruction is TABLESWITCH ([#2782](https://github.com/spotbugs/spotbugs/issues/2782))
1111

12+
### Added
13+
- New detector `ResourceInMultipleThreadsDetector` and introduced new bug type:
14+
- `AT_UNSAFE_RESOURCE_ACCESS_IN_THREAD` is reported in case of unsafe resource access in multiple threads.
15+
1216
## 4.8.5 - 2024-05-03
1317
### Fixed
1418
- Fix FP `SING_SINGLETON_GETTER_NOT_SYNCHRONIZED` with eager instances ([#2932](https://github.com/spotbugs/spotbugs/issues/2932))

‎spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/ResourceInMultipleThreadsDetectorTest.java

+56-2
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,65 @@
1212
class ResourceInMultipleThreadsDetectorTest extends AbstractIntegrationTest {
1313

1414
@Test
15-
void testUSCurrency() {
16-
performAnalysis("atomicMethods/ExampleClientCode.class", "atomicMethods/USCurrency.class");
15+
void testSafeUsages() {
16+
performAnalysis("commonResources/SafeAtomicFieldUsage.class",
17+
"commonResources/SafeSynchronizedCollectionUsage.class",
18+
"commonResources/SafeFieldUsages.class",
19+
"commonResources/SynchronizedSafeFieldUsage.class",
20+
"commonResources/SafeFieldGetterUsage.class",
21+
"commonResources/SafeBuilderPattern.class",
22+
"commonResources/SafePutFieldWithBuilderPattern.class",
23+
"commonResources/SafeFieldUsageInMainThread.class",
24+
"commonResources/CombinedThreadsInShutdownHook.class",
25+
"commonResources/CombinedThreadsInShutdownHook$1.class",
26+
"commonResources/Vehicle.class",
27+
"commonResources/Vehicle$Builder.class");
1728
assertNumOfBugs(0);
1829
}
1930

31+
@Test
32+
void testUnsafeFieldUsages() {
33+
performAnalysis("commonResources/UnsafeFieldUsage.class",
34+
"commonResources/Vehicle.class",
35+
"commonResources/Vehicle$Builder.class");
36+
assertNumOfBugs(5);
37+
assertBug("UnsafeFieldUsage", "lambda$new$0", 8);
38+
assertBug("UnsafeFieldUsage", "createVehicle", 16);
39+
assertBug("UnsafeFieldUsage", "createVehicle", 17);
40+
assertBug("UnsafeFieldUsage", "createVehicle", 18);
41+
assertBug("UnsafeFieldUsage", "createVehicle", 19);
42+
}
43+
44+
@Test
45+
void testUnsafeFieldUsages2() {
46+
performAnalysis("commonResources/UnsafeFieldUsage2.class",
47+
"commonResources/Vehicle.class",
48+
"commonResources/Vehicle$Builder.class");
49+
assertNumOfBugs(2);
50+
assertBug("UnsafeFieldUsage2", "lambda$new$0", 8);
51+
assertBug("UnsafeFieldUsage2", "createVehicle", 16);
52+
}
53+
54+
@Test
55+
void testUnsafeFieldUsages3() {
56+
performAnalysis("commonResources/UnsafeFieldUsage3.class",
57+
"commonResources/Vehicle.class",
58+
"commonResources/Vehicle$Builder.class");
59+
assertNumOfBugs(2);
60+
assertBug("UnsafeFieldUsage3", "lambda$new$0", 12);
61+
assertBug("UnsafeFieldUsage3", "createVehicle", 8);
62+
}
63+
64+
@Test
65+
void testSynchronizedUnsafeFieldUsage() {
66+
performAnalysis("commonResources/SynchronizedUnsafeFieldUsage.class",
67+
"commonResources/Vehicle.class",
68+
"commonResources/Vehicle$Builder.class");
69+
assertNumOfBugs(2);
70+
assertBug("SynchronizedUnsafeFieldUsage", "lambda$new$0", 8);
71+
assertBug("SynchronizedUnsafeFieldUsage", "createVehicle", 16);
72+
}
73+
2074
private void assertNumOfBugs(int num) {
2175
final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
2276
.bugType("AT_UNSAFE_RESOURCE_ACCESS_IN_THREAD").build();

‎spotbugs/etc/findbugs.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@
644644
reports="UR_UNINIT_READ_CALLED_FROM_SUPER_CONSTRUCTOR"/>
645645
<Detector class="edu.umd.cs.findbugs.detect.AtomicityProblem"
646646
reports="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION"/>
647-
<Detector class="edu.umd.cs.findbugs.detect.ResourceInMultipleThreadsDetector"
647+
<Detector class="edu.umd.cs.findbugs.detect.ResourceInMultipleThreadsDetector" speed="fast"
648648
reports="AT_UNSAFE_RESOURCE_ACCESS_IN_THREAD"/>
649649
<Detector class="edu.umd.cs.findbugs.detect.CheckExpectedWarnings"
650650
reports="FB_UNEXPECTED_WARNING,FB_MISSING_EXPECTED_WARNING" disabled="true"

‎spotbugs/etc/messages.xml

+10-5
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,7 @@ applied to method parameters and uses of those method parameters. </p>
232232
<Detector class="edu.umd.cs.findbugs.detect.ResourceInMultipleThreadsDetector">
233233
<Details>
234234
<![CDATA[
235-
<p> Finds sequences of operations on synchronized/atomic abstractions
236-
like SynchronizedList or AtomicReference that are not in a synchronized block.
235+
<p> Finds non-atomic typed resources that are used in multiple threads without proper synchronization.
237236
</p>
238237
]]>
239238
</Details>
@@ -8682,11 +8681,17 @@ after the call to initLogging, the logger configuration is lost
86828681
</Details>
86838682
</BugPattern>
86848683
<BugPattern type="AT_UNSAFE_RESOURCE_ACCESS_IN_THREAD">
8685-
<ShortDescription>todo</ShortDescription>
8686-
<LongDescription>todo</LongDescription>
8684+
<ShortDescription>Operation on resource is not safe in a multithreaded context</ShortDescription>
8685+
<LongDescription>Operation on resource {3} is not safe in a multithreaded context in {1}</LongDescription>
86878686
<Details>
86888687
<![CDATA[
8689-
<p>todo
8688+
<p>This code contains an operation on a resource that is not safe in a multithreaded context.
8689+
The resource may be accessed by multiple threads concurrently without proper synchronization.
8690+
This may lead to data corruption. Use synchronization or other
8691+
concurrency control mechanisms to ensure that the resource is accessed safely.</p>
8692+
<p>See the related SEI CERT rule, but the detector is not restricted to chained methods:
8693+
<a href="https://wiki.sei.cmu.edu/confluence/display/java/VNA04-J.+Ensure+that+calls+to+chained+methods+are+atomic">
8694+
VNA04-J. Ensure that calls to chained methods are atomic</a>.
86908695
</p>]]>
86918696
</Details>
86928697
</BugPattern>

‎spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindPublicAttributes.java

+2-26
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,8 @@
1818

1919
package edu.umd.cs.findbugs.detect;
2020

21-
import java.util.Arrays;
2221
import java.util.HashMap;
2322
import java.util.HashSet;
24-
import java.util.List;
2523
import java.util.Map;
2624
import java.util.Set;
2725

@@ -39,15 +37,6 @@
3937

4038
public class FindPublicAttributes extends OpcodeStackDetector {
4139

42-
private static final Set<String> CONSTRUCTOR_LIKE_NAMES = new HashSet<>(Arrays.asList(
43-
Const.CONSTRUCTOR_NAME, Const.STATIC_INITIALIZER_NAME,
44-
"clone", "init", "initialize", "dispose", "finalize", "this",
45-
"_jspInit", "jspDestroy"));
46-
47-
private static final List<String> SETTER_LIKE_NAMES = Arrays.asList(
48-
"set", "put", "add", "insert", "delete", "remove", "erase", "clear",
49-
"push", "pop", "enqueue", "dequeue", "write", "append");
50-
5140
private final BugReporter bugReporter;
5241

5342
private final Set<XField> writtenFields = new HashSet<>();
@@ -84,7 +73,7 @@ public void visit(JavaClass obj) {
8473
public void sawOpcode(int seen) {
8574
// It is normal that classes used as simple data types have a
8675
// constructor to make initialization easy.
87-
if (isConstructorLikeMethod(getMethodName())) {
76+
if (MutableClasses.isConstructorLikeMethod(getMethodName())) {
8877
return;
8978
}
9079

@@ -153,7 +142,7 @@ public void sawOpcode(int seen) {
153142
// which roughly describes its behavior, beginning with the verb.
154143
// If the verb does not hint that the method writes the object
155144
// then we skip it.
156-
if (!looksLikeASetter(xmo.getName())) {
145+
if (!MutableClasses.looksLikeASetter(xmo.getName())) {
157146
return;
158147
}
159148

@@ -192,17 +181,4 @@ public void sawOpcode(int seen) {
192181
writtenFields.add(field);
193182
}
194183
}
195-
196-
private static boolean isConstructorLikeMethod(String methodName) {
197-
return CONSTRUCTOR_LIKE_NAMES.contains(methodName);
198-
}
199-
200-
private static boolean looksLikeASetter(String methodName) {
201-
for (String name : SETTER_LIKE_NAMES) {
202-
if (methodName.startsWith(name)) {
203-
return true;
204-
}
205-
}
206-
return false;
207-
}
208184
}

0 commit comments

Comments
 (0)