[starvation] fix inner class tests

Summary: Use more informative method names, and add comments explaining the logic behind each test.  Correct two cases which are FPs instead of legitimate reports.

Reviewed By: artempyanykh

Differential Revision: D19465227

fbshipit-source-id: 29332e2b9

infer/tests/codetoanalyze/java/starvation/InnerClass.java

@@ -6,46 +6,54 @@
  */
 
 class InnerClass {
+  // shouldn't be flagged
+  // we don't know that [a.this$0 == this] and even if it were
+  // this will simply lock this twice
   synchronized void outerInnerOk(InnerClassA a) {
-    a.foo();
+    a.lockOuter();
   }
 
-  synchronized void bar() {}
+  synchronized void lockOuter() {}
 
-  synchronized void outerInnerBad(InnerClassA a) {
+  // following is flagged currently but shouldn't
-    a.baz();
+  // we don't known that [a.this$0 == this]!
+  synchronized void FP_outerInnerOk(InnerClassA a) {
+    a.lockInner();
   }
 
   class InnerClassA {
-    void foo() {
+    void lockOuter() {
       synchronized (InnerClass.this) {
       }
     }
 
     void outerInnerOk() {
       synchronized (InnerClass.this) {
-        InnerClass.this.bar();
+        InnerClass.this.lockOuter();
       }
     }
 
-    synchronized void baz() {}
+    synchronized void lockInner() {}
 
     synchronized void innerOuterBad() {
-      InnerClass.this.bar();
+      InnerClass.this.lockOuter();
     }
 
-    // ctrs generate different access paths so test these too
+    // constructors generate different access paths so test these too
-    // following should not be flagged
+    // TODO these tests do not generate yet different access paths to the above :(
+
+    // following should not be flagged -- it's a double lock on [this.this$0]
     InnerClassA() {
       synchronized (InnerClass.this) {
-        InnerClass.this.bar();
+        InnerClass.this.lockOuter();
       }
     }
 
-    // following should be flagged with outer_inner_bad()
+    // following would be flagged with outerInnerBad but should not
+    // because [this] is not accessible yet to any other thread!
     InnerClassA(Object o) {
       synchronized (this) {
-        InnerClass.this.bar();
+        InnerClass.this.lockOuter();
       }
     }
   }

infer/tests/codetoanalyze/java/starvation/issues.exp

@@ -13,10 +13,10 @@ codetoanalyze/java/starvation/FutureGet.java, FutureGet.getTimeoutOneDayBad():vo
 codetoanalyze/java/starvation/FutureGet.java, FutureGet.getTimeoutOneHourBad():void, 59, STARVATION, no_bucket, ERROR, [`void FutureGet.getTimeoutOneHourBad()`,calls `Object Future.get(long,TimeUnit)`]
 codetoanalyze/java/starvation/IndirectBlock.java, IndirectBlock.takeExpensiveLockOnUiThreadBad():void, 23, STARVATION, no_bucket, ERROR, [[Trace 1] `void IndirectBlock.takeExpensiveLockOnUiThreadBad()`, locks `this.expensiveLock` in `class IndirectBlock`,[Trace 2] `void IndirectBlock.doTransactUnderLock()`, locks `this.expensiveLock` in `class IndirectBlock`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation/IndirectBlock.java, IndirectBlock.takeRemoteExpensiveLockOnUiThreadBad(IndirectInterproc):void, 35, STARVATION, no_bucket, ERROR, [[Trace 1] `void IndirectBlock.takeRemoteExpensiveLockOnUiThreadBad(IndirectInterproc)`,Method call: `void IndirectInterproc.takeLock()`, locks `this` in `class IndirectInterproc`,[Trace 2] `void IndirectInterproc.doTransactUnderLock(Binder)`, locks `this` in `class IndirectInterproc`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
-codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.<init>(InnerClass,java.lang.Object), 47, DEADLOCK, no_bucket, ERROR, [[Trace 1] `InnerClass$InnerClassA.<init>(InnerClass,Object)`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.bar()`, locks `this` in `class InnerClass`,[Trace 2] `void InnerClass.outerInnerBad(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.baz()`, locks `this` in `class InnerClass$InnerClassA`]
+codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.<init>(InnerClass,java.lang.Object), 55, DEADLOCK, no_bucket, ERROR, [[Trace 1] `InnerClass$InnerClassA.<init>(InnerClass,Object)`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.lockOuter()`, locks `this` in `class InnerClass`,[Trace 2] `void InnerClass.FP_outerInnerOk(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.lockInner()`, locks `this` in `class InnerClass$InnerClassA`]
-codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.innerOuterBad():void, 34, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass$InnerClassA.innerOuterBad()`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.bar()`, locks `this` in `class InnerClass`,[Trace 2] `void InnerClass.outerInnerBad(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.baz()`, locks `this` in `class InnerClass$InnerClassA`]
+codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.innerOuterBad():void, 39, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass$InnerClassA.innerOuterBad()`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.lockOuter()`, locks `this` in `class InnerClass`,[Trace 2] `void InnerClass.FP_outerInnerOk(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.lockInner()`, locks `this` in `class InnerClass$InnerClassA`]
-codetoanalyze/java/starvation/InnerClass.java, InnerClass.outerInnerBad(InnerClass$InnerClassA):void, 16, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass.outerInnerBad(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.baz()`, locks `this` in `class InnerClass$InnerClassA`,[Trace 2] `InnerClass$InnerClassA.<init>(InnerClass,Object)`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.bar()`, locks `this` in `class InnerClass`]
+codetoanalyze/java/starvation/InnerClass.java, InnerClass.FP_outerInnerOk(InnerClass$InnerClassA):void, 21, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass.FP_outerInnerOk(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.lockInner()`, locks `this` in `class InnerClass$InnerClassA`,[Trace 2] `InnerClass$InnerClassA.<init>(InnerClass,Object)`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.lockOuter()`, locks `this` in `class InnerClass`]
-codetoanalyze/java/starvation/InnerClass.java, InnerClass.outerInnerBad(InnerClass$InnerClassA):void, 16, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass.outerInnerBad(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.baz()`, locks `this` in `class InnerClass$InnerClassA`,[Trace 2] `void InnerClass$InnerClassA.innerOuterBad()`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.bar()`, locks `this` in `class InnerClass`]
+codetoanalyze/java/starvation/InnerClass.java, InnerClass.FP_outerInnerOk(InnerClass$InnerClassA):void, 21, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InnerClass.FP_outerInnerOk(InnerClass$InnerClassA)`, locks `this` in `class InnerClass`,Method call: `void InnerClass$InnerClassA.lockInner()`, locks `this` in `class InnerClass$InnerClassA`,[Trace 2] `void InnerClass$InnerClassA.innerOuterBad()`, locks `this` in `class InnerClass$InnerClassA`,Method call: `void InnerClass.lockOuter()`, locks `this` in `class InnerClass`]
 codetoanalyze/java/starvation/Interclass.java, Interclass.interclass1Bad(InterclassA):void, 10, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Interclass.interclass1Bad(InterclassA)`, locks `this` in `class Interclass`,Method call: `void InterclassA.interclass1Bad()`, locks `this` in `class InterclassA`,[Trace 2] `void InterclassA.interclass2Bad(Interclass)`, locks `this` in `class InterclassA`,Method call: `void Interclass.interclass2Bad()`, locks `this` in `class Interclass`]
 codetoanalyze/java/starvation/Interclass.java, InterclassA.interclass2Bad(Interclass):void, 37, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void InterclassA.interclass2Bad(Interclass)`, locks `this` in `class InterclassA`,Method call: `void Interclass.interclass2Bad()`, locks `this` in `class Interclass`,[Trace 2] `void Interclass.interclass1Bad(InterclassA)`, locks `this` in `class Interclass`,Method call: `void InterclassA.interclass1Bad()`, locks `this` in `class InterclassA`]
 codetoanalyze/java/starvation/Interproc.java, Interproc.interproc1Bad(InterprocA):void, 10, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void Interproc.interproc1Bad(InterprocA)`, locks `this` in `class Interproc`,Method call: `void Interproc.interproc2Bad(InterprocA)`, locks `b` in `class InterprocA`,[Trace 2] `void InterprocA.interproc1Bad(Interproc)`, locks `this` in `class InterprocA`,Method call: `void InterprocA.interproc2Bad(Interproc)`, locks `d` in `class Interproc`]