[starvation] add static timeout recognition and 5 second android limit

Reviewed By: jvillard

Differential Revision: D7953454

fbshipit-source-id: 76a72f0

infer/src/concurrency/RacerDConfig.ml

@@ -586,6 +586,38 @@ module Models = struct
         false
 
 
+  (** magical value from https://developer.android.com/topic/performance/vitals/anr *)
+  let android_anr_time_limit = 5.0
+
+  (** can this timeout cause an anr? *)
+  let is_excessive_timeout =
+    let time_units =
+      String.Map.of_alist_exn
+        [ ("NANOSECONDS", 0.000_000_001)
+        ; ("MICROSECONDS", 0.000_001)
+        ; ("MILLISECONDS", 0.001)
+        ; ("SECONDS", 1.0)
+        ; ("MINUTES", 60.0)
+        ; ("HOURS", 3_600.0)
+        ; ("DAYS", 86_400.0) ]
+    in
+    fun duration_exp timeunit_exp ->
+      match (duration_exp, timeunit_exp) with
+      | HilExp.Constant (Const.Cint duration_lit), HilExp.AccessExpression timeunit_acc_exp -> (
+        match AccessExpression.to_access_path timeunit_acc_exp with
+        | _, [AccessPath.FieldAccess field]
+          when String.equal "java.util.concurrent.TimeUnit" (Typ.Fieldname.Java.get_class field) ->
+            let fieldname = Typ.Fieldname.Java.get_field field in
+            let duration = float_of_int (IntLit.to_int duration_lit) in
+            String.Map.find time_units fieldname
+            |> Option.value_map ~default:false ~f:(fun unit_in_secs ->
+                   unit_in_secs *. duration >. android_anr_time_limit )
+        | _ ->
+            false )
+      | _ ->
+          false
+
+
   (** It's surprisingly difficult making any sense of the official documentation on java.io classes
                 as to whether methods may block.  We approximate these by calls in traditionally blocking
                 classes (non-channel based I/O), to methods `(Reader|InputStream).read*`.
@@ -596,9 +628,19 @@ module Models = struct
       "read"
 
 
+  let actuals_are_empty_or_timeout = function
+    | [_] ->
+        true
+    | [_; duration; timeunit] ->
+        is_excessive_timeout duration timeunit
+    | _ ->
+        false
+
+
   (** is the method called CountDownLath.await or on subclass? *)
   let is_countdownlatch_await =
-    is_call_of_class_or_superclass ["java.util.concurrent.CountDownLatch"] "await"
+    is_call_of_class_or_superclass ~actuals_pred:actuals_are_empty_or_timeout
+      ["java.util.concurrent.CountDownLatch"] "await"
 
 
   (** an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
@@ -618,15 +660,18 @@ module Models = struct
 
 
   (** is it a call to Future.get() or on sublass? *)
-  let is_future_get = is_call_of_class_or_superclass ["java.util.concurrent.Future"] "get"
+  let is_future_get =
+    is_call_of_class_or_superclass ~actuals_pred:actuals_are_empty_or_timeout
+      ["java.util.concurrent.Future"] "get"
+
 
   let is_accountManager_setUserData =
     is_call_of_class_or_superclass ["android.accounts.AccountManager"] "setUserData"
 
 
-  let is_asyncTask_get_without_timeout =
-    let actuals_pred actuals = Int.equal 1 (List.length actuals) in
-    is_call_of_class_or_superclass ~actuals_pred ["android.os.AsyncTask"] "get"
+  let is_asyncTask_get =
+    is_call_of_class_or_superclass ~actuals_pred:actuals_are_empty_or_timeout
+      ["android.os.AsyncTask"] "get"
 
 
   let may_block =
@@ -637,7 +682,7 @@ module Models = struct
       ; is_getWindowVisibleDisplayFrame
       ; is_future_get
       ; is_accountManager_setUserData
-      ; is_asyncTask_get_without_timeout ]
+      ; is_asyncTask_get ]
     in
     fun tenv pn actuals -> List.exists matchers ~f:(fun matcher -> matcher tenv pn actuals)
 end

infer/tests/codetoanalyze/java/starvation/FutureGet.java

@@ -9,7 +9,9 @@
 
 import java.util.concurrent.Future;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeoutException;
 import android.support.annotation.UiThread;
+import java.util.concurrent.TimeUnit;
 
 class FutureGet {
   Future future;
@@ -34,4 +36,46 @@ class FutureGet {
   void getOnOtherThreadOk() throws InterruptedException, ExecutionException {
     future.get();
   }
+
+  @UiThread
+  void getTimeoutOneDayBad() throws InterruptedException, ExecutionException {
+    try {
+      future.get(1L, TimeUnit.DAYS);
+    } catch (TimeoutException e) {}
+  }
+
+  @UiThread
+  void getTimeoutOneSecondOk() throws InterruptedException, ExecutionException {
+    try {
+      future.get(1L, TimeUnit.SECONDS);
+    } catch (TimeoutException e) {}
+  }
+
+  @UiThread
+  void getTimeoutOneHourBad() throws InterruptedException, ExecutionException {
+    try {
+      future.get(1L, TimeUnit.HOURS);
+    } catch (TimeoutException e) {}
+  }
+
+  @UiThread
+  void getTimeoutFourSecondsOk() throws InterruptedException, ExecutionException {
+    try {
+      future.get(4L, TimeUnit.SECONDS);
+    } catch (TimeoutException e) {}
+  }
+
+  @UiThread
+  void getTimeout4999MilliSecondsOk() throws InterruptedException, ExecutionException {
+    try {
+      future.get(4999L, TimeUnit.MILLISECONDS);
+    } catch (TimeoutException e) {}
+  }
+
+  @UiThread
+  void getTimeout50000001MicroSecondsBad() throws InterruptedException, ExecutionException {
+    try {
+      future.get(5000001L, TimeUnit.MICROSECONDS);
+    } catch (TimeoutException e) {}
+  }
 }

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

@@ -7,8 +7,11 @@ codetoanalyze/java/starvation/Binders.java, void Binders.interBad(), 25, STARVAT
 codetoanalyze/java/starvation/Binders.java, void Binders.intraBad(), 30, STARVATION, ERROR, [ void Binders.intraBad(),Method call: void Binders.doTransact(),calls boolean Binder.transact(int,Parcel,Parcel,int) from void Binders.doTransact()]
 codetoanalyze/java/starvation/Countdwn.java, void Countdwn.awaitOnMainByAnnotBad(), 22, STARVATION, ERROR, [ void Countdwn.awaitOnMainByAnnotBad(),calls void CountDownLatch.await() from void Countdwn.awaitOnMainByAnnotBad()]
 codetoanalyze/java/starvation/Countdwn.java, void Countdwn.awaitOnMainByCallBad(), 16, STARVATION, ERROR, [ void Countdwn.awaitOnMainByCallBad(),calls void CountDownLatch.await() from void Countdwn.awaitOnMainByCallBad()]
-codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getDirectBad(), 19, STARVATION, ERROR, [ void FutureGet.getDirectBad(),calls Object Future.get() from void FutureGet.getDirectBad()]
-codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getIndirectBad(), 24, STARVATION, ERROR, [[Trace 1]  void FutureGet.getIndirectBad(),locks `this.FutureGet.lock` in class `FutureGet*`,[Trace 2]  void FutureGet.getUnderLock(),locks `this.FutureGet.lock` in class `FutureGet*`,calls Object Future.get() from void FutureGet.getUnderLock()]
+codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getDirectBad(), 21, STARVATION, ERROR, [ void FutureGet.getDirectBad(),calls Object Future.get() from void FutureGet.getDirectBad()]
+codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getIndirectBad(), 26, STARVATION, ERROR, [[Trace 1]  void FutureGet.getIndirectBad(),locks `this.FutureGet.lock` in class `FutureGet*`,[Trace 2]  void FutureGet.getUnderLock(),locks `this.FutureGet.lock` in class `FutureGet*`,calls Object Future.get() from void FutureGet.getUnderLock()]
+codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getTimeout50000001MicroSecondsBad(), 76, STARVATION, ERROR, [ void FutureGet.getTimeout50000001MicroSecondsBad(),calls Object Future.get(long,TimeUnit) from void FutureGet.getTimeout50000001MicroSecondsBad()]
+codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getTimeoutOneDayBad(), 41, STARVATION, ERROR, [ void FutureGet.getTimeoutOneDayBad(),calls Object Future.get(long,TimeUnit) from void FutureGet.getTimeoutOneDayBad()]
+codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getTimeoutOneHourBad(), 55, STARVATION, ERROR, [ void FutureGet.getTimeoutOneHourBad(),calls Object Future.get(long,TimeUnit) from void FutureGet.getTimeoutOneHourBad()]
 codetoanalyze/java/starvation/IndirectBlock.java, void IndirectBlock.takeExpensiveLockOnUiThreadBad(), 23, STARVATION, ERROR, [[Trace 1]  void IndirectBlock.takeExpensiveLockOnUiThreadBad(),locks `this.IndirectBlock.expensiveLock` in class `IndirectBlock*`,[Trace 2]  void IndirectBlock.doTransactUnderLock(),locks `this.IndirectBlock.expensiveLock` in class `IndirectBlock*`,calls boolean Binder.transact(int,Parcel,Parcel,int) from void IndirectBlock.doTransactUnderLock()]
 codetoanalyze/java/starvation/IndirectBlock.java, void IndirectBlock.takeRemoteExpensiveLockOnUiThreadBad(IndirectInterproc), 34, STARVATION, 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) from void IndirectInterproc.doTransactUnderLock(Binder)]
 codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.<init>(InnerClass,Object), 49, DEADLOCK, 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*`]