diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 77eefb4e27433..379b494a86fa4 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); @@ -452,13 +455,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/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index 28bd9c367fff7..5848011a508dd 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -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); diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index a36a9f64bded8..b265c5758f7d7 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -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( 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..2a54f463f44ea 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); @@ -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 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); @@ -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;