[starvation] catch calls to AcccountManager.setUserData

Summary: Calls to AccountManager.setUserData are associated with ANRs.

Reviewed By: sblackshear

Differential Revision: D7859585

fbshipit-source-id: 215a522

infer/src/concurrency/RacerDConfig.ml

@@ -546,7 +546,10 @@ module Models = struct
        && not (Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting)
 
 
-  let is_call_of_class_or_superclass class_names ?(method_prefix= false) method_name tenv pn =
+  let is_call_of_class_or_superclass ?(method_prefix= false) ?(actuals_pred= fun _ -> true)
+      class_names method_name tenv pn actuals =
+    actuals_pred actuals
+    &&
     match pn with
     | Typ.Procname.Java java_pname ->
         let classname = Typ.Procname.Java.get_class_type_name java_pname in
@@ -562,25 +565,52 @@ module Models = struct
         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*`.
+                Initially, (Writer|OutputStream).(flush|close) were also matched, but this generated too
+                many reports *)
   let is_blocking_java_io =
     is_call_of_class_or_superclass ["java.io.Reader"; "java.io.InputStream"] ~method_prefix:true
       "read"
 
 
+  (** is the method called CountDownLath.await or on subclass? *)
   let is_countdownlatch_await =
     is_call_of_class_or_superclass ["java.util.concurrent.CountDownLatch"] "await"
 
 
+  (** an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
+           is int-zero then a reply is expected and returned from the remote process, thus potentially
+           blocking.  If the 4th argument is anything else, we assume a one-way call which doesn't block.
+        *)
   let is_two_way_binder_transact =
-    let matcher = is_call_of_class_or_superclass ["android.os.IBinder"] "transact" in
-    fun tenv actuals pn ->
+    let actuals_pred actuals =
       List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero
-      && matcher tenv pn
+    in
+    is_call_of_class_or_superclass ~actuals_pred ["android.os.IBinder"] "transact"
 
 
+  (** is it a call to android.view.View.getWindowVisibleDisplayFrame or on sublass? *)
   let is_getWindowVisibleDisplayFrame =
     is_call_of_class_or_superclass ["android.view.View"] "getWindowVisibleDisplayFrame"
 
 
+  (** 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_accountManager_setUserData =
+    is_call_of_class_or_superclass ["android.accounts.AccountManager"] "setUserData"
+
+
+  let may_block =
+    let matchers =
+      [ is_blocking_java_io
+      ; is_countdownlatch_await
+      ; is_two_way_binder_transact
+      ; is_getWindowVisibleDisplayFrame
+      ; is_future_get
+      ; is_accountManager_setUserData ]
+    in
+    fun tenv pn actuals -> List.exists matchers ~f:(fun matcher -> matcher tenv pn actuals)
 end

infer/src/concurrency/RacerDConfig.mli

@@ -79,25 +79,6 @@ module Models : sig
   (** return true if procedure is at an abstraction boundary or reporting has been explicitly
      requested via @ThreadSafe *)
 
-  val is_blocking_java_io : Tenv.t -> Typ.Procname.t -> bool
-  (** 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*`.
-          Initially, (Writer|OutputStream).(flush|close) were also matched, but this generated too
-          many reports *)
-
-  val is_countdownlatch_await : Tenv.t -> Typ.Procname.t -> bool
-  (** is the method called CountDownLath.await or on subclass? *)
-
-  val is_two_way_binder_transact : Tenv.t -> HilExp.t list -> Typ.Procname.t -> bool
-  (** an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
-           is int-zero then a reply is expected and returned from the remote process, thus potentially
-           blocking.  If the 4th argument is anything else, we assume a one-way call which doesn't block.
-        *)
-
-  val is_getWindowVisibleDisplayFrame : Tenv.t -> Typ.Procname.t -> bool
-  (** is it a call to android.view.View.getWindowVisibleDisplayFrame or on sublass? *)
-
-  val is_future_get : Tenv.t -> Typ.Procname.t -> bool
-  (** is it a call to Future.get() or on sublass? *)
+  val may_block : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool
+  (** is the method call potentially blocking, given the actuals passed? *)
 end

infer/src/concurrency/starvation.ml

@@ -79,13 +79,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
       | LockedIfTrue ->
           astate
       | NoEffect ->
-          if
-            Models.is_countdownlatch_await tenv callee
-            || Models.is_two_way_binder_transact tenv actuals callee
-            || Models.is_blocking_java_io tenv callee
-            || Models.is_getWindowVisibleDisplayFrame tenv callee
-            || Models.is_future_get tenv callee
-          then
+          if Models.may_block tenv callee actuals then
             let caller = Procdesc.get_proc_name pdesc in
             Domain.blocking_call ~caller ~callee loc astate
           else if is_on_main_thread callee then Domain.set_on_main_thread astate

infer/tests/codetoanalyze/java/starvation/AccMgr.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2018 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+import android.accounts.Account;
+import android.accounts.AccountManager;
+import android.support.annotation.UiThread;
+
+class AccMgr {
+  AccountManager am;
+  Account account;
+  String key, data;
+  Object lock;
+
+  @UiThread
+  void onUiThreadBad() throws InterruptedException {
+    am.setUserData(account, key, data);
+  }
+
+  @UiThread
+  void lockOnUiThreadBad() throws InterruptedException {
+    synchronized(lock) {}
+  }
+
+  void setUserDataUnderLock() {
+    synchronized(lock) {
+      am.setUserData(account, key, data);
+    }
+  }
+
+  void onOtherThreadOk() {
+    am.setUserData(account, key, data);
+  }
+}

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

@@ -1,3 +1,5 @@
+codetoanalyze/java/starvation/AccMgr.java, void AccMgr.lockOnUiThreadBad(), 0, STARVATION, ERROR, [[Trace 1]  void AccMgr.lockOnUiThreadBad(),locks `this.AccMgr.lock` in class `AccMgr*`,[Trace 2]  void AccMgr.setUserDataUnderLock(),locks `this.AccMgr.lock` in class `AccMgr*`,calls void AccountManager.setUserData(Account,String,String) from void AccMgr.setUserDataUnderLock()]
+codetoanalyze/java/starvation/AccMgr.java, void AccMgr.onUiThreadBad(), 0, STARVATION, ERROR, [ void AccMgr.onUiThreadBad(),calls void AccountManager.setUserData(Account,String,String) from void AccMgr.onUiThreadBad()]
 codetoanalyze/java/starvation/Binders.java, void Binders.annotationBad(), 0, STARVATION, ERROR, [ void Binders.annotationBad(),Method call: void Binders.doTransact(),calls boolean Binder.transact(int,Parcel,Parcel,int) from void Binders.doTransact()]
 codetoanalyze/java/starvation/Binders.java, void Binders.interBad(), 0, STARVATION, ERROR, [ void Binders.interBad(),calls boolean Binder.transact(int,Parcel,Parcel,int) from void Binders.interBad()]
 codetoanalyze/java/starvation/Binders.java, void Binders.intraBad(), 0, STARVATION, ERROR, [ void Binders.intraBad(),Method call: void Binders.doTransact(),calls boolean Binder.transact(int,Parcel,Parcel,int) from void Binders.doTransact()]