[starvation] Catch indirect blocks to the UI thread

Reviewed By: jeremydubreil

Differential Revision: D7774011

fbshipit-source-id: fe014b6
master
Nikos Gorogiannis 7 years ago committed by Facebook Github Bot
parent d92e82b379
commit dab8e2f17d

@ -143,12 +143,7 @@ let get_summary caller_pdesc callee_pdesc =
let make_trace_with_header ?(header= "") elem start_loc pname =
let trace = StarvationDomain.LockOrder.make_loc_trace elem in
let first_step = List.hd_exn trace in
if Location.equal first_step.Errlog.lt_loc start_loc then
let trace_descr = header ^ first_step.Errlog.lt_description in
Errlog.make_trace_element 0 start_loc trace_descr [] :: List.tl_exn trace
else
let trace_descr = Format.asprintf "%sMethod start: %a" header Typ.Procname.pp pname in
let trace_descr = Format.asprintf "%s %a" header Typ.Procname.pp pname in
Errlog.make_trace_element 0 start_loc trace_descr [] :: trace
@ -157,6 +152,15 @@ let make_loc_trace pname trace_id start_loc elem =
make_trace_with_header ~header elem start_loc pname
let get_summaries_of_methods_in_class get_proc_desc tenv current_pdesc clazz =
let tstruct_opt = Tenv.lookup tenv clazz in
let methods =
Option.value_map tstruct_opt ~default:[] ~f:(fun tstruct -> tstruct.Typ.Struct.methods)
in
let pdescs = List.rev_filter_map methods ~f:get_proc_desc in
List.rev_filter_map pdescs ~f:(get_summary current_pdesc)
(* Note about how many times we report a deadlock: normally twice, at each trace starting point.
Due to the fact we look for deadlocks in the summaries of the class at the root of a path,
this will fail when (a) the lock is of class type (ie as used in static sync methods), because
@ -201,14 +205,8 @@ let report_deadlocks get_proc_desc tenv current_pdesc (summary, _) =
| Some endpoint_class ->
(* get the class of the root variable of the lock in the endpoint event
and retrieve all the summaries of the methods of that class *)
let endpoint_tstruct = Tenv.lookup tenv endpoint_class in
let methods =
Option.value_map endpoint_tstruct ~default:[] ~f:(fun tstruct ->
tstruct.Typ.Struct.methods )
in
let endpoint_pdescs = List.rev_filter_map methods ~f:get_proc_desc in
let endpoint_summaries =
List.rev_filter_map endpoint_pdescs ~f:(get_summary current_pdesc)
get_summaries_of_methods_in_class get_proc_desc tenv current_pdesc endpoint_class
in
(* for each summary related to the endpoint, analyse and report on its pairs *)
List.iter endpoint_summaries ~f:(fun (endpoint_pdesc, (summary, _)) ->
@ -220,13 +218,32 @@ let report_deadlocks get_proc_desc tenv current_pdesc (summary, _) =
LockOrderDomain.iter report_on_current_elem summary
let report_direct_blocks_on_main_thread proc_desc summary =
let report_blocks_on_main_thread get_proc_desc tenv current_pdesc summary =
let open StarvationDomain in
let report_pair ({LockOrder.eventually} as elem) =
let current_loc = Procdesc.get_loc current_pdesc in
let current_pname = Procdesc.get_proc_name current_pdesc in
let report_remote_block current_elem current_lock endpoint_pname endpoint_loc endpoint_elem =
match endpoint_elem with
| { LockOrder.first= Some {LockEvent.event= LockEvent.LockAcquire lock}
; eventually= {LockEvent.event= LockEvent.MayBlock block_descr} }
when LockIdentity.equal current_lock lock ->
let error_message =
Format.asprintf "UI thread %a, which may be held by another thread which %s"
LockIdentity.pp lock block_descr
in
let exn =
Exceptions.Checkers (IssueType.starvation, Localise.verbatim_desc error_message)
in
let first_trace = List.rev (make_loc_trace current_pname 1 current_loc current_elem) in
let second_trace = make_loc_trace endpoint_pname 2 endpoint_loc endpoint_elem in
let ltr = List.rev_append first_trace second_trace in
Reporting.log_error_deprecated ~store_summary:true current_pname ~loc:current_loc ~ltr exn
| _ ->
()
in
let report_on_current_elem ({LockOrder.eventually} as elem) =
match eventually with
| {LockEvent.event= LockEvent.MayBlock _} ->
let current_loc = Procdesc.get_loc proc_desc in
let current_pname = Procdesc.get_proc_name proc_desc in
let error_message =
Format.asprintf "UI-thread method may block; %a" LockEvent.pp_event
eventually.LockEvent.event
@ -236,10 +253,28 @@ let report_direct_blocks_on_main_thread proc_desc summary =
in
let ltr = make_trace_with_header elem current_loc current_pname in
Reporting.log_error_deprecated ~store_summary:true current_pname ~loc:current_loc ~ltr exn
| _ ->
| {LockEvent.event= LockEvent.LockAcquire endpoint_lock} ->
match LockIdentity.owner_class endpoint_lock with
| None ->
()
| Some endpoint_class ->
(* get the class of the root variable of the lock in the endpoint event
and retrieve all the summaries of the methods of that class *)
let endpoint_summaries =
get_summaries_of_methods_in_class get_proc_desc tenv current_pdesc endpoint_class
in
(* for each summary related to the endpoint, analyse and report on its pairs *)
List.iter endpoint_summaries ~f:(fun (endpoint_pdesc, (summary, main)) ->
(* skip methods known to run on ui thread, as they cannot run in parallel to us *)
if main then ()
else
let endpoint_loc = Procdesc.get_loc endpoint_pdesc in
let endpoint_pname = Procdesc.get_proc_name endpoint_pdesc in
LockOrderDomain.iter
(report_remote_block elem endpoint_lock endpoint_pname endpoint_loc)
summary )
in
LockOrderDomain.iter report_pair summary
LockOrderDomain.iter report_on_current_elem summary
let reporting {Callbacks.procedures; get_proc_desc} =
@ -247,6 +282,6 @@ let reporting {Callbacks.procedures; get_proc_desc} =
Summary.read_summary proc_desc (Procdesc.get_proc_name proc_desc)
|> Option.iter ~f:(fun ((s, main) as summary) ->
report_deadlocks get_proc_desc tenv proc_desc summary ;
if main then report_direct_blocks_on_main_thread proc_desc s )
if main then report_blocks_on_main_thread get_proc_desc tenv proc_desc s )
in
List.iter procedures ~f:report_procedure

@ -17,6 +17,8 @@ module LockIdentity : sig
val owner_class : t -> Typ.name option
(** Class of the root variable of the path representing the lock *)
val equal : t -> t -> bool
end
(** A lock event. Equality/comparison disregards the call trace but includes location. *)

@ -0,0 +1,45 @@
/*
* 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.support.annotation.UiThread;
import android.os.Binder;
import android.os.RemoteException;
class IndirectBlock {
Object expensiveLock;
Binder binder;
void takeExpensiveLockOk() {
synchronized(expensiveLock) {}
}
@UiThread
void takeExpensiveLockOnUiThreadBad() {
synchronized(expensiveLock) {}
}
void doTransactUnderLock() throws RemoteException {
synchronized(expensiveLock) {
binder.transact(0, null, null, 0);
}
}
@UiThread
void takeRemoteExpensiveLockOnUiThreadBad(IndirectInterproc i) {
i.takeLock();
}
}
class IndirectInterproc {
synchronized public void takeLock() {}
synchronized public void doTransactUnderLock(Binder binder) throws RemoteException {
binder.transact(0, null, null, 0);
}
}

@ -1,19 +1,21 @@
codetoanalyze/java/starvation/Binders.java, void Binders.annotationBad(), 0, STARVATION, ERROR, [Method start: 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, [Method start: 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, [Method start: 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(), 0, STARVATION, ERROR, [Method start: void Countdwn.awaitOnMainByAnnotBad(),calls void CountDownLatch.await() from void Countdwn.awaitOnMainByAnnotBad()]
codetoanalyze/java/starvation/Countdwn.java, void Countdwn.awaitOnMainByCallBad(), 0, STARVATION, ERROR, [Method start: void Countdwn.awaitOnMainByCallBad(),calls void CountDownLatch.await() from void Countdwn.awaitOnMainByCallBad()]
codetoanalyze/java/starvation/InnerClass.java, InnerClass$InnerClassA.<init>(InnerClass,Object), 0, STARVATION, ERROR, [[Trace 1] Method start: InnerClass$InnerClassA.<init>(InnerClass,Object),locks `this` in class `InnerClass$InnerClassA*`,Method call: void InnerClass.bar(),locks `this` in class `InnerClass*`,[Trace 2] locks `this` in class `InnerClass*`,Method call: void InnerClass$InnerClassA.baz(),locks `this` in class `InnerClass$InnerClassA*`]
codetoanalyze/java/starvation/InnerClass.java, void InnerClass$InnerClassA.innerOuterBad(), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `InnerClass$InnerClassA*`,Method call: void InnerClass.bar(),locks `this` in class `InnerClass*`,[Trace 2] locks `this` in class `InnerClass*`,Method call: void InnerClass$InnerClassA.baz(),locks `this` in class `InnerClass$InnerClassA*`]
codetoanalyze/java/starvation/InnerClass.java, void InnerClass.outerInnerBad(InnerClass$InnerClassA), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `InnerClass*`,Method call: void InnerClass$InnerClassA.baz(),locks `this` in class `InnerClass$InnerClassA*`,[Trace 2] locks `this` in class `InnerClass$InnerClassA*`,Method call: void InnerClass.bar(),locks `this` in class `InnerClass*`]
codetoanalyze/java/starvation/InnerClass.java, void InnerClass.outerInnerBad(InnerClass$InnerClassA), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `InnerClass*`,Method call: void InnerClass$InnerClassA.baz(),locks `this` in class `InnerClass$InnerClassA*`,[Trace 2] Method start: InnerClass$InnerClassA.<init>(InnerClass,Object),locks `this` in class `InnerClass$InnerClassA*`,Method call: void InnerClass.bar(),locks `this` in class `InnerClass*`]
codetoanalyze/java/starvation/Interclass.java, void Interclass.interclass1Bad(InterclassA), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `Interclass*`,Method call: void InterclassA.interclass1Bad(),locks `this` in class `InterclassA*`,[Trace 2] locks `this` in class `InterclassA*`,Method call: void Interclass.interclass2Bad(),locks `this` in class `Interclass*`]
codetoanalyze/java/starvation/Interclass.java, void InterclassA.interclass2Bad(Interclass), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `InterclassA*`,Method call: void Interclass.interclass2Bad(),locks `this` in class `Interclass*`,[Trace 2] locks `this` in class `Interclass*`,Method call: void InterclassA.interclass1Bad(),locks `this` in class `InterclassA*`]
codetoanalyze/java/starvation/Interproc.java, void Interproc.interproc1Bad(InterprocA), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `Interproc*`,Method call: void Interproc.interproc2Bad(InterprocA),locks `b` in class `InterprocA*`,[Trace 2] locks `this` in class `InterprocA*`,Method call: void InterprocA.interproc2Bad(Interproc),locks `d` in class `Interproc*`]
codetoanalyze/java/starvation/Interproc.java, void InterprocA.interproc1Bad(Interproc), 0, STARVATION, ERROR, [[Trace 1] locks `this` in class `InterprocA*`,Method call: void InterprocA.interproc2Bad(Interproc),locks `d` in class `Interproc*`,[Trace 2] locks `this` in class `Interproc*`,Method call: void Interproc.interproc2Bad(InterprocA),locks `b` in class `InterprocA*`]
codetoanalyze/java/starvation/Intraproc.java, void Intraproc.intraBad(IntraprocA), 0, STARVATION, ERROR, [[Trace 1] Method start: void Intraproc.intraBad(IntraprocA),locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`,[Trace 2] Method start: void IntraprocA.intraBad(Intraproc),locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`]
codetoanalyze/java/starvation/Intraproc.java, void IntraprocA.intraBad(Intraproc), 0, STARVATION, ERROR, [[Trace 1] Method start: void IntraprocA.intraBad(Intraproc),locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`,[Trace 2] Method start: void Intraproc.intraBad(IntraprocA),locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`]
codetoanalyze/java/starvation/JavaIO.java, void JavaIO.fileReadBad(), 0, STARVATION, ERROR, [Method start: void JavaIO.fileReadBad(),Method call: int JavaIO.doFileRead(),calls int InputStreamReader.read() from int JavaIO.doFileRead()]
codetoanalyze/java/starvation/JavaIO.java, void JavaIO.streamReadBad(), 0, STARVATION, ERROR, [Method start: void JavaIO.streamReadBad(),Method call: String JavaIO.doStreamRead(),calls String DataInputStream.readUTF() from String JavaIO.doStreamRead()]
codetoanalyze/java/starvation/StaticLock.java, void StaticLock.lockOtherClassOneWayBad(), 0, STARVATION, ERROR, [[Trace 1] Method start: void StaticLock.lockOtherClassOneWayBad(),locks `StaticLock$0` in class `java.lang.Class*`,locks `this` in class `StaticLock*`,[Trace 2] locks `this` in class `StaticLock*`,Method call: void StaticLock.staticSynced(),locks `StaticLock$0` in class `java.lang.Class*`]
codetoanalyze/java/starvation/VisDispFrame.java, void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(), 0, STARVATION, ERROR, [Method start: void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(),calls void View.getWindowVisibleDisplayFrame(Rect) from void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad()]
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()]
codetoanalyze/java/starvation/Countdwn.java, void Countdwn.awaitOnMainByAnnotBad(), 0, STARVATION, ERROR, [ void Countdwn.awaitOnMainByAnnotBad(),calls void CountDownLatch.await() from void Countdwn.awaitOnMainByAnnotBad()]
codetoanalyze/java/starvation/Countdwn.java, void Countdwn.awaitOnMainByCallBad(), 0, STARVATION, ERROR, [ void Countdwn.awaitOnMainByCallBad(),calls void CountDownLatch.await() from void Countdwn.awaitOnMainByCallBad()]
codetoanalyze/java/starvation/IndirectBlock.java, void IndirectBlock.takeExpensiveLockOnUiThreadBad(), 0, 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), 0, 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), 0, STARVATION, 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*`]
codetoanalyze/java/starvation/InnerClass.java, void InnerClass$InnerClassA.innerOuterBad(), 0, STARVATION, ERROR, [[Trace 1] void InnerClass$InnerClassA.innerOuterBad(),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*`]
codetoanalyze/java/starvation/InnerClass.java, void InnerClass.outerInnerBad(InnerClass$InnerClassA), 0, STARVATION, ERROR, [[Trace 1] void InnerClass.outerInnerBad(InnerClass$InnerClassA),locks `this` in class `InnerClass*`,Method call: void InnerClass$InnerClassA.baz(),locks `this` in class `InnerClass$InnerClassA*`,[Trace 2] void InnerClass$InnerClassA.innerOuterBad(),locks `this` in class `InnerClass$InnerClassA*`,Method call: void InnerClass.bar(),locks `this` in class `InnerClass*`]
codetoanalyze/java/starvation/InnerClass.java, void InnerClass.outerInnerBad(InnerClass$InnerClassA), 0, STARVATION, ERROR, [[Trace 1] void InnerClass.outerInnerBad(InnerClass$InnerClassA),locks `this` in class `InnerClass*`,Method call: void InnerClass$InnerClassA.baz(),locks `this` in class `InnerClass$InnerClassA*`,[Trace 2] InnerClass$InnerClassA.<init>(InnerClass,Object),locks `this` in class `InnerClass$InnerClassA*`,Method call: void InnerClass.bar(),locks `this` in class `InnerClass*`]
codetoanalyze/java/starvation/Interclass.java, void Interclass.interclass1Bad(InterclassA), 0, STARVATION, ERROR, [[Trace 1] void Interclass.interclass1Bad(InterclassA),locks `this` in class `Interclass*`,Method call: void InterclassA.interclass1Bad(),locks `this` in class `InterclassA*`,[Trace 2] void InterclassA.interclass2Bad(Interclass),locks `this` in class `InterclassA*`,Method call: void Interclass.interclass2Bad(),locks `this` in class `Interclass*`]
codetoanalyze/java/starvation/Interclass.java, void InterclassA.interclass2Bad(Interclass), 0, STARVATION, ERROR, [[Trace 1] void InterclassA.interclass2Bad(Interclass),locks `this` in class `InterclassA*`,Method call: void Interclass.interclass2Bad(),locks `this` in class `Interclass*`,[Trace 2] void Interclass.interclass1Bad(InterclassA),locks `this` in class `Interclass*`,Method call: void InterclassA.interclass1Bad(),locks `this` in class `InterclassA*`]
codetoanalyze/java/starvation/Interproc.java, void Interproc.interproc1Bad(InterprocA), 0, STARVATION, ERROR, [[Trace 1] void Interproc.interproc1Bad(InterprocA),locks `this` in class `Interproc*`,Method call: void Interproc.interproc2Bad(InterprocA),locks `b` in class `InterprocA*`,[Trace 2] void InterprocA.interproc1Bad(Interproc),locks `this` in class `InterprocA*`,Method call: void InterprocA.interproc2Bad(Interproc),locks `d` in class `Interproc*`]
codetoanalyze/java/starvation/Interproc.java, void InterprocA.interproc1Bad(Interproc), 0, STARVATION, ERROR, [[Trace 1] void InterprocA.interproc1Bad(Interproc),locks `this` in class `InterprocA*`,Method call: void InterprocA.interproc2Bad(Interproc),locks `d` in class `Interproc*`,[Trace 2] void Interproc.interproc1Bad(InterprocA),locks `this` in class `Interproc*`,Method call: void Interproc.interproc2Bad(InterprocA),locks `b` in class `InterprocA*`]
codetoanalyze/java/starvation/Intraproc.java, void Intraproc.intraBad(IntraprocA), 0, STARVATION, ERROR, [[Trace 1] void Intraproc.intraBad(IntraprocA),locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`,[Trace 2] void IntraprocA.intraBad(Intraproc),locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`]
codetoanalyze/java/starvation/Intraproc.java, void IntraprocA.intraBad(Intraproc), 0, STARVATION, ERROR, [[Trace 1] void IntraprocA.intraBad(Intraproc),locks `this` in class `IntraprocA*`,locks `o` in class `Intraproc*`,[Trace 2] void Intraproc.intraBad(IntraprocA),locks `this` in class `Intraproc*`,locks `o` in class `IntraprocA*`]
codetoanalyze/java/starvation/JavaIO.java, void JavaIO.fileReadBad(), 0, STARVATION, ERROR, [ void JavaIO.fileReadBad(),Method call: int JavaIO.doFileRead(),calls int InputStreamReader.read() from int JavaIO.doFileRead()]
codetoanalyze/java/starvation/JavaIO.java, void JavaIO.streamReadBad(), 0, STARVATION, ERROR, [ void JavaIO.streamReadBad(),Method call: String JavaIO.doStreamRead(),calls String DataInputStream.readUTF() from String JavaIO.doStreamRead()]
codetoanalyze/java/starvation/StaticLock.java, void StaticLock.lockOtherClassOneWayBad(), 0, STARVATION, ERROR, [[Trace 1] void StaticLock.lockOtherClassOneWayBad(),locks `StaticLock$0` in class `java.lang.Class*`,locks `this` in class `StaticLock*`,[Trace 2] void StaticLock.lockOtherClassAnotherWayNad(),locks `this` in class `StaticLock*`,Method call: void StaticLock.staticSynced(),locks `StaticLock$0` in class `java.lang.Class*`]
codetoanalyze/java/starvation/VisDispFrame.java, void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(), 0, STARVATION, ERROR, [ void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(),calls void View.getWindowVisibleDisplayFrame(Rect) from void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad()]

Loading…
Cancel
Save