[starvation][whole-program] allow scheduled work on unknown threads

Summary: When we see a call to schedule some work on an executor and we don't have evidence that it is on some specific thread (UI/BG), instead of dropping the work, assign it `UnknownThread` and treat it as running on the background by default.

Reviewed By: jvillard

Differential Revision: D18615649

fbshipit-source-id: e8bad64b6

infer/src/concurrency/StarvationModels.ml

@@ -223,7 +223,8 @@ let schedules_work_on_ui_thread =
   fun tenv pname -> matcher tenv pname []
 
 
-type executor_thread_constraint = ForUIThread | ForNonUIThread [@@deriving equal]
+type executor_thread_constraint = ForUIThread | ForNonUIThread | ForUnknownThread
+[@@deriving equal]
 
 (* Executors are usually stored in fields and annotated according to what type of thread 
    they schedule work on.  Given an expression representing such a field, try to find the kind of 

infer/src/concurrency/StarvationModels.mli

@@ -36,11 +36,13 @@ val schedules_work : Tenv.t -> Typ.Procname.t -> bool
 (** call known to schedule runnable first argument to some thread/executor *)
 
 (** an instance field holding a reference to an executor may be annotated as running on UI/non-UI thread *)
-type executor_thread_constraint = ForUIThread | ForNonUIThread [@@deriving equal]
+type executor_thread_constraint = ForUIThread | ForNonUIThread | ForUnknownThread
+[@@deriving equal]
 
 val get_executor_thread_constraint :
   Tenv.t -> HilExp.AccessExpression.t -> executor_thread_constraint option
-(** given an executor receiver, get its thread constraint, if any *)
+(** given an executor receiver, get its thread constraint, if any. [None] means lookup somehow failed, 
+    whereas [Some UnknownThread] means the receiver is an unannotated executor. *)
 
 val get_run_method_from_runnable : Tenv.t -> HilExp.AccessExpression.t -> Typ.Procname.t option
 (** given a receiver, find the [run()] method in the appropriate class *)

infer/src/concurrency/starvation.ml

@@ -99,7 +99,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
             [ (* match an executor call that we have a model for *)
               lazy (StarvationModels.get_executor_thread_constraint tenv executor)
             ; (* match an executor call where the executor is stored in a variable *)
-              lazy (Domain.AttributeDomain.get_executor_constraint executor astate.attributes) ]
+              lazy (Domain.AttributeDomain.get_executor_constraint executor astate.attributes)
+            ; (* the receiver is an executor stored in a variable for which we have no knowledge *)
+              lazy (Some StarvationModels.ForUnknownThread) ]
         in
         Option.fold thread_opt ~init:astate ~f:(schedule_work runnable)
     | HilExp.AccessExpression runnable :: _

infer/src/concurrency/starvationDomain.ml

@@ -460,7 +460,9 @@ module Attribute = struct
     | Executor StarvationModels.ForUIThread ->
         "Executor(UI)"
     | Executor StarvationModels.ForNonUIThread ->
-        "Executor(BG)" )
+        "Executor(BG)"
+    | Executor StarvationModels.ForUnknownThread ->
+        "Executor(Unknown)" )
     |> F.pp_print_string fmt
 
 
@@ -630,12 +632,14 @@ let filter_blocking_calls ({critical_pairs} as astate) =
 
 
 let schedule_work loc thread_constraint astate procname =
-  let thread =
+  let thread : ThreadDomain.t =
-    match thread_constraint with
+    match (thread_constraint : StarvationModels.executor_thread_constraint) with
-    | StarvationModels.ForUIThread ->
+    | ForUIThread ->
-        ThreadDomain.UIThread
+        UIThread
-    | StarvationModels.ForNonUIThread ->
+    | ForNonUIThread ->
-        ThreadDomain.BGThread
+        BGThread
+    | ForUnknownThread ->
+        UnknownThread
   in
   let work_item = ScheduledWorkItem.{procname; loc; thread} in
   {astate with scheduled_work= ScheduledWorkDomain.add work_item astate.scheduled_work}

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

@@ -0,0 +1,106 @@
+/*
+ * 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.os.Binder;
+import android.os.RemoteException;
+import java.util.concurrent.Executor;
+
+// we treat executors of unknown thread as implicitly running in the background
+
+class UnknownThread {
+  static Binder binder;
+
+  private static void doTransact() {
+    try {
+      binder.transact(0, null, null, 0);
+    } catch (RemoteException e) {
+    }
+  }
+
+  @ForUiThread private final Executor mUiThreadExecutor = null;
+  @ForNonUiThread private final Executor mNonUiThreadExecutor = null;
+  Executor unknownThreadExecutor = null;
+
+  private static Executor getSomeExecutor() {
+    return null;
+  }
+
+  public void postBlockingCallToUnknownExecutorFieldOk() {
+    unknownThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            doTransact();
+          }
+        });
+  }
+
+  public void postBlockingCallToUnknownExecutorViaMethodOk() {
+    getSomeExecutor()
+        .execute(
+            new Runnable() {
+              @Override
+              public void run() {
+                doTransact();
+              }
+            });
+  }
+
+  Object monitorA, monitorB;
+
+  // text-book deadlock between unknown and background thread
+  public void postDeadlockToUnknownAndBackgroundBad() {
+    unknownThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (monitorA) {
+              synchronized (monitorB) {
+              }
+            }
+          }
+        });
+
+    mNonUiThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (monitorB) {
+              synchronized (monitorA) {
+              }
+            }
+          }
+        });
+  }
+
+  Object monitorC, monitorD;
+
+  // text-book deadlock between unknown and background thread
+  public void postDeadlockToUIAndBackgroundBad() {
+    unknownThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (monitorC) {
+              synchronized (monitorD) {
+              }
+            }
+          }
+        });
+
+    mUiThreadExecutor.execute(
+        new Runnable() {
+          @Override
+          public void run() {
+            synchronized (monitorD) {
+              synchronized (monitorC) {
+              }
+            }
+          }
+        });
+  }
+}

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

@@ -20,3 +20,7 @@ codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceC
 codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onNullBinding(android.content.ComponentName):void, 31, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onNullBinding(ComponentName)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onServiceConnected(android.content.ComponentName,android.os.IBinder):void, 36, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onServiceConnected(ComponentName,IBinder)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onServiceDisconnected(android.content.ComponentName):void, 41, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onServiceDisconnected(ComponentName)`,Method call: `void MyServiceConnection.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
+codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUIAndBackgroundBad():void, 84, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$5.run()`, locks `this.monitorC` in `class UnknownThread`, locks `this.monitorD` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$6.run()`, locks `this.monitorD` in `class UnknownThread`, locks `this.monitorC` in `class UnknownThread`]
+codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUIAndBackgroundBad():void, 95, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$6.run()`, locks `this.monitorD` in `class UnknownThread`, locks `this.monitorC` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUIAndBackgroundBad()`,Method call: `void UnknownThread$5.run()`, locks `this.monitorC` in `class UnknownThread`, locks `this.monitorD` in `class UnknownThread`]
+codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUnknownAndBackgroundBad():void, 57, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUnknownAndBackgroundBad()`,Method call: `void UnknownThread$3.run()`, locks `this.monitorA` in `class UnknownThread`, locks `this.monitorB` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUnknownAndBackgroundBad()`,Method call: `void UnknownThread$4.run()`, locks `this.monitorB` in `class UnknownThread`, locks `this.monitorA` in `class UnknownThread`]
+codetoanalyze/java/starvation-whole-program/UnknownThread.java, UnknownThread.postDeadlockToUnknownAndBackgroundBad():void, 68, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UnknownThread.postDeadlockToUnknownAndBackgroundBad()`,Method call: `void UnknownThread$4.run()`, locks `this.monitorB` in `class UnknownThread`, locks `this.monitorA` in `class UnknownThread`,[Trace 2] `void UnknownThread.postDeadlockToUnknownAndBackgroundBad()`,Method call: `void UnknownThread$3.run()`, locks `this.monitorA` in `class UnknownThread`, locks `this.monitorB` in `class UnknownThread`]