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

Commit 5f31d431 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

BroadcastQueue: tests for failing starts, dead. 

Add tests that verify the behavior of failing cold process starts
and handling of DeadObjectException.  This generally means we
skip the active broadcast and keep making progress on our queue,
instead of risking stalling out.

This testing uncovered a subtle bug in the "default" implementation,
where we'd clean up a dead registered receiver, but leave a dead
manifest receiver floating.  We fix this by giving both cases the
same treatment.

Add string caching to improve performance.

Bug: 245771249
Test: atest FrameworksMockingServicesTests:BroadcastQueueTest
Change-Id: Ic7ece84d6b0404d49b2fdf586f4574efdd4175fa
parent c6dde4f8

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

+13 −4
Original line number Diff line number Diff line
@@ -119,6 +119,9 @@ class BroadcastProcessQueue { 


    private boolean mProcessCached;



    private String mCachedToString;

    private String mCachedToShortString;



    public BroadcastProcessQueue(@NonNull BroadcastConstants constants,

            @NonNull String processName, int uid) {

        this.constants = Objects.requireNonNull(constants);
@@ -452,13 +455,19 @@ class BroadcastProcessQueue { 


    @Override

    public String toString() {

        return "BroadcastProcessQueue{"

        if (mCachedToString == null) {

            mCachedToString = "BroadcastProcessQueue{"

                    + Integer.toHexString(System.identityHashCode(this))

                    + " " + processName + "/" + UserHandle.formatUid(uid) + "}";

        }

        return mCachedToString;

    }



    public String toShortString() {

        return processName + "/" + UserHandle.formatUid(uid);

        if (mCachedToShortString == null) {

            mCachedToShortString = processName + "/" + UserHandle.formatUid(uid);

        }

        return mCachedToShortString;

    }



    public void dumpLocked(@NonNull IndentingPrintWriter pw) {

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

+2 −0
Original line number Diff line number Diff line
@@ -1381,6 +1381,8 @@ public class BroadcastQueueImpl extends BroadcastQueue { 
            } catch (RemoteException e) {

                Slog.w(TAG, "Exception when sending broadcast to "

                      + r.curComponent, e);

                app.scheduleCrashLocked("can't deliver broadcast",

                        CannotDeliverBroadcastException.TYPE_ID, /* extras=*/ null);

            } catch (RuntimeException e) {

                Slog.wtf(TAG, "Failed sending broadcast to "

                        + r.curComponent + " with " + r.intent, e);

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

+14 −8
Original line number Diff line number Diff line
@@ -604,7 +604,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
        }



        if (DEBUG_BROADCAST) logv("Scheduling " + r + " to warm " + app);

        setDeliveryState(queue, r, index, receiver, BroadcastRecord.DELIVERY_SCHEDULED);

        setDeliveryState(queue, app, r, index, receiver, BroadcastRecord.DELIVERY_SCHEDULED);



        final IApplicationThread thread = app.getThread();

        if (thread != null) {
@@ -628,8 +628,11 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
                            app.mState.getReportedProcState());

                }

            } catch (RemoteException e) {

                finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);

                Slog.w(TAG, "Failed to schedule " + r + " to " + receiver

                        + " via " + app + ": " + e);

                app.scheduleCrashLocked(TAG, CannotDeliverBroadcastException.TYPE_ID, null);

                app.setKilled(true);

                finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);

            }

        } else {

            finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);
@@ -652,6 +655,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
                        r.resultCode, r.resultData, r.resultExtras, false, r.initialSticky,

                        r.userId, app.mState.getReportedProcState());

            } catch (RemoteException e) {

                Slog.w(TAG, "Failed to schedule result of " + r + " via " + app + ": " + e);

                app.scheduleCrashLocked(TAG, CannotDeliverBroadcastException.TYPE_ID, null);

            }

        }
@@ -674,7 +678,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
        // receivers as skipped

        if (r.ordered && r.resultAbort) {

            for (int i = r.finishedCount + 1; i < r.receivers.size(); i++) {

                setDeliveryState(null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED);

                setDeliveryState(null, null, r, i, r.receivers.get(i),

                        BroadcastRecord.DELIVERY_SKIPPED);

            }

        }
@@ -690,7 +695,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
        final int index = queue.getActiveIndex();

        final Object receiver = r.receivers.get(index);



        setDeliveryState(queue, r, index, receiver, deliveryState);

        setDeliveryState(queue, app, r, index, receiver, deliveryState);



        if (deliveryState == BroadcastRecord.DELIVERY_TIMEOUT) {

            if (app != null && !app.isDebugging()) {
@@ -733,12 +738,13 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
     * bookkeeping related to ordered broadcasts.

     */

    private void setDeliveryState(@Nullable BroadcastProcessQueue queue,

            @NonNull BroadcastRecord r, int index, @NonNull Object receiver,

            @DeliveryState int newDeliveryState) {

            @Nullable ProcessRecord app, @NonNull BroadcastRecord r, int index,

            @NonNull Object receiver, @DeliveryState int newDeliveryState) {

        final int oldDeliveryState = getDeliveryState(r, index);



        if (newDeliveryState != BroadcastRecord.DELIVERY_DELIVERED) {

            Slog.w(TAG, "Delivery state of " + r + " to " + receiver + " changed from "

            Slog.w(TAG, "Delivery state of " + r + " to " + receiver

                    + " via " + app + " changed from "

                    + deliveryStateToString(oldDeliveryState) + " to "

                    + deliveryStateToString(newDeliveryState));

        }
@@ -837,7 +843,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { 
     * of it matching a predicate.

     */

    private final BroadcastConsumer mBroadcastConsumerSkip = (r, i) -> {

        setDeliveryState(null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED);

        setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED);

    };



    private boolean skipMatchingBroadcasts(

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

+19 −10
Original line number Diff line number Diff line
@@ -128,6 +128,9 @@ final class BroadcastRecord extends Binder { 
    @Nullable

    final BiFunction<Integer, Bundle, Bundle> filterExtrasForReceiver;



    String cachedToString;

    String cachedToShortString;



    static final int IDLE = 0;

    static final int APP_RECEIVE = 1;

    static final int CALL_IN_RECEIVE = 2;
@@ -700,21 +703,27 @@ final class BroadcastRecord extends Binder { 


    @Override

    public String toString() {

        if (cachedToString == null) {

            String label = intent.getAction();

            if (label == null) {

                label = intent.toString();

            }

        return "BroadcastRecord{"

            cachedToString = "BroadcastRecord{"

                + Integer.toHexString(System.identityHashCode(this))

                + " u" + userId + " " + label + "}";

        }

        return cachedToString;

    }



    public String toShortString() {

        if (cachedToShortString == null) {

            String label = intent.getAction();

            if (label == null) {

                label = intent.toString();

            }

        return label + "/u" + userId;

            cachedToShortString = label + "/u" + userId;

        }

        return cachedToShortString;

    }



    public void dumpDebug(ProtoOutputStream proto, long fieldId) {

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

+161 −15
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.am;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNotEquals;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -42,6 +43,7 @@ import android.app.ActivityManager; 
import android.app.AppOpsManager;

import android.app.BroadcastOptions;

import android.app.IApplicationThread;

import android.app.RemoteServiceException.CannotDeliverBroadcastException;

import android.app.usage.UsageEvents.Event;

import android.app.usage.UsageStatsManagerInternal;

import android.content.ComponentName;
@@ -56,6 +58,7 @@ import android.content.pm.PackageManagerInternal; 
import android.content.pm.ResolveInfo;

import android.os.Binder;

import android.os.Bundle;

import android.os.DeadObjectException;

import android.os.Handler;

import android.os.HandlerThread;

import android.os.IBinder;
@@ -135,6 +138,12 @@ public class BroadcastQueueTest { 
    private ActivityManagerService mAms;

    private BroadcastQueue mQueue;



    /**

     * When enabled {@link ActivityManagerService#startProcessLocked} will fail

     * by returning {@code null}; otherwise it will spawn a new mock process.

     */

    private boolean mFailStartProcess;



    /**

     * Map from PID to registered registered runtime receivers.

     */
@@ -185,10 +194,13 @@ public class BroadcastQueueTest { 
        doAnswer((invocation) -> {

            Log.v(TAG, "Intercepting startProcessLocked() for "

                    + Arrays.toString(invocation.getArguments()));

            if (mFailStartProcess) {

                return null;

            }

            final String processName = invocation.getArgument(0);

            final ApplicationInfo ai = invocation.getArgument(1);

            final ProcessRecord res = makeActiveProcessRecord(ai, processName, false,

                    false, UnaryOperator.identity());

            final ProcessRecord res = makeActiveProcessRecord(ai, processName,

                    ProcessBehavior.NORMAL, UnaryOperator.identity());

            mHandlerThread.getThreadHandler().post(() -> {

                synchronized (mAms) {

                    mQueue.onApplicationAttachedLocked(res);
@@ -278,25 +290,48 @@ public class BroadcastQueueTest { 
        }

    }



    private enum ProcessBehavior {

        /** Process broadcasts normally */

        NORMAL,

        /** Wedge and never confirm broadcast receipt */

        WEDGE,

        /** Process broadcast by requesting abort */

        ABORT,

        /** Appear to behave completely dead */

        DEAD,

    }



    private ProcessRecord makeActiveProcessRecord(String packageName) throws Exception {

        final ApplicationInfo ai = makeApplicationInfo(packageName);

        return makeActiveProcessRecord(ai, ai.processName, false, false,

        return makeActiveProcessRecord(ai, ai.processName, ProcessBehavior.NORMAL,

                UnaryOperator.identity());

    }



    private ProcessRecord makeWedgedActiveProcessRecord(String packageName) throws Exception {

    private ProcessRecord makeActiveProcessRecord(String packageName,

            ProcessBehavior behavior) throws Exception {

        final ApplicationInfo ai = makeApplicationInfo(packageName);

        return makeActiveProcessRecord(ai, ai.processName, true, false,

        return makeActiveProcessRecord(ai, ai.processName, behavior,

                UnaryOperator.identity());

    }



    private ProcessRecord makeActiveProcessRecord(ApplicationInfo ai, String processName,

            boolean wedged, boolean abort, UnaryOperator<Bundle> extrasOperator) throws Exception {

            ProcessBehavior behavior, UnaryOperator<Bundle> extrasOperator) throws Exception {

        final boolean wedge = (behavior == ProcessBehavior.WEDGE);

        final boolean abort = (behavior == ProcessBehavior.ABORT);

        final boolean dead = (behavior == ProcessBehavior.DEAD);



        final ProcessRecord r = spy(new ProcessRecord(mAms, ai, processName, ai.uid));

        r.setPid(mNextPid.getAndIncrement());

        mActiveProcesses.add(r);



        final IApplicationThread thread = mock(IApplicationThread.class);

        final IApplicationThread thread;

        if (dead) {

            thread = mock(IApplicationThread.class, (invocation) -> {

                throw new DeadObjectException();

            });

        } else {

            thread = mock(IApplicationThread.class);

        }

        final IBinder threadBinder = new Binder();

        doReturn(threadBinder).when(thread).asBinder();

        r.makeActive(thread, mAms.mProcessStats);
@@ -309,18 +344,21 @@ public class BroadcastQueueTest { 
                UserHandle.getUserId(r.info.uid), receiver);

        mRegisteredReceivers.put(r.getPid(), receiverList);



        // If we're entirely dead, rely on default behaviors above

        if (dead) return r;



        doAnswer((invocation) -> {

            Log.v(TAG, "Intercepting scheduleReceiver() for "

                    + Arrays.toString(invocation.getArguments()));

            final Bundle extras = invocation.getArgument(5);

            if (!wedged) {

            if (!wedge) {

                assertTrue(r.mReceivers.numberOfCurReceivers() > 0);

                assertTrue(mQueue.getPreferredSchedulingGroupLocked(r)

                        != ProcessList.SCHED_GROUP_UNDEFINED);

                mHandlerThread.getThreadHandler().post(() -> {

                    synchronized (mAms) {

                        mQueue.finishReceiverLocked(r, Activity.RESULT_OK,

                                null, extrasOperator.apply(extras), abort, false);

                        mQueue.finishReceiverLocked(r, Activity.RESULT_OK, null,

                                extrasOperator.apply(extras), abort, false);

                    }

                });

            }
@@ -333,7 +371,7 @@ public class BroadcastQueueTest { 
                    + Arrays.toString(invocation.getArguments()));

            final Bundle extras = invocation.getArgument(4);

            final boolean ordered = invocation.getArgument(5);

            if (!wedged && ordered) {

            if (!wedge && ordered) {

                assertTrue(r.mReceivers.numberOfCurReceivers() > 0);

                assertTrue(mQueue.getPreferredSchedulingGroupLocked(r)

                        != ProcessList.SCHED_GROUP_UNDEFINED);
@@ -449,6 +487,13 @@ public class BroadcastQueueTest { 
                any(), eq(false), eq(UserHandle.USER_SYSTEM), anyInt());

    }



    private void verifyScheduleReceiver(VerificationMode mode, ProcessRecord app, Intent intent)

            throws Exception {

        verify(app.getThread(), mode).scheduleReceiver(

                argThat(filterEqualsIgnoringComponent(intent)), any(), any(), anyInt(), any(),

                any(), eq(false), eq(UserHandle.USER_SYSTEM), anyInt());

    }



    private void verifyScheduleReceiver(VerificationMode mode, ProcessRecord app, Intent intent,

            ComponentName component) throws Exception {

        final Intent targetedIntent = new Intent(intent);
@@ -689,7 +734,8 @@ public class BroadcastQueueTest { 
    @Test

    public void testWedged() throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);

        final ProcessRecord receiverApp = makeWedgedActiveProcessRecord(PACKAGE_GREEN);

        final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN,

                ProcessBehavior.WEDGE);



        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,
@@ -699,6 +745,106 @@ public class BroadcastQueueTest { 
        verify(mAms).appNotResponding(eq(receiverApp), any());

    }



    /**

     * Verify that we handle registered receivers in a process that always

     * responds with {@link DeadObjectException}, recovering to restart the

     * process and deliver their next broadcast.

     */

    @Test

    public void testDead_Registered() throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);

        final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN,

                ProcessBehavior.DEAD);



        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,

                List.of(makeRegisteredReceiver(receiverApp))));

        final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN))));

        waitForIdle();



        // First broadcast should have already been dead

        verifyScheduleRegisteredReceiver(receiverApp, airplane);

        verify(receiverApp).scheduleCrashLocked(any(),

                eq(CannotDeliverBroadcastException.TYPE_ID), any());



        // Second broadcast in new process should work fine

        final ProcessRecord restartedReceiverApp = mAms.getProcessRecordLocked(PACKAGE_GREEN,

                getUidForPackage(PACKAGE_GREEN));

        assertNotEquals(receiverApp, restartedReceiverApp);

        verifyScheduleReceiver(restartedReceiverApp, timezone);

    }



    /**

     * Verify that we handle manifest receivers in a process that always

     * responds with {@link DeadObjectException}, recovering to restart the

     * process and deliver their next broadcast.

     */

    @Test

    public void testDead_Manifest() throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);

        final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN,

                ProcessBehavior.DEAD);



        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN))));

        final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN))));

        waitForIdle();



        // First broadcast should have already been dead

        verifyScheduleReceiver(receiverApp, airplane);

        verify(receiverApp).scheduleCrashLocked(any(),

                eq(CannotDeliverBroadcastException.TYPE_ID), any());



        // Second broadcast in new process should work fine

        final ProcessRecord restartedReceiverApp = mAms.getProcessRecordLocked(PACKAGE_GREEN,

                getUidForPackage(PACKAGE_GREEN));

        assertNotEquals(receiverApp, restartedReceiverApp);

        verifyScheduleReceiver(restartedReceiverApp, timezone);

    }



    /**

     * Verify that we handle the system failing to start a process.

     */

    @Test

    public void testFailStartProcess() throws Exception {

        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);



        // Send broadcast while process starts are failing

        mFailStartProcess = true;

        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(airplane, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN),

                        makeManifestReceiver(PACKAGE_YELLOW, CLASS_YELLOW))));



        // Confirm that queue goes idle, with no processes

        waitForIdle();

        assertEquals(1, mActiveProcesses.size());



        // Send more broadcasts with working process starts

        mFailStartProcess = false;

        final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);

        enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,

                List.of(makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN),

                        makeManifestReceiver(PACKAGE_YELLOW, CLASS_YELLOW))));



        // Confirm that we only saw second broadcast

        waitForIdle();

        assertEquals(3, mActiveProcesses.size());

        final ProcessRecord receiverGreenApp = mAms.getProcessRecordLocked(PACKAGE_GREEN,

                getUidForPackage(PACKAGE_GREEN));

        final ProcessRecord receiverYellowApp = mAms.getProcessRecordLocked(PACKAGE_YELLOW,

                getUidForPackage(PACKAGE_YELLOW));

        verifyScheduleReceiver(never(), receiverGreenApp, airplane);

        verifyScheduleReceiver(never(), receiverYellowApp, airplane);

        verifyScheduleReceiver(times(1), receiverGreenApp, timezone);

        verifyScheduleReceiver(times(1), receiverYellowApp, timezone);

    }



    /**

     * Verify that we cleanup a disabled component, skipping a pending dispatch

     * of broadcast to that component.
@@ -770,13 +916,13 @@ public class BroadcastQueueTest { 
        // Purposefully warm-start the middle apps to make sure we dispatch to

        // both cold and warm apps in expected order

        makeActiveProcessRecord(makeApplicationInfo(PACKAGE_BLUE), PACKAGE_BLUE,

                false, false, (extras) -> {

                ProcessBehavior.NORMAL, (extras) -> {

                    extras = clone(extras);

                    extras.putBoolean(PACKAGE_BLUE, true);

                    return extras;

                });

        makeActiveProcessRecord(makeApplicationInfo(PACKAGE_YELLOW), PACKAGE_YELLOW,

                false, false, (extras) -> {

                ProcessBehavior.NORMAL, (extras) -> {

                    extras = clone(extras);

                    extras.putBoolean(PACKAGE_YELLOW, true);

                    return extras;
@@ -858,7 +1004,7 @@ public class BroadcastQueueTest { 


        // Create a process that aborts any ordered broadcasts

        makeActiveProcessRecord(makeApplicationInfo(PACKAGE_GREEN), PACKAGE_GREEN,

                false, true, (extras) -> {

                ProcessBehavior.ABORT, (extras) -> {

                    extras = clone(extras);

                    extras.putBoolean(PACKAGE_GREEN, true);

                    return extras;