Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ce7a0597 authored by Jay Thomas Sullivan's avatar Jay Thomas Sullivan
Browse files

Fix intermittent async noteOps 

This removes the 'sThreadsListeningForOpNotedInBinderTransaction'
variable and its usages.

I think the intention of this variable was to act as a bloom filter:
i.e., "threadId is possibly in set or definitely not in set", but...
it's actually only giving us "threadId is possibly in set or possibly
not in set". This leads to false negatives, which leads to incorrect
behavior.

In this case, the incorrect behavior was that noteOps would sporatically
turn into async noteOps.  This is currently causing a few tests to be
flaky: when setting a setOnOpNotedCallback listener, sometimes noteOps
code come back sync, sometimes async, and sometimes they get lost in transit.

Bug: 187722787
Test: atest --iterations=100 AppOpsLoggingTest#getBTScanResults
Change-Id: I26c7b8029ea140d850aba7ec77c691ed9fea76d9
parent 63a14538

core/java/android/app/AppOpsManager.java

+1 −15
Original line number Diff line number Diff line
@@ -2782,16 +2782,6 @@ public class AppOpsManager { 
     */

    private static final ThreadLocal<Integer> sBinderThreadCallingUid = new ThreadLocal<>();



    /**

     * Optimization: we need to propagate to IPCs whether the current thread is collecting

     * app ops but using only the thread local above is too slow as it requires a map lookup

     * on every IPC. We add this static var that is lockless and stores an OR-ed mask of the

     * thread id's currently collecting ops, thus reducing the map lookup to a simple bit

     * operation except the extremely unlikely case when threads with overlapping id bits

     * execute op collecting ops.

     */

    private static volatile long sThreadsListeningForOpNotedInBinderTransaction = 0L;



    /**

     * If a thread is currently executing a two-way binder transaction, this stores the

     * ops that were noted blaming any app (the caller, the caller of the caller, etc).
@@ -8882,7 +8872,6 @@ public class AppOpsManager { 
     * @hide

     */

    public static void startNotedAppOpsCollection(int callingUid) {

        sThreadsListeningForOpNotedInBinderTransaction |= Thread.currentThread().getId();

        sBinderThreadCallingUid.set(callingUid);

    }
@@ -8897,7 +8886,6 @@ public class AppOpsManager { 
     */

    public static void finishNotedAppOpsCollection() {

        sBinderThreadCallingUid.remove();

        sThreadsListeningForOpNotedInBinderTransaction &= ~Thread.currentThread().getId();

        sAppOpsNotedInThisBinderTransaction.remove();

    }
@@ -9242,9 +9230,7 @@ public class AppOpsManager { 
     * @return whether we are in a binder transaction and collecting appops.

     */

    private static boolean isListeningForOpNotedInBinderTransaction() {

        return (sThreadsListeningForOpNotedInBinderTransaction

                        & Thread.currentThread().getId()) != 0

                && sBinderThreadCallingUid.get() != null;

        return sBinderThreadCallingUid.get() != null;

    }



    /**