[concurrency] android methods starting with "on" are on UI thread

Summary:
Android OS calls certain overridden class methods always on the UI thread.  The function changed here attempted to build a list of all these methods, one by one.  It's much more complete to simply consider a method as callable on the UI thread if it's an override of an Android library method, and it starts with "on".  Only a single instance is known not to obey this pattern, so it's easier to blacklist than to whitelist.

Also clarify the name to `is_android_lifecycle_method`.

Reviewed By: ezgicicek

Differential Revision: D21703365

fbshipit-source-id: 41ca3e998

infer/src/absint/ConcurrencyModels.ml

@@ -307,52 +307,36 @@ let get_current_class_and_annotated_superclasses is_annot tenv pname =
       None
 
 
-let ui_matcher_records =
+let is_android_lifecycle_method tenv pname =
-  let open MethodMatcher in
+  let package_starts_with_android procname =
-  let fragment_methods =
+    Procname.get_class_type_name procname
-    (* sort police: this is in lifecycle order *)
+    |> Option.exists ~f:(fun typename ->
-    [ "onAttach"
+           match (typename : Typ.Name.t) with
-    ; "onCreate"
+           | CUnion _ | CStruct _ | CppClass _ | ObjcClass _ | ObjcProtocol _ ->
-    ; "onCreateView"
+               false
-    ; "onActivityCreated"
+           | JavaClass java_class_name ->
-    ; "onStart"
+               JavaClassName.package java_class_name
-    ; "onResume"
+               |> Option.exists ~f:(String.is_prefix ~prefix:"android") )
-    ; "onPause"
+  in
-    ; "onStop"
+  let overrides_android_method tenv pname =
-    ; "onDestroyView"
+    PatternMatch.override_exists package_starts_with_android tenv pname
-    ; "onDestroy"
-    ; "onDetach" ]
   in
-  (* search_superclasses is true by default in how [default] is treated *)
+  let method_starts_with_on pname = Procname.get_method pname |> String.is_prefix ~prefix:"on" in
-  [ {default with classname= "android.support.v4.app.Fragment"; methods= fragment_methods}
+  let is_whitelisted pname =
-  ; {default with classname= "android.app.Fragment"; methods= fragment_methods}
+    match Procname.get_method pname with
-  ; {default with classname= "androidx.fragment.app.Fragment"; methods= fragment_methods}
+    (* [IntentService.onHandleIntent] is an exception *)
-  ; {default with classname= "android.content.ContentProvider"; methods= ["onCreate"]}
+    | "onHandleIntent" ->
-  ; {default with classname= "android.content.BroadcastReceiver"; methods= ["onReceive"]}
+        true
-  ; { default with
+    | _ ->
-      classname= "android.app.Service"
+        false
-    ; methods= ["onBind"; "onCreate"; "onDestroy"; "onStartCommand"] }
+  in
-  ; {default with classname= "android.app.Application"; methods= ["onCreate"]}
+  match (pname : Procname.t) with
-  ; { default with
+  | C _ | Linters_dummy_method | Block _ | ObjC_Cpp _ | WithBlockParameters _ ->
-      classname= "android.app.Activity"
+      false
-    ; methods= ["onCreate"; "onStart"; "onRestart"; "onResume"; "onPause"; "onStop"; "onDestroy"] }
+  | Java _ ->
-  ; { default with
+      method_starts_with_on pname
-      (* according to Android documentation, *all* methods of the View class run on UI thread, but
+      && (not (is_whitelisted pname))
-         let's be a bit conservative and catch all methods that start with "on".
+      && overrides_android_method tenv pname
-         https://developer.android.com/reference/android/view/View.html *)
-      method_prefix= true
-    ; classname= "android.view.View"
-    ; methods= ["on"] }
-  ; { default with
-      classname= "android.content.ServiceConnection"
-    ; methods= ["onBindingDied"; "onNullBinding"; "onServiceConnected"; "onServiceDisconnected"] }
-  ]
-
-
-let is_modeled_ui_method =
-  let matchers = List.map ui_matcher_records ~f:MethodMatcher.of_record in
-  (* we pass an empty actuals list because all matching is done on class and method name here *)
-  fun tenv pname -> MethodMatcher.of_list matchers tenv pname []
 
 
 type annotation_trail = DirectlyAnnotated | Override of Procname.t | SuperClass of Typ.name
@@ -392,7 +376,7 @@ let annotated_as_uithread_equivalent tenv pname =
 
 
 let runs_on_ui_thread tenv pname =
-  is_modeled_ui_method tenv pname || annotated_as_uithread_equivalent tenv pname
+  is_android_lifecycle_method tenv pname || annotated_as_uithread_equivalent tenv pname
 
 
 let is_recursive_lock_type = function

infer/src/absint/ConcurrencyModels.mli

@@ -55,5 +55,5 @@ val runs_on_ui_thread : Tenv.t -> Procname.t -> bool
 (** is method not transitively annotated [@WorkerThread] and is modeled or annotated [@UIThread] or
     equivalent? *)
 
-val is_modeled_ui_method : Tenv.t -> Procname.t -> bool
+val is_android_lifecycle_method : Tenv.t -> Procname.t -> bool
 (** is method a known Android UI thread callback (eg [Activity.onCreate]) *)

infer/src/backend/StarvationGlobalAnalysis.ml

@@ -29,7 +29,7 @@ let iter_summary ~f exe_env (summary : Summary.t) =
   |> 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 ;
+         if ConcurrencyModels.is_android_lifecycle_method tenv pname then f pname critical_pairs ;
          ScheduledWorkDomain.iter
            (fun work -> get_summary_of_scheduled_work work |> Option.iter ~f:(f pname))
            scheduled_work )

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

@@ -23,6 +23,7 @@ class MyServiceConnection implements ServiceConnection {
 
   // implemented/overrides so no Bad suffixes
 
+  // following two methods are FNs due to a separate issue with default interface methods
   void onBindingDied(ComponentName name) {
     bad();
   }

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

@@ -29,10 +29,8 @@ codetoanalyze/java/starvation-whole-program/MyActivity.java, MyActivity.onResume
 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.this$0.FP_monitorE` in `class MyActivity$3`, locks `this.this$0.FP_monitorD` in `class MyActivity$3`,[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.this$0.monitorA` in `class MyActivity$1`,Method call: `void MyActivity.bad()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
-codetoanalyze/java/starvation-whole-program/MyServiceConnection.java, MyServiceConnection.onBindingDied(android.content.ComponentName):void, 27, STARVATION, no_bucket, ERROR, [`void MyServiceConnection.onBindingDied(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, 37, 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.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.onServiceDisconnected(android.content.ComponentName):void, 42, 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/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/StaticInitAttributes.java, StaticInitAttributes.postBlockingCallToUIExecutorBad():void, 52, STARVATION, no_bucket, ERROR, [`void StaticInitAttributes.postBlockingCallToUIExecutorBad()`,Method call: `void StaticInitAttributes$1.run()`,Method call: `void StaticInitAttributes.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/StaticInitAttributes.java, StaticInitAttributes.postBlockingCallToUIHandlerBad():void, 64, STARVATION, no_bucket, ERROR, [`void StaticInitAttributes.postBlockingCallToUIHandlerBad()`,Method call: `void StaticInitAttributes$1.run()`,Method call: `void StaticInitAttributes.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]
 codetoanalyze/java/starvation-whole-program/ThreadScheduling.java, ThreadScheduling.scheduleBlockingCallOnContendedLockBad():void, 36, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadScheduling.scheduleBlockingCallOnContendedLockBad()`,Method call: `void ThreadScheduling$2.run()`, locks `this.this$0.monitorA` in `class ThreadScheduling$2`,[Trace 2] `void ThreadScheduling.scheduleBlockingCallOnContendedLockBad()`,Method call: `void ThreadScheduling$1.run()`, locks `this.this$0.monitorA` in `class ThreadScheduling$1`,Method call: `void ThreadScheduling.doTransact()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)`]