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

Commit 8ff393d2 authored by Sudheer Shanka's avatar Sudheer Shanka
Browse files

Replace only matching records when enqueuing a broadcast. 

Unless we are inserting the new receivers (that are replacing the
existing ones in the queue) based on their new timestamps, it is
possible for priority inversion to occur. So, when enqueuing a new
broadcast, only replace the matching records to avoid potential
priority inversion issues.

Bug: 311288757
Test: atest services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
Test: atest services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
Change-Id: I9b918e5e42862064b0d5fc623f09308c3c3f2f7d
parent 546a73c3

services/core/java/com/android/server/am/BroadcastProcessQueue.java

+2 −1
Original line number Diff line number Diff line
@@ -361,7 +361,8 @@ class BroadcastProcessQueue { 
                    && (record.userId == testRecord.userId)

                    && record.intent.filterEquals(testRecord.intent)

                    && isReceiverEquals(receiver, testReceiver)

                    && testRecord.allReceiversPending()) {

                    && testRecord.allReceiversPending()

                    && record.isMatchingRecord(testRecord)) {

                // Exact match found; perform in-place swap

                args.arg1 = record;

                args.argi1 = recordIndex;

services/core/java/com/android/server/am/BroadcastQueueModernImpl.java

+17 −0
Original line number Diff line number Diff line
@@ -223,6 +223,14 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
    private final AtomicReference<ArrayMap<BroadcastRecord, Boolean>> mRecordsLookupCache =

            new AtomicReference<>();



    /**

     * Container for holding the set of broadcast records that matches an enqueueing record.

     * @see BroadcastRecord#isMatchingRecord(BroadcastRecord)

     */

    @GuardedBy("mService")

    private final AtomicReference<ArrayMap<BroadcastRecord, Boolean>> mMatchingRecordsCache =

            new AtomicReference<>();



    /**

     * Map from UID to its last known "foreground" state. A UID is considered to be in

     * "foreground" state when it's procState is {@link ActivityManager#PROCESS_STATE_TOP}.
@@ -735,6 +743,12 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
        if (replacedBroadcasts == null) {

            replacedBroadcasts = new ArraySet<>();

        }

        ArrayMap<BroadcastRecord, Boolean> matchingBroadcasts =

                mMatchingRecordsCache.getAndSet(null);

        if (matchingBroadcasts == null) {

            matchingBroadcasts = new ArrayMap<>();

        }

        r.setMatchingRecordsCache(matchingBroadcasts);

        boolean enqueuedBroadcast = false;



        for (int i = 0; i < r.receivers.size(); i++) {
@@ -768,6 +782,9 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
        skipAndCancelReplacedBroadcasts(replacedBroadcasts);

        replacedBroadcasts.clear();

        mReplacedBroadcastsCache.compareAndSet(null, replacedBroadcasts);

        matchingBroadcasts.clear();

        r.clearMatchingRecordsCache();

        mMatchingRecordsCache.compareAndSet(null, matchingBroadcasts);



        // If nothing to dispatch, send any pending result immediately

        if (r.receivers.isEmpty() || !enqueuedBroadcast) {

services/core/java/com/android/server/am/BroadcastRecord.java

+33 −0
Original line number Diff line number Diff line
@@ -61,6 +61,7 @@ import android.os.Binder; 
import android.os.Bundle;

import android.os.SystemClock;

import android.os.UserHandle;

import android.util.ArrayMap;

import android.util.PrintWriterPrinter;

import android.util.SparseArray;

import android.util.TimeUtils;
@@ -164,6 +165,11 @@ final class BroadcastRecord extends Binder { 
    @Nullable

    final BiFunction<Integer, Bundle, Bundle> filterExtrasForReceiver;



    // Cache of records that are "matching" this. Only used at the time of enqueuing this record

    // into the queue.

    @Nullable

    private ArrayMap<BroadcastRecord, Boolean> mMatchingRecordsCache;



    private @Nullable String mCachedToString;

    private @Nullable String mCachedToShortString;
@@ -1250,6 +1256,33 @@ final class BroadcastRecord extends Binder { 
        return (terminalCount == 0 && dispatchTime <= 0);

    }



    boolean isMatchingRecord(@NonNull BroadcastRecord record) {

        final int idx = mMatchingRecordsCache.indexOfKey(record);

        if (idx > 0) {

            return mMatchingRecordsCache.valueAt(idx);

        }

        // Consider a record to be matching if has the same receivers in the same order.

        boolean matches = (receivers.size() == record.receivers.size());

        if (matches) {

            for (int i = receivers.size() - 1; i >= 0; --i) {

                if (!isReceiverEquals(receivers.get(i), record.receivers.get(i))) {

                    matches = false;

                    break;

                }

            }

        }

        mMatchingRecordsCache.put(record, matches);

        return matches;

    }



    void setMatchingRecordsCache(@NonNull ArrayMap<BroadcastRecord, Boolean> matchingRecordsCache) {

        mMatchingRecordsCache = matchingRecordsCache;

    }



    void clearMatchingRecordsCache() {

        mMatchingRecordsCache = null;

    }



    @Override

    public String toString() {

        if (mCachedToString == null) {

services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java

+49 −4
Original line number Diff line number Diff line
@@ -1665,7 +1665,8 @@ public class BroadcastQueueTest extends BaseBroadcastQueueTest { 
        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,

                List.of(makeManifestReceiver(PACKAGE_BLUE, CLASS_RED))));

        enqueueBroadcast(makeOrderedBroadcastRecord(timezoneSecond, callerApp,

                List.of(makeManifestReceiver(PACKAGE_BLUE, CLASS_GREEN)),

                List.of(makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE),

                        makeManifestReceiver(PACKAGE_BLUE, CLASS_GREEN)),

                resultToSecond, null));



        waitForIdle();
@@ -1681,6 +1682,11 @@ public class BroadcastQueueTest extends BaseBroadcastQueueTest { 
                anyInt(), anyInt(), any());



        // We deliver second broadcast to app

        timezoneSecond.setClassName(PACKAGE_BLUE, CLASS_BLUE);

        inOrder.verify(blueThread).scheduleReceiver(

                argThat(filterAndExtrasEquals(timezoneSecond)), any(), any(),

                anyInt(), any(), any(), eq(true), eq(false), anyInt(),

                anyInt(), anyInt(), any());

        timezoneSecond.setClassName(PACKAGE_BLUE, CLASS_GREEN);

        inOrder.verify(blueThread).scheduleReceiver(

                argThat(filterAndExtrasEquals(timezoneSecond)), any(), any(),
@@ -1797,10 +1803,16 @@ public class BroadcastQueueTest extends BaseBroadcastQueueTest { 


        waitForIdle();



        if (mImpl == Impl.MODERN) {

            verifyScheduleRegisteredReceiver(times(2), receiverGreenApp, airplane);

            verifyScheduleRegisteredReceiver(times(2), receiverBlueApp, airplane);

            verifyScheduleRegisteredReceiver(times(1), receiverYellowApp, airplane);

        } else {

            verifyScheduleRegisteredReceiver(times(1), receiverGreenApp, airplane);

            verifyScheduleRegisteredReceiver(times(1), receiverBlueApp, airplane);

            verifyScheduleRegisteredReceiver(never(), receiverYellowApp, airplane);

        }

    }



    @Test

    public void testReplacePending_sameProcess_diffReceivers() throws Exception {
@@ -1829,6 +1841,39 @@ public class BroadcastQueueTest extends BaseBroadcastQueueTest { 
        }

    }



    @Test

    public void testReplacePending_existingDiffReceivers() throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);

        final ProcessRecord receiverGreenApp = makeActiveProcessRecord(PACKAGE_GREEN);

        final ProcessRecord receiverBlueApp = makeActiveProcessRecord(PACKAGE_BLUE);

        final BroadcastFilter receiverGreen = makeRegisteredReceiver(receiverGreenApp);

        final BroadcastFilter receiverBlue = makeRegisteredReceiver(receiverBlueApp);



        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED)

                .addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING);

        final Intent timeTick = new Intent(Intent.ACTION_TIME_TICK);



        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, List.of(

                withPriority(receiverGreen, 5))));

        enqueueBroadcast(makeBroadcastRecord(timeTick, callerApp, List.of(

                withPriority(receiverGreen, 10),

                withPriority(receiverBlue, 5))));

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, List.of(

                withPriority(receiverBlue, 10),

                withPriority(receiverGreen, 5))));



        waitForIdle();



        verifyScheduleRegisteredReceiver(times(1), receiverGreenApp, timeTick);

        verifyScheduleRegisteredReceiver(times(1), receiverBlueApp, timeTick);

        if (mImpl == Impl.MODERN) {

            verifyScheduleRegisteredReceiver(times(2), receiverGreenApp, airplane);

        } else {

            verifyScheduleRegisteredReceiver(times(1), receiverGreenApp, airplane);

        }

        verifyScheduleRegisteredReceiver(times(1), receiverBlueApp, airplane);

    }



    @Test

    public void testIdleAndBarrier() throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);