[starvation-whole-program] recognize Android callback methods as scheduled work

Summary: Android may spontaneously call these methods on the UI thread, so recognize the fact.

Reviewed By: ezgicicek

Differential Revision: D18530477

fbshipit-source-id: a8a798779

infer/src/concurrency/ConcurrencyModels.mli

@@ -61,3 +61,6 @@ val annotated_as_worker_thread :
 val runs_on_ui_thread :
   attrs_of_pname:(Typ.Procname.t -> ProcAttributes.t option) -> Tenv.t -> Typ.Procname.t -> bool
 (** is method not transitively annotated @WorkerThread and is modeled or annotated @UIThread or equivalent? *)
+
+val is_modeled_ui_method : Tenv.t -> Typ.Procname.t -> bool
+(** is method a known Android UI thread callback (eg [Activity.onCreate]) *)

infer/src/concurrency/starvation.ml

@@ -578,13 +578,15 @@ let get_summary_of_scheduled_work (work_item : Domain.ScheduledWorkItem.t) =
 
 (* given a summary, do [f work critical_pairs] for each [work] item scheduled in the summary,
    where [critical_pairs] are those of the method scheduled, adapted to the thread it's scheduled for *)
-let iter_summary ~f (summary : Summary.t) =
-  let caller = Summary.get_proc_name summary in
+let iter_summary ~f exe_env (summary : Summary.t) =
   let open Domain in
   Payloads.starvation summary.payloads
-  |> Option.iter ~f:(fun ({scheduled_work} : summary) ->
+  |> Option.iter ~f:(fun ({scheduled_work; critical_pairs} : summary) ->
+         let pname = Summary.get_proc_name summary in
+         let tenv = Exe_env.get_tenv exe_env pname in
+         if ConcurrencyModels.is_modeled_ui_method tenv pname then f pname critical_pairs ;
          ScheduledWorkDomain.iter
-           (fun work -> get_summary_of_scheduled_work work |> Option.iter ~f:(f caller))
+           (fun work -> get_summary_of_scheduled_work work |> Option.iter ~f:(f pname))
            scheduled_work )
 
 
@@ -636,7 +638,7 @@ let whole_program_analysis () =
   let work_set = WorkHashSet.create 1 in
   let exe_env = Exe_env.mk () in
   L.progress "Processing on-disk summaries...@." ;
-  SpecsFiles.iter ~f:(iter_summary ~f:(WorkHashSet.add_pairs work_set)) ;
+  SpecsFiles.iter ~f:(iter_summary exe_env ~f:(WorkHashSet.add_pairs work_set)) ;
   L.progress "Loaded %d pairs@." (WorkHashSet.length work_set) ;
   L.progress "Reporting on processed summaries...@." ;
   report exe_env work_set

infer/tests/codetoanalyze/java/starvation-whole-program/MyActivity.java

@@ -0,0 +1,111 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import android.app.Activity;
+import android.os.Binder;
+import android.os.Bundle;
+import android.os.RemoteException;
+import java.util.concurrent.Executor;
+
+class MyActivity extends Activity {
+  Binder b;
+
+  private void bad() {
+    try {
+      b.transact(0, null, null, 0);
+    } catch (RemoteException r) {
+    }
+  }
+
+  // overrides so no Bad suffixes
+
+  @Override
+  protected void onCreate(Bundle savedInstanceState) {
+    bad();
+  }
+
+  @Override
+  public void onStart() {
+    bad();
+  }
+
+  @Override
+  public void onRestart() {
+    bad();
+  }
+
+  Object monitorA;
+  @ForNonUiThread private final Executor mNonUiThreadExecutor = null;
+
+  // method is a UI thread callback, and schedules a transaction in the background
+  // but it synchronises on the lock protecting the transaction, thus stalling the main thread
+  @Override
+  public void onStop() {
+    synchronized (monitorA) {
+    }
+
+    mNonUiThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (monitorA) {
+              bad();
+            }
+          }
+        });
+  }
+
+  Object monitorB, monitorC;
+
+  // method is a UI thread callback and deadlocks with work scheduled in
+  // another callback (onPause) but which schedules work to a background thread
+  @Override
+  public void onDestroy() {
+    synchronized (monitorC) {
+      synchronized (monitorB) {
+      }
+    }
+  }
+
+  @Override
+  public void onPause() {
+    mNonUiThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (monitorB) {
+              synchronized (monitorC) {
+              }
+            }
+          }
+        });
+  }
+
+  Object FP_monitorD, FP_monitorE;
+
+  // False positive: by the time the work is scheduled, no lock is held, so no deadlock
+  // Locks are named FP_* so that the report is clearly an FP (we can't change the name of the
+  // override).
+  @Override
+  public void onResume() {
+    synchronized (FP_monitorD) {
+      synchronized (FP_monitorE) {
+      }
+    }
+
+    mNonUiThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (FP_monitorE) {
+              synchronized (FP_monitorD) {
+              }
+            }
+          }
+        });
+  }
+}

infer/tests/codetoanalyze/java/starvation-whole-program/issues.exp

@@ -5,3 +5,11 @@ codetoanalyze/java/starvation-whole-program/Deadlock.java, Deadlock.postOnBGThre
 codetoanalyze/java/starvation-whole-program/DirectStarvation.java, DirectStarvation.postBlockingCallToUIThreadBad():void, 29, STARVATION, no_bucket, ERROR, [`void DirectStarvation.postBlockingCallToUIThreadBad()`,Method call: `void DirectStarvation$1.run()`,Method call: `void DirectStarvation.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/IndirectStarvation.java, IndirectStarvation.postBlockingCallToBackgroundThreadAndLockBad():void, 32, STARVATION, no_bucket, ERROR, [[Trace 1] `void IndirectStarvation.postBlockingCallToBackgroundThreadAndLockBad()`,Method call: `void IndirectStarvation$1.run()`, locks `this.monitorA` in `class IndirectStarvation`,[Trace 2] `void IndirectStarvation.postBlockingCallToBackgroundThreadAndLockBad()`,Method call: `void IndirectStarvation$2.run()`, locks `this.monitorA` in `class IndirectStarvation`,Method call: `void IndirectStarvation.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/ModeledExecutors.java, ModeledExecutors.postBlockingCallToUIThreadBad():void, 25, STARVATION, no_bucket, ERROR, [`void ModeledExecutors.postBlockingCallToUIThreadBad()`,Method call: `void ModeledExecutors$1.run()`,Method call: `void ModeledExecutors.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onCreate(android.os.Bundle):void, 28, STARVATION, no_bucket, ERROR, [`void MyActivity.onCreate(Bundle)`,Method call: `void MyActivity.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onDestroy():void, 68, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void MyActivity.onDestroy()`, locks `this.monitorC` in `class MyActivity`, locks `this.monitorB` in `class MyActivity`,[Trace 2] `void MyActivity.onPause()`,Method call: `void MyActivity$2.run()`, locks `this.monitorB` in `class MyActivity`, locks `this.monitorC` in `class MyActivity`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onPause():void, 76, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void MyActivity.onPause()`,Method call: `void MyActivity$2.run()`, locks `this.monitorB` in `class MyActivity`, locks `this.monitorC` in `class MyActivity`,[Trace 2] `void MyActivity.onDestroy()`, locks `this.monitorC` in `class MyActivity`, locks `this.monitorB` in `class MyActivity`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onRestart():void, 38, STARVATION, no_bucket, ERROR, [`void MyActivity.onRestart()`,Method call: `void MyActivity.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onResume():void, 95, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void MyActivity.onResume()`, locks `this.FP_monitorD` in `class MyActivity`, locks `this.FP_monitorE` in `class MyActivity`,[Trace 2] `void MyActivity.onResume()`,Method call: `void MyActivity$3.run()`, locks `this.FP_monitorE` in `class MyActivity`, locks `this.FP_monitorD` in `class MyActivity`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onResume():void, 100, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void MyActivity.onResume()`,Method call: `void MyActivity$3.run()`, locks `this.FP_monitorE` in `class MyActivity`, locks `this.FP_monitorD` in `class MyActivity`,[Trace 2] `void MyActivity.onResume()`, locks `this.FP_monitorD` in `class MyActivity`, locks `this.FP_monitorE` in `class MyActivity`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onStart():void, 33, STARVATION, no_bucket, ERROR, [`void MyActivity.onStart()`,Method call: `void MyActivity.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onStop():void, 48, STARVATION, no_bucket, ERROR, [[Trace 1] `void MyActivity.onStop()`, locks `this.monitorA` in `class MyActivity`,[Trace 2] `void MyActivity.onStop()`,Method call: `void MyActivity$1.run()`, locks `this.monitorA` in `class MyActivity`,Method call: `void MyActivity.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]