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

Commit a4c77ee3 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

BroadcastQueue: ignore dead registered receivers.

When a process dies (or is restarted), any registered receivers that
came from the old PID are no longer valid, and should be skipped.

Tests to verify, along with slight update to "default" implementation
to skip sending to a known-dead process.

Bug: 248605002
Test: atest FrameworksMockingServicesTests:BroadcastQueueTest
Change-Id: I9c82595fe2c729cb50213377e1808ffd4bebaa75
parent e40ab122
Loading
Loading
Loading
Loading
+6 −3
Original line number Diff line number Diff line
@@ -114,6 +114,9 @@ public abstract class BroadcastQueue {
    /**
     * Signal from OS internals that the given process has just been actively
     * attached, and is ready to begin receiving broadcasts.
     *
     * @return if the queue performed an action on the given process, such as
     *         dispatching a pending broadcast
     */
    @GuardedBy("mService")
    public abstract boolean onApplicationAttachedLocked(@NonNull ProcessRecord app);
@@ -123,7 +126,7 @@ public abstract class BroadcastQueue {
     * an attempted start and attachment.
     */
    @GuardedBy("mService")
    public abstract boolean onApplicationTimeoutLocked(@NonNull ProcessRecord app);
    public abstract void onApplicationTimeoutLocked(@NonNull ProcessRecord app);

    /**
     * Signal from OS internals that the given process, which had already been
@@ -131,14 +134,14 @@ public abstract class BroadcastQueue {
     * not responding.
     */
    @GuardedBy("mService")
    public abstract boolean onApplicationProblemLocked(@NonNull ProcessRecord app);
    public abstract void onApplicationProblemLocked(@NonNull ProcessRecord app);

    /**
     * Signal from OS internals that the given process has been killed, and is
     * no longer actively running.
     */
    @GuardedBy("mService")
    public abstract boolean onApplicationCleanupLocked(@NonNull ProcessRecord app);
    public abstract void onApplicationCleanupLocked(@NonNull ProcessRecord app);

    /**
     * Signal from OS internals that the given package (or some subset of that
+11 −7
Original line number Diff line number Diff line
@@ -426,16 +426,16 @@ public class BroadcastQueueImpl extends BroadcastQueue {
        }
    }

    public boolean onApplicationTimeoutLocked(ProcessRecord app) {
        return skipCurrentOrPendingReceiverLocked(app);
    public void onApplicationTimeoutLocked(ProcessRecord app) {
        skipCurrentOrPendingReceiverLocked(app);
    }

    public boolean onApplicationProblemLocked(ProcessRecord app) {
        return skipCurrentOrPendingReceiverLocked(app);
    public void onApplicationProblemLocked(ProcessRecord app) {
        skipCurrentOrPendingReceiverLocked(app);
    }

    public boolean onApplicationCleanupLocked(ProcessRecord app) {
        return skipCurrentOrPendingReceiverLocked(app);
    public void onApplicationCleanupLocked(ProcessRecord app) {
        skipCurrentOrPendingReceiverLocked(app);
    }

    public boolean sendPendingBroadcastsLocked(ProcessRecord app) {
@@ -815,7 +815,11 @@ public class BroadcastQueueImpl extends BroadcastQueue {
        try {
            if (DEBUG_BROADCAST_LIGHT) Slog.i(TAG_BROADCAST,
                    "Delivering to " + filter + " : " + r);
            if (filter.receiverList.app != null && filter.receiverList.app.isInFullBackup()) {
            final boolean isInFullBackup = (filter.receiverList.app != null)
                    && filter.receiverList.app.isInFullBackup();
            final boolean isKilled = (filter.receiverList.app != null)
                    && filter.receiverList.app.isKilled();
            if (isInFullBackup || isKilled) {
                // Skip delivery if full backup in progress
                // If it's an ordered broadcast, we need to continue to the next receiver.
                if (ordered) {
+27 −11
Original line number Diff line number Diff line
@@ -420,18 +420,17 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
    }

    @Override
    public boolean onApplicationTimeoutLocked(@NonNull ProcessRecord app) {
        return onApplicationCleanupLocked(app);
    public void onApplicationTimeoutLocked(@NonNull ProcessRecord app) {
        onApplicationCleanupLocked(app);
    }

    @Override
    public boolean onApplicationProblemLocked(@NonNull ProcessRecord app) {
        return onApplicationCleanupLocked(app);
    public void onApplicationProblemLocked(@NonNull ProcessRecord app) {
        onApplicationCleanupLocked(app);
    }

    @Override
    public boolean onApplicationCleanupLocked(@NonNull ProcessRecord app) {
        boolean didSomething = false;
    public void onApplicationCleanupLocked(@NonNull ProcessRecord app) {
        if ((mRunningColdStart != null) && (mRunningColdStart.app == app)) {
            // We've been waiting for this app to cold start, and it had
            // trouble; clear the slot and fail delivery below
@@ -439,7 +438,6 @@ class BroadcastQueueModernImpl extends BroadcastQueue {

            // We might be willing to kick off another cold start
            enqueueUpdateRunningList();
            didSomething = true;
        }

        final BroadcastProcessQueue queue = getProcessQueue(app);
@@ -449,16 +447,19 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
            // If queue was running a broadcast, fail it
            if (queue.isActive()) {
                finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);
                didSomething = true;
            }

            // Skip any pending registered receivers, since the old process
            // would never be around to receive them
            queue.removeMatchingBroadcasts((r, i) -> {
                return (r.receivers.get(i) instanceof BroadcastFilter);
            }, mBroadcastConsumerSkip);

            // If queue has nothing else pending, consider cleaning it
            if (queue.isEmpty()) {
                updateRunnableList(queue);
            }
        }

        return didSomething;
    }

    @Override
@@ -515,6 +516,13 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
        final int index = queue.getActiveIndex();
        final Object receiver = r.receivers.get(index);

        // Ignore registered receivers from a previous PID
        if (receiver instanceof BroadcastFilter) {
            mRunningColdStart = null;
            finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED);
            return;
        }

        final ApplicationInfo info = ((ResolveInfo) receiver).activityInfo.applicationInfo;
        final ComponentName component = ((ResolveInfo) receiver).activityInfo.getComponentName();

@@ -536,6 +544,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
        } else {
            mRunningColdStart = null;
            finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE);
            return;
        }
    }

@@ -563,7 +572,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
            return;
        }

        // Consider additional cases where we'd want fo finish immediately
        // Consider additional cases where we'd want to finish immediately
        if (app.isInFullBackup()) {
            finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED);
            return;
@@ -578,6 +587,13 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
            return;
        }

        // Ignore registered receivers from a previous PID
        if ((receiver instanceof BroadcastFilter)
                && ((BroadcastFilter) receiver).receiverList.pid != app.getPid()) {
            finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED);
            return;
        }

        if (mService.mProcessesReady && !r.timeoutExempt) {
            final long timeout = r.isForeground() ? mFgConstants.TIMEOUT : mBgConstants.TIMEOUT;
            mLocalHandler.sendMessageDelayed(
+57 −1
Original line number Diff line number Diff line
@@ -209,6 +209,16 @@ public class BroadcastQueueTest {
            return res;
        }).when(mAms).startProcessLocked(any(), any(), anyBoolean(), anyInt(),
                any(), anyInt(), anyBoolean(), anyBoolean());
        doAnswer((invocation) -> {
            final String processName = invocation.getArgument(0);
            final int uid = invocation.getArgument(1);
            for (ProcessRecord r : mActiveProcesses) {
                if (Objects.equals(r.processName, processName) && r.uid == uid) {
                    return r;
                }
            }
            return null;
        }).when(mAms).getProcessRecordLocked(any(), anyInt());
        doNothing().when(mAms).appNotResponding(any(), any());

        final BroadcastConstants constants = new BroadcastConstants(
@@ -335,7 +345,6 @@ public class BroadcastQueueTest {
        final IBinder threadBinder = new Binder();
        doReturn(threadBinder).when(thread).asBinder();
        r.makeActive(thread, mAms.mProcessStats);
        doReturn(r).when(mAms).getProcessRecordLocked(eq(r.info.processName), eq(r.info.uid));

        final IIntentReceiver receiver = mock(IIntentReceiver.class);
        final IBinder receiverBinder = new Binder();
@@ -344,6 +353,14 @@ public class BroadcastQueueTest {
                UserHandle.getUserId(r.info.uid), receiver);
        mRegisteredReceivers.put(r.getPid(), receiverList);

        doAnswer((invocation) -> {
            Log.v(TAG, "Intercepting killLocked() for "
                    + Arrays.toString(invocation.getArguments()));
            mActiveProcesses.remove(r);
            mRegisteredReceivers.remove(r.getPid());
            return invocation.callRealMethod();
        }).when(r).killLocked(any(), any(), anyInt(), anyInt(), anyBoolean());

        // If we're entirely dead, rely on default behaviors above
        if (dead) return r;

@@ -887,6 +904,45 @@ public class BroadcastQueueTest {
                new ComponentName(PACKAGE_GREEN, CLASS_BLUE));
    }

    /**
     * Verify that killing a running process skips registered receivers.
     */
    @Test
    public void testKill() throws Exception {
        final ProcessRecord callerApp = makeActiveProcessRecord(PACKAGE_RED);
        final ProcessRecord oldApp = makeActiveProcessRecord(PACKAGE_GREEN);

        final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
        try (SyncBarrier b = new SyncBarrier()) {
            enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, new ArrayList<>(
                    List.of(makeRegisteredReceiver(oldApp),
                            makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN)))));

            synchronized (mAms) {
                oldApp.killLocked(TAG, 42, false);
                mQueue.onApplicationCleanupLocked(oldApp);
            }
        }
        waitForIdle();

        // Confirm that we cold-started after the kill
        final ProcessRecord newApp = mAms.getProcessRecordLocked(PACKAGE_GREEN,
                getUidForPackage(PACKAGE_GREEN));
        assertNotEquals(oldApp, newApp);

        // Confirm that we saw no registered receiver traffic
        final IApplicationThread oldThread = oldApp.getThread();
        verify(oldThread, never()).scheduleRegisteredReceiver(any(),
                any(), anyInt(), any(), any(), anyBoolean(), anyBoolean(), anyInt(), anyInt());
        final IApplicationThread newThread = newApp.getThread();
        verify(newThread, never()).scheduleRegisteredReceiver(any(),
                any(), anyInt(), any(), any(), anyBoolean(), anyBoolean(), anyInt(), anyInt());

        // Confirm that we saw final manifest broadcast
        verifyScheduleReceiver(times(1), newApp, airplane,
                new ComponentName(PACKAGE_GREEN, CLASS_GREEN));
    }

    /**
     * Verify that we skip broadcasts to an app being backed up.
     */