Hi
Thnaks your findbugs effort. It's great work for us the java programmers.
BTW, in IS2_INCONSISTENT_SYNC finder, I think it's better to uncount
read-unlock while write-unlock was found in one method.
for example)
public class Is2 {
int count;
public void countUp() {
count++;
}
public synchronized int getCount() {
return count;
}
}
In this case, method Is2#countUp has both read/write for
post-incremental operation, but the meaning of the code, it's just a
write operation, not read one.
But FindInconsistentSync2 handles it just as
findbugs -property "fis.debug=true" -textui Is2.class
******** Analyzing class Is2
Call graph (not unlocked methods): 3 nodes, 0 edges
Apparently not unlocked methods:
<init>
Apparently locked methods:
getCount
Methods apparently reachable from public non-constructor methods:
getCount
countUp
**** Analyzing method Is2.getCount()
Handling field access: 1: getfield[180](3) 2 (frame=[0|0])
IS2: Is2.getCount() Is2.count R/L
**** Analyzing method Is2.countUp()
Handling field access: 2: getfield[180](3) 2 (frame=[0|0,0])
IS2: Is2.countUp() Is2.count R/U
Handling field access: 7: putfield[181](3) 2 (frame=[0|0,3])
IS2: Is2.countUp() Is2.count W/U
IS2: Is2.count
RL: 1
WL: 0
RU: 1
WU: 1
Sync %: 33
The post incremental causes one RU and one WU, so Sync is 33%, and no
message was generated (it's just a low priority message). But, I think
that it might be 50% and with normal priority message.
So I believe that If an unlocked method contains write operation(s),
then it's better to ignore read operation(s) for the accurate detection.
Then I try to make experimental patch.
diff -u FindInconsistentSync2.java FindInconsistentSync2.java.new
--- FindInconsistentSync2.java Sun May 01 14:52:00 2005
+++ FindInconsistentSync2.java.new Wed May 04 06:04:06 2005
@@ -105,15 +105,26 @@
*/
private static class FieldStats {
private int[] countList = new int[4];
+ private int[] currentList = new int[4];
private int numLocalLocks = 0;
private int numGetterMethodAccesses = 0;
private LinkedList<SourceLineAnnotation> unsyncAccessList = new
LinkedList<SourceLineAnnotation>();
private LinkedList<SourceLineAnnotation> syncAccessList = new
LinkedList<SourceLineAnnotation>();
public void addAccess(int kind) {
- countList[kind]++;
+ currentList[kind]++;
}
+ public void accumulate() {
+ if (currentList[WRITE_UNLOCKED] > 0) {
+ currentList[READ_UNLOCKED] = 0;
+ }
+ for (int i = 0; i < countList.length; i++) {
+ countList[i] += currentList[i];
+ currentList[i] = 0;
+ }
+ }
+
public int getNumAccesses(int kind) {
return countList[kind];
}
@@ -497,6 +508,9 @@
bugReporter.reportMissingClass(e);
}
}
+ for (FieldStats stats : statMap.values()) {
+ stats.accumulate();
+ }
}
With above patch, findbugs warns as I suppose.
findbugs -property "fis.debug=true" -textui Is2.class
******** Analyzing class Is2
Call graph (not unlocked methods): 3 nodes, 0 edges
Apparently not unlocked methods:
<init>
Apparently locked methods:
getCount
Methods apparently reachable from public non-constructor methods:
getCount
countUp
**** Analyzing method Is2.getCount()
Handling field access: 1: getfield[180](3) 2 (frame=[0|0])
IS2: Is2.getCount() Is2.count R/L
**** Analyzing method Is2.countUp()
Handling field access: 2: getfield[180](3) 2 (frame=[0|0,0])
IS2: Is2.countUp() Is2.count R/U
Handling field access: 7: putfield[181](3) 2 (frame=[0|0,3])
IS2: Is2.countUp() Is2.count W/U
IS2: Is2.count
RL: 1
WL: 0
RU: 0
WU: 1
Sync %: 50
M M IS2: Is2.count(snip)
What do you think ?
--
arton <art@xxxxxxxxxxxxx>
|