diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index ae8b33492..ae1efc6f8 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -563,24 +563,24 @@ module Models = struct let is_blocking_java_io = - let target_classes = ["java.io.Reader"; "java.io.InputStream"] in - fun tenv pn -> is_call_of_class_or_superclass target_classes ~method_prefix:true "read" tenv pn + is_call_of_class_or_superclass ["java.io.Reader"; "java.io.InputStream"] ~method_prefix:true + "read" let is_countdownlatch_await = - let target_classes = ["java.util.concurrent.CountDownLatch"] in - fun tenv pn -> is_call_of_class_or_superclass target_classes "await" tenv pn + is_call_of_class_or_superclass ["java.util.concurrent.CountDownLatch"] "await" let is_two_way_binder_transact = - let target_classes = ["android.os.IBinder"] in + let matcher = is_call_of_class_or_superclass ["android.os.IBinder"] "transact" in fun tenv actuals pn -> List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero - && is_call_of_class_or_superclass target_classes "transact" tenv pn + && matcher tenv pn let is_getWindowVisibleDisplayFrame = - let target_classes = ["android.view.View"] in - fun tenv pn -> - is_call_of_class_or_superclass target_classes "getWindowVisibleDisplayFrame" tenv pn + is_call_of_class_or_superclass ["android.view.View"] "getWindowVisibleDisplayFrame" + + + let is_future_get = is_call_of_class_or_superclass ["java.util.concurrent.Future"] "get" end diff --git a/infer/src/concurrency/RacerDConfig.mli b/infer/src/concurrency/RacerDConfig.mli index 584fc0280..cd0ba4f7e 100644 --- a/infer/src/concurrency/RacerDConfig.mli +++ b/infer/src/concurrency/RacerDConfig.mli @@ -90,12 +90,14 @@ module Models : sig (** 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) + (** 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? *) + (** 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? *) end diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index 0237c3d15..8e58e686c 100644 --- a/infer/src/concurrency/starvation.ml +++ b/infer/src/concurrency/starvation.ml @@ -84,6 +84,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct || 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 let caller = Procdesc.get_proc_name pdesc in Domain.blocking_call ~caller ~callee loc astate diff --git a/infer/tests/codetoanalyze/java/starvation/FutureGet.java b/infer/tests/codetoanalyze/java/starvation/FutureGet.java new file mode 100644 index 000000000..f9f9ba1b2 --- /dev/null +++ b/infer/tests/codetoanalyze/java/starvation/FutureGet.java @@ -0,0 +1,37 @@ +/* + * 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 java.util.concurrent.Future; +import java.util.concurrent.ExecutionException; +import android.support.annotation.UiThread; + +class FutureGet { + Future future; + Object lock; + + @UiThread + void getDirectBad() throws InterruptedException, ExecutionException { + future.get(); + } + + @UiThread + void getIndirectBad() { + synchronized(lock) {} + } + + void getUnderLock() throws InterruptedException, ExecutionException { + synchronized(lock) { + future.get(); + } + } + + void getOnOtherThreadOk() throws InterruptedException, ExecutionException { + future.get(); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index d63a42b87..69027fa38 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/infer/tests/codetoanalyze/java/starvation/issues.exp @@ -3,6 +3,8 @@ codetoanalyze/java/starvation/Binders.java, void Binders.interBad(), 0, STARVATI 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/FutureGet.java, void FutureGet.getDirectBad(), 0, STARVATION, ERROR, [ void FutureGet.getDirectBad(),calls Object Future.get() from void FutureGet.getDirectBad()] +codetoanalyze/java/starvation/FutureGet.java, void FutureGet.getIndirectBad(), 0, STARVATION, ERROR, [[Trace 1] void FutureGet.getIndirectBad(),locks `this.FutureGet.lock` in class `FutureGet*`,[Trace 2] void FutureGet.getUnderLock(),locks `this.FutureGet.lock` in class `FutureGet*`,calls Object Future.get() from void FutureGet.getUnderLock()] 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.(InnerClass,Object), 0, STARVATION, ERROR, [[Trace 1] InnerClass$InnerClassA.(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*`]