diff --git a/services/core/java/com/android/server/am/BroadcastConstants.java b/services/core/java/com/android/server/am/BroadcastConstants.java index 3efb628a8b759..f9b0dd0d6f28d 100644 --- a/services/core/java/com/android/server/am/BroadcastConstants.java +++ b/services/core/java/com/android/server/am/BroadcastConstants.java @@ -136,6 +136,14 @@ public class BroadcastConstants { public int MAX_RUNNING_ACTIVE_BROADCASTS = DEFAULT_MAX_RUNNING_ACTIVE_BROADCASTS; private static final int DEFAULT_MAX_RUNNING_ACTIVE_BROADCASTS = 16; + /** + * For {@link BroadcastQueueModernImpl}: Maximum number of pending + * broadcasts to hold for a process before we ignore any delays that policy + * might have applied to that process. + */ + public int MAX_PENDING_BROADCASTS = DEFAULT_MAX_PENDING_BROADCASTS; + private static final int DEFAULT_MAX_PENDING_BROADCASTS = 256; + /** * For {@link BroadcastQueueModernImpl}: Default delay to apply to normal * broadcasts, giving a chance for debouncing of rapidly changing events. @@ -217,6 +225,8 @@ public class BroadcastConstants { DEFAULT_MAX_RUNNING_PROCESS_QUEUES); MAX_RUNNING_ACTIVE_BROADCASTS = properties.getInt("bcast_max_running_active_broadcasts", DEFAULT_MAX_RUNNING_ACTIVE_BROADCASTS); + MAX_PENDING_BROADCASTS = properties.getInt("bcast_max_pending_broadcasts", + DEFAULT_MAX_PENDING_BROADCASTS); DELAY_NORMAL_MILLIS = properties.getLong("bcast_delay_normal_millis", DEFAULT_DELAY_NORMAL_MILLIS); DELAY_CACHED_MILLIS = properties.getLong("bcast_delay_cached_millis", diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 77eefb4e27433..342d1f2f3131e 100644 --- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java +++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java @@ -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); @@ -327,7 +330,7 @@ class BroadcastProcessQueue { } public boolean isEmpty() { - return (mActive != null) && mPending.isEmpty(); + return mPending.isEmpty(); } public boolean isActive() { @@ -385,6 +388,12 @@ class BroadcastProcessQueue { } else { mRunnableAt = runnableAt + constants.DELAY_NORMAL_MILLIS; } + + // If we have too many broadcasts pending, bypass any delays that + // might have been applied above to aid draining + if (mPending.size() >= constants.MAX_PENDING_BROADCASTS) { + mRunnableAt = runnableAt; + } } else { mRunnableAt = Long.MAX_VALUE; } @@ -452,13 +461,19 @@ class BroadcastProcessQueue { @Override public String toString() { - return "BroadcastProcessQueue{" - + Integer.toHexString(System.identityHashCode(this)) - + " " + processName + "/" + UserHandle.formatUid(uid) + "}"; + 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) { diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 972a1cea682fb..b46a2b2575feb 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -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 diff --git a/services/core/java/com/android/server/am/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index 28bd9c367fff7..16711853267ec 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -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) { @@ -732,9 +732,10 @@ public class BroadcastQueueImpl extends BroadcastQueue { } catch (RemoteException ex) { // Failed to call into the process. It's either dying or wedged. Kill it gently. synchronized (mService) { - Slog.w(TAG, "Can't deliver broadcast to " + app.processName - + " (pid " + app.getPid() + "). Crashing it."); - app.scheduleCrashLocked("can't deliver broadcast", + final String msg = "Failed to schedule " + intent + " to " + receiver + + " via " + app + ": " + ex; + Slog.w(TAG, msg); + app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, /* extras=*/ null); } throw ex; @@ -814,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) { @@ -1379,8 +1384,11 @@ public class BroadcastQueueImpl extends BroadcastQueue { processCurBroadcastLocked(r, app); return; } catch (RemoteException e) { - Slog.w(TAG, "Exception when sending broadcast to " - + r.curComponent, e); + final String msg = "Failed to schedule " + r.intent + " to " + info + + " via " + app + ": " + e; + Slog.w(TAG, msg); + app.scheduleCrashLocked(msg, + CannotDeliverBroadcastException.TYPE_ID, /* extras=*/ null); } catch (RuntimeException e) { Slog.wtf(TAG, "Failed sending broadcast to " + r.curComponent + " with " + r.intent, e); diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index a36a9f64bded8..7c236ffe7ba18 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -290,7 +290,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } // If app isn't running, and there's nothing in the queue, clean up - if (queue.isEmpty() && !queue.isProcessWarm()) { + if (queue.isEmpty() && !queue.isActive() && !queue.isProcessWarm()) { removeProcessQueue(queue.processName, queue.uid); } } @@ -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,7 +587,14 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return; } - if (!r.timeoutExempt) { + // 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( Message.obtain(mLocalHandler, MSG_DELIVERY_TIMEOUT, queue), timeout); @@ -604,7 +620,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 +644,12 @@ class BroadcastQueueModernImpl extends BroadcastQueue { app.mState.getReportedProcState()); } } catch (RemoteException e) { + final String msg = "Failed to schedule " + r + " to " + receiver + + " via " + app + ": " + e; + Slog.w(TAG, msg); + app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, null); + app.setKilled(true); finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE); - app.scheduleCrashLocked(TAG, CannotDeliverBroadcastException.TYPE_ID, null); } } else { finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE); @@ -652,7 +672,9 @@ class BroadcastQueueModernImpl extends BroadcastQueue { r.resultCode, r.resultData, r.resultExtras, false, r.initialSticky, r.userId, app.mState.getReportedProcState()); } catch (RemoteException e) { - app.scheduleCrashLocked(TAG, CannotDeliverBroadcastException.TYPE_ID, null); + final String msg = "Failed to schedule result of " + r + " via " + app + ": " + e; + Slog.w(TAG, msg); + app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, null); } } } @@ -674,7 +696,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,9 +713,10 @@ 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) { + r.anrCount++; if (app != null && !app.isDebugging()) { mService.appNotResponding(queue.app, TimeoutRecord .forBroadcastReceiver("Broadcast of " + r.toShortString())); @@ -704,7 +728,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // Even if we have more broadcasts, if we've made reasonable progress // and someone else is waiting, retire ourselves to avoid starvation final boolean shouldRetire = (mRunnableHead != null) - && (queue.getActiveCountSinceIdle() > mConstants.MAX_RUNNING_ACTIVE_BROADCASTS); + && (queue.getActiveCountSinceIdle() >= mConstants.MAX_RUNNING_ACTIVE_BROADCASTS); if (queue.isRunnable() && queue.isProcessWarm() && !shouldRetire) { // We're on a roll; move onto the next broadcast for this process @@ -733,17 +757,22 @@ 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)); } - r.setDeliveryState(index, newDeliveryState); + // Only apply state when we haven't already reached a terminal state; + // this is how we ignore racing timeout messages + if (!isDeliveryStateTerminal(oldDeliveryState)) { + r.setDeliveryState(index, newDeliveryState); + } // Emit any relevant tracing results when we're changing the delivery // state as part of running from a queue @@ -837,7 +866,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( diff --git a/services/core/java/com/android/server/am/BroadcastRecord.java b/services/core/java/com/android/server/am/BroadcastRecord.java index ae7f2a5d8565a..33f74f3bd7794 100644 --- a/services/core/java/com/android/server/am/BroadcastRecord.java +++ b/services/core/java/com/android/server/am/BroadcastRecord.java @@ -128,6 +128,9 @@ final class BroadcastRecord extends Binder { @Nullable final BiFunction 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() { - String label = intent.getAction(); - if (label == null) { - label = intent.toString(); + if (cachedToString == null) { + String label = intent.getAction(); + if (label == null) { + label = intent.toString(); + } + cachedToString = "BroadcastRecord{" + + Integer.toHexString(System.identityHashCode(this)) + + " u" + userId + " " + label + "}"; } - return "BroadcastRecord{" - + Integer.toHexString(System.identityHashCode(this)) - + " u" + userId + " " + label + "}"; + return cachedToString; } public String toShortString() { - String label = intent.getAction(); - if (label == null) { - label = intent.toString(); + if (cachedToShortString == null) { + String label = intent.getAction(); + if (label == null) { + label = intent.toString(); + } + cachedToShortString = label + "/u" + userId; } - return label + "/u" + userId; + return cachedToShortString; } public void dumpDebug(ProtoOutputStream proto, long fieldId) { diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java index cf5d1133b741e..89accf877f998 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -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); @@ -197,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( @@ -278,29 +300,51 @@ 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 extrasOperator) throws Exception { + ProcessBehavior behavior, UnaryOperator 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); - doReturn(r).when(mAms).getProcessRecordLocked(eq(r.info.processName), eq(r.info.uid)); final IIntentReceiver receiver = mock(IIntentReceiver.class); final IBinder receiverBinder = new Binder(); @@ -309,18 +353,29 @@ 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; + 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 +388,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 +504,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 +751,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 +762,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. @@ -741,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. */ @@ -770,13 +972,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 +1060,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;