From 3dc5a4b1bbe88d5ec8fe30cff19aadeb401c9d78 Mon Sep 17 00:00:00 2001 From: Li Li Date: Wed, 16 Nov 2022 11:05:58 -0800 Subject: [PATCH] Stop using CannotDeliverBroadcastException This crash behavior was introduced a long time ago to show a crash dialog if the broadcast couldn't be delivered (b/28196243). Android has changed a lot since then. The crash dialog won't be shown to the user as it impacts user experience, especially when the app is in the background. Furthermore, this CannotDeliverBroadcastException crashes the app with the same callstack, without any useful information for app developers to debug the issue. And it pollutes the app crashlytics stats in a negative way so app developers choose to suppress it (b/245258072). This exception is hidden and not documented at developers.android.com. Let's stop sending it to apps. Kill the app gently instead (as what's already mentioned in the comments). Bug: 239419108 Test: atest BroadcastQueueTest Test: observe app is killed gently on failed broadcast (instead of crash) Change-Id: I93aabe99167105111f9f3bd945cf4977cfb8cdb1 --- core/java/android/app/ActivityThread.java | 4 ---- .../android/app/RemoteServiceException.java | 23 ++++--------------- .../android/server/am/BroadcastQueueImpl.java | 15 ++++-------- .../server/am/BroadcastQueueModernImpl.java | 7 +++--- .../android/server/am/BroadcastQueueTest.java | 9 ++++---- 5 files changed, 15 insertions(+), 43 deletions(-) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 1f6334381bc99..e001c7df506ee 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -40,7 +40,6 @@ import static com.android.internal.annotations.VisibleForTesting.Visibility.PACK import android.annotation.NonNull; import android.annotation.Nullable; import android.app.RemoteServiceException.BadForegroundServiceNotificationException; -import android.app.RemoteServiceException.CannotDeliverBroadcastException; import android.app.RemoteServiceException.CannotPostForegroundServiceNotificationException; import android.app.RemoteServiceException.CrashedByAdbException; import android.app.RemoteServiceException.ForegroundServiceDidNotStartInTimeException; @@ -1994,9 +1993,6 @@ public final class ActivityThread extends ClientTransactionHandler case ForegroundServiceDidNotStartInTimeException.TYPE_ID: throw generateForegroundServiceDidNotStartInTimeException(message, extras); - case CannotDeliverBroadcastException.TYPE_ID: - throw new CannotDeliverBroadcastException(message); - case CannotPostForegroundServiceNotificationException.TYPE_ID: throw new CannotPostForegroundServiceNotificationException(message); diff --git a/core/java/android/app/RemoteServiceException.java b/core/java/android/app/RemoteServiceException.java index e220627706f92..620adbedc9032 100644 --- a/core/java/android/app/RemoteServiceException.java +++ b/core/java/android/app/RemoteServiceException.java @@ -70,21 +70,6 @@ public class RemoteServiceException extends AndroidRuntimeException { } } - /** - * Exception used to crash an app process when the system received a RemoteException - * while delivering a broadcast to an app process. - * - * @hide - */ - public static class CannotDeliverBroadcastException extends RemoteServiceException { - /** The type ID passed to {@link IApplicationThread#scheduleCrash}. */ - public static final int TYPE_ID = 2; - - public CannotDeliverBroadcastException(String msg) { - super(msg); - } - } - /** * Exception used to crash an app process when the system received a RemoteException * while posting a notification of a foreground service. @@ -94,7 +79,7 @@ public class RemoteServiceException extends AndroidRuntimeException { public static class CannotPostForegroundServiceNotificationException extends RemoteServiceException { /** The type ID passed to {@link IApplicationThread#scheduleCrash}. */ - public static final int TYPE_ID = 3; + public static final int TYPE_ID = 2; public CannotPostForegroundServiceNotificationException(String msg) { super(msg); @@ -109,7 +94,7 @@ public class RemoteServiceException extends AndroidRuntimeException { */ public static class BadForegroundServiceNotificationException extends RemoteServiceException { /** The type ID passed to {@link IApplicationThread#scheduleCrash}. */ - public static final int TYPE_ID = 4; + public static final int TYPE_ID = 3; public BadForegroundServiceNotificationException(String msg) { super(msg); @@ -125,7 +110,7 @@ public class RemoteServiceException extends AndroidRuntimeException { public static class MissingRequestPasswordComplexityPermissionException extends RemoteServiceException { /** The type ID passed to {@link IApplicationThread#scheduleCrash}. */ - public static final int TYPE_ID = 5; + public static final int TYPE_ID = 4; public MissingRequestPasswordComplexityPermissionException(String msg) { super(msg); @@ -139,7 +124,7 @@ public class RemoteServiceException extends AndroidRuntimeException { */ public static class CrashedByAdbException extends RemoteServiceException { /** The type ID passed to {@link IApplicationThread#scheduleCrash}. */ - public static final int TYPE_ID = 6; + public static final int TYPE_ID = 5; public CrashedByAdbException(String msg) { super(msg); diff --git a/services/core/java/com/android/server/am/BroadcastQueueImpl.java b/services/core/java/com/android/server/am/BroadcastQueueImpl.java index 454b28402ced7..2e2788b9c23b5 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueImpl.java @@ -42,16 +42,15 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.app.Activity; import android.app.ActivityManager; +import android.app.ApplicationExitInfo; import android.app.BroadcastOptions; import android.app.IApplicationThread; -import android.app.RemoteServiceException.CannotDeliverBroadcastException; import android.app.usage.UsageEvents.Event; import android.content.ComponentName; import android.content.ContentResolver; import android.content.IIntentReceiver; import android.content.Intent; import android.content.pm.ActivityInfo; -import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.content.pm.UserInfo; @@ -729,19 +728,14 @@ public class BroadcastQueueImpl extends BroadcastQueue { thread.scheduleRegisteredReceiver(receiver, intent, resultCode, data, extras, ordered, sticky, sendingUser, app.mState.getReportedProcState()); - // TODO: Uncomment this when (b/28322359) is fixed and we aren't getting - // DeadObjectException when the process isn't actually dead. - //} catch (DeadObjectException ex) { - // Failed to call into the process. It's dying so just let it die and move on. - // throw ex; } catch (RemoteException ex) { // Failed to call into the process. It's either dying or wedged. Kill it gently. synchronized (mService) { final String msg = "Failed to schedule " + intent + " to " + receiver + " via " + app + ": " + ex; Slog.w(TAG, msg); - app.scheduleCrashLocked(msg, - CannotDeliverBroadcastException.TYPE_ID, /* extras=*/ null); + app.killLocked("Can't deliver broadcast", ApplicationExitInfo.REASON_OTHER, + true); } throw ex; } @@ -1394,8 +1388,7 @@ public class BroadcastQueueImpl extends BroadcastQueue { 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); + app.killLocked("Can't deliver broadcast", ApplicationExitInfo.REASON_OTHER, true); } 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 008d95a51cd92..8418987cff9c7 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -42,9 +42,9 @@ import android.annotation.Nullable; import android.annotation.UptimeMillisLong; import android.app.Activity; import android.app.ActivityManager; +import android.app.ApplicationExitInfo; import android.app.BroadcastOptions; import android.app.IApplicationThread; -import android.app.RemoteServiceException.CannotDeliverBroadcastException; import android.app.UidObserver; import android.app.usage.UsageEvents.Event; import android.content.ComponentName; @@ -801,8 +801,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { final String msg = "Failed to schedule " + r + " to " + receiver + " via " + app + ": " + e; logw(msg); - app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, null); - app.setKilled(true); + app.killLocked("Can't deliver broadcast", ApplicationExitInfo.REASON_OTHER, true); enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_FAILURE, "remote app"); } } else { @@ -829,7 +828,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } catch (RemoteException e) { final String msg = "Failed to schedule result of " + r + " via " + app + ": " + e; logw(msg); - app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, null); + app.killLocked("Can't deliver broadcast", ApplicationExitInfo.REASON_OTHER, true); } } // Clear so both local and remote references can be GC'ed 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 fd605f7796251..b9615f6b3771c 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -50,7 +50,6 @@ 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; @@ -964,8 +963,8 @@ public class BroadcastQueueTest { // First broadcast should have already been dead verifyScheduleRegisteredReceiver(receiverApp, airplane); - verify(receiverApp).scheduleCrashLocked(any(), - eq(CannotDeliverBroadcastException.TYPE_ID), any()); + // The old receiverApp should be killed gently + assertTrue(receiverApp.isKilled()); // Second broadcast in new process should work fine final ProcessRecord restartedReceiverApp = mAms.getProcessRecordLocked(PACKAGE_GREEN, @@ -995,8 +994,8 @@ public class BroadcastQueueTest { // First broadcast should have already been dead verifyScheduleReceiver(receiverApp, airplane); - verify(receiverApp).scheduleCrashLocked(any(), - eq(CannotDeliverBroadcastException.TYPE_ID), any()); + // The old receiverApp should be killed gently + assertTrue(receiverApp.isKilled()); // Second broadcast in new process should work fine final ProcessRecord restartedReceiverApp = mAms.getProcessRecordLocked(PACKAGE_GREEN,