logo       
Google Custom Search
    AddThis Social Bookmark Button
-->

IS2_INCONSISTENT_SYNC (unlock-read/write in one method): msg#00003

Subject: IS2_INCONSISTENT_SYNC (unlock-read/write in one method)
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>



<Prev in Thread] Current Thread [Next in Thread>