From 9a3109144e45ca6cd5f2e79d8f01f334a542e81c Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 31 Oct 2022 12:19:55 -0600 Subject: [PATCH 1/5] BroadcastQueue: skip ANRs when assuming success. Both the legacy and modern stack assume delivery success for non-ordered registered receivers. In these situations we don't need to start ANR tracking logic, which can give some performance wins. Bug: 255936508 Test: ./frameworks/base/libs/hwui/tests/scripts/prep_generic.sh little && atest ActivityManagerPerfTests:BroadcastPerfTest Change-Id: Id8d01b055f5009850cc6e4f3d3fe45e2d68a4968 --- .../com/android/server/am/BroadcastQueueModernImpl.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 57506199f0e1e..b7a476e8e4a0b 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -747,7 +747,10 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return; } - if (mService.mProcessesReady && !r.timeoutExempt) { + // Skip ANR tracking early during boot, when requested, or when we + // immediately assume delivery success + final boolean assumeDelivered = (receiver instanceof BroadcastFilter) && !r.ordered; + if (mService.mProcessesReady && !r.timeoutExempt && !assumeDelivered) { queue.lastCpuDelayTime = queue.app.getCpuDelayTime(); final long timeout = r.isForeground() ? mFgConstants.TIMEOUT : mBgConstants.TIMEOUT; @@ -789,7 +792,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // TODO: consider making registered receivers of unordered // broadcasts report results to detect ANRs - if (!r.ordered) { + if (assumeDelivered) { finishReceiverLocked(queue, BroadcastRecord.DELIVERY_DELIVERED); } } else { From f078425b4fa68abee9fe737d79d503d87523f733 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 28 Oct 2022 09:44:01 -0600 Subject: [PATCH 2/5] BroadcastQueue: return reasons from skip policy. Adjust BroadcastSkipPolicy to return the skip "reason" as a String to let callers control the logging, but don't wire up into setDeliveryState() just yet, since there's still a lot of duplicated information, and some scripts may be parsing the current raw format. Bug: 254521331 Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Change-Id: I9ff1d3b2c86fcec6080dbeba0f5a2d4a40ccc571 --- .../server/am/BroadcastSkipPolicy.java | 236 +++++++++--------- .../android/server/am/BroadcastQueueTest.java | 6 +- 2 files changed, 119 insertions(+), 123 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastSkipPolicy.java b/services/core/java/com/android/server/am/BroadcastSkipPolicy.java index 60fddf0c7f227..481ab17b609eb 100644 --- a/services/core/java/com/android/server/am/BroadcastSkipPolicy.java +++ b/services/core/java/com/android/server/am/BroadcastSkipPolicy.java @@ -21,6 +21,7 @@ import static com.android.server.am.ActivityManagerService.checkComponentPermiss import static com.android.server.am.BroadcastQueue.TAG; import android.annotation.NonNull; +import android.annotation.Nullable; import android.app.ActivityManager; import android.app.AppGlobals; import android.app.AppOpsManager; @@ -42,6 +43,8 @@ import android.util.Slog; import com.android.internal.util.ArrayUtils; +import java.util.Objects; + /** * Policy logic that decides if delivery of a particular {@link BroadcastRecord} * should be skipped for a given {@link ResolveInfo} or {@link BroadcastFilter}. @@ -51,8 +54,8 @@ import com.android.internal.util.ArrayUtils; public class BroadcastSkipPolicy { private final ActivityManagerService mService; - public BroadcastSkipPolicy(ActivityManagerService service) { - mService = service; + public BroadcastSkipPolicy(@NonNull ActivityManagerService service) { + mService = Objects.requireNonNull(service); } /** @@ -60,18 +63,39 @@ public class BroadcastSkipPolicy { * the given {@link BroadcastFilter} or {@link ResolveInfo}. */ public boolean shouldSkip(@NonNull BroadcastRecord r, @NonNull Object target) { - if (target instanceof BroadcastFilter) { - return shouldSkip(r, (BroadcastFilter) target); + final String msg = shouldSkipMessage(r, target); + if (msg != null) { + Slog.w(TAG, msg); + return true; } else { - return shouldSkip(r, (ResolveInfo) target); + return false; + } + } + + /** + * Determine if the given {@link BroadcastRecord} is eligible to be sent to + * the given {@link BroadcastFilter} or {@link ResolveInfo}. + * + * @return message indicating why the argument should be skipped, otherwise + * {@code null} if it can proceed. + */ + public @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r, @NonNull Object target) { + if (target instanceof BroadcastFilter) { + return shouldSkipMessage(r, (BroadcastFilter) target); + } else { + return shouldSkipMessage(r, (ResolveInfo) target); } } /** * Determine if the given {@link BroadcastRecord} is eligible to be sent to * the given {@link ResolveInfo}. + * + * @return message indicating why the argument should be skipped, otherwise + * {@code null} if it can proceed. */ - public boolean shouldSkip(@NonNull BroadcastRecord r, @NonNull ResolveInfo info) { + private @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r, + @NonNull ResolveInfo info) { final BroadcastOptions brOptions = r.options; final ComponentName component = new ComponentName( info.activityInfo.applicationInfo.packageName, @@ -82,58 +106,52 @@ public class BroadcastSkipPolicy { < brOptions.getMinManifestReceiverApiLevel() || info.activityInfo.applicationInfo.targetSdkVersion > brOptions.getMaxManifestReceiverApiLevel())) { - Slog.w(TAG, "Target SDK mismatch: receiver " + info.activityInfo + return "Target SDK mismatch: receiver " + info.activityInfo + " targets " + info.activityInfo.applicationInfo.targetSdkVersion + " but delivery restricted to [" + brOptions.getMinManifestReceiverApiLevel() + ", " + brOptions.getMaxManifestReceiverApiLevel() - + "] broadcasting " + broadcastDescription(r, component)); - return true; + + "] broadcasting " + broadcastDescription(r, component); } if (brOptions != null && !brOptions.testRequireCompatChange(info.activityInfo.applicationInfo.uid)) { - Slog.w(TAG, "Compat change filtered: broadcasting " + broadcastDescription(r, component) + return "Compat change filtered: broadcasting " + broadcastDescription(r, component) + " to uid " + info.activityInfo.applicationInfo.uid + " due to compat change " - + r.options.getRequireCompatChangeId()); - return true; + + r.options.getRequireCompatChangeId(); } if (!mService.validateAssociationAllowedLocked(r.callerPackage, r.callingUid, component.getPackageName(), info.activityInfo.applicationInfo.uid)) { - Slog.w(TAG, "Association not allowed: broadcasting " - + broadcastDescription(r, component)); - return true; + return "Association not allowed: broadcasting " + + broadcastDescription(r, component); } if (!mService.mIntentFirewall.checkBroadcast(r.intent, r.callingUid, r.callingPid, r.resolvedType, info.activityInfo.applicationInfo.uid)) { - Slog.w(TAG, "Firewall blocked: broadcasting " - + broadcastDescription(r, component)); - return true; + return "Firewall blocked: broadcasting " + + broadcastDescription(r, component); } int perm = checkComponentPermission(info.activityInfo.permission, r.callingPid, r.callingUid, info.activityInfo.applicationInfo.uid, info.activityInfo.exported); if (perm != PackageManager.PERMISSION_GRANTED) { if (!info.activityInfo.exported) { - Slog.w(TAG, "Permission Denial: broadcasting " + return "Permission Denial: broadcasting " + broadcastDescription(r, component) - + " is not exported from uid " + info.activityInfo.applicationInfo.uid); + + " is not exported from uid " + info.activityInfo.applicationInfo.uid; } else { - Slog.w(TAG, "Permission Denial: broadcasting " + return "Permission Denial: broadcasting " + broadcastDescription(r, component) - + " requires " + info.activityInfo.permission); + + " requires " + info.activityInfo.permission; } - return true; } else if (info.activityInfo.permission != null) { final int opCode = AppOpsManager.permissionToOpCode(info.activityInfo.permission); if (opCode != AppOpsManager.OP_NONE && mService.getAppOpsManager().noteOpNoThrow(opCode, r.callingUid, r.callerPackage, r.callerFeatureId, "Broadcast delivered to " + info.activityInfo.name) != AppOpsManager.MODE_ALLOWED) { - Slog.w(TAG, "Appop Denial: broadcasting " + return "Appop Denial: broadcasting " + broadcastDescription(r, component) + " requires appop " + AppOpsManager.permissionToOp( - info.activityInfo.permission)); - return true; + info.activityInfo.permission); } } @@ -142,38 +160,34 @@ public class BroadcastSkipPolicy { android.Manifest.permission.INTERACT_ACROSS_USERS, info.activityInfo.applicationInfo.uid) != PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Permission Denial: Receiver " + component.flattenToShortString() + return "Permission Denial: Receiver " + component.flattenToShortString() + " requests FLAG_SINGLE_USER, but app does not hold " - + android.Manifest.permission.INTERACT_ACROSS_USERS); - return true; + + android.Manifest.permission.INTERACT_ACROSS_USERS; } } if (info.activityInfo.applicationInfo.isInstantApp() && r.callingUid != info.activityInfo.applicationInfo.uid) { - Slog.w(TAG, "Instant App Denial: receiving " + return "Instant App Denial: receiving " + r.intent + " to " + component.flattenToShortString() + " due to sender " + r.callerPackage + " (uid " + r.callingUid + ")" - + " Instant Apps do not support manifest receivers"); - return true; + + " Instant Apps do not support manifest receivers"; } if (r.callerInstantApp && (info.activityInfo.flags & ActivityInfo.FLAG_VISIBLE_TO_INSTANT_APP) == 0 && r.callingUid != info.activityInfo.applicationInfo.uid) { - Slog.w(TAG, "Instant App Denial: receiving " + return "Instant App Denial: receiving " + r.intent + " to " + component.flattenToShortString() + " requires receiver have visibleToInstantApps set" + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } if (r.curApp != null && r.curApp.mErrorState.isCrashing()) { // If the target process is crashing, just skip it. - Slog.w(TAG, "Skipping deliver ordered [" + r.queue.toString() + "] " + r - + " to " + r.curApp + ": process crashing"); - return true; + return "Skipping deliver ordered [" + r.queue.toString() + "] " + r + + " to " + r.curApp + ": process crashing"; } boolean isAvailable = false; @@ -183,15 +197,13 @@ public class BroadcastSkipPolicy { UserHandle.getUserId(info.activityInfo.applicationInfo.uid)); } catch (Exception e) { // all such failures mean we skip this receiver - Slog.w(TAG, "Exception getting recipient info for " - + info.activityInfo.packageName, e); + return "Exception getting recipient info for " + + info.activityInfo.packageName; } if (!isAvailable) { - Slog.w(TAG, - "Skipping delivery to " + info.activityInfo.packageName + " / " + return "Skipping delivery to " + info.activityInfo.packageName + " / " + info.activityInfo.applicationInfo.uid - + " : package no longer available"); - return true; + + " : package no longer available"; } // If permissions need a review before any of the app components can run, we drop @@ -201,10 +213,8 @@ public class BroadcastSkipPolicy { if (!requestStartTargetPermissionsReviewIfNeededLocked(r, info.activityInfo.packageName, UserHandle.getUserId( info.activityInfo.applicationInfo.uid))) { - Slog.w(TAG, - "Skipping delivery: permission review required for " - + broadcastDescription(r, component)); - return true; + return "Skipping delivery: permission review required for " + + broadcastDescription(r, component); } final int allowed = mService.getAppStartModeLOSP( @@ -216,10 +226,9 @@ public class BroadcastSkipPolicy { // to it and the app is in a state that should not receive it // (depending on how getAppStartModeLOSP has determined that). if (allowed == ActivityManager.APP_START_MODE_DISABLED) { - Slog.w(TAG, "Background execution disabled: receiving " + return "Background execution disabled: receiving " + r.intent + " to " - + component.flattenToShortString()); - return true; + + component.flattenToShortString(); } else if (((r.intent.getFlags()&Intent.FLAG_RECEIVER_EXCLUDE_BACKGROUND) != 0) || (r.intent.getComponent() == null && r.intent.getPackage() == null @@ -228,10 +237,9 @@ public class BroadcastSkipPolicy { && !isSignaturePerm(r.requiredPermissions))) { mService.addBackgroundCheckViolationLocked(r.intent.getAction(), component.getPackageName()); - Slog.w(TAG, "Background execution not allowed: receiving " + return "Background execution not allowed: receiving " + r.intent + " to " - + component.flattenToShortString()); - return true; + + component.flattenToShortString(); } } @@ -239,10 +247,8 @@ public class BroadcastSkipPolicy { && !mService.mUserController .isUserRunning(UserHandle.getUserId(info.activityInfo.applicationInfo.uid), 0 /* flags */)) { - Slog.w(TAG, - "Skipping delivery to " + info.activityInfo.packageName + " / " - + info.activityInfo.applicationInfo.uid + " : user is not running"); - return true; + return "Skipping delivery to " + info.activityInfo.packageName + " / " + + info.activityInfo.applicationInfo.uid + " : user is not running"; } if (r.excludedPermissions != null && r.excludedPermissions.length > 0) { @@ -268,13 +274,15 @@ public class BroadcastSkipPolicy { info.activityInfo.applicationInfo.uid, info.activityInfo.packageName) == AppOpsManager.MODE_ALLOWED)) { - return true; + return "Skipping delivery to " + info.activityInfo.packageName + + " due to excluded permission " + excludedPermission; } } else { // When there is no app op associated with the permission, // skip when permission is granted. if (perm == PackageManager.PERMISSION_GRANTED) { - return true; + return "Skipping delivery to " + info.activityInfo.packageName + + " due to excluded permission " + excludedPermission; } } } @@ -283,13 +291,12 @@ public class BroadcastSkipPolicy { // Check that the receiver does *not* belong to any of the excluded packages if (r.excludedPackages != null && r.excludedPackages.length > 0) { if (ArrayUtils.contains(r.excludedPackages, component.getPackageName())) { - Slog.w(TAG, "Skipping delivery of excluded package " + return "Skipping delivery of excluded package " + r.intent + " to " + component.flattenToShortString() + " excludes package " + component.getPackageName() + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } } @@ -307,95 +314,94 @@ public class BroadcastSkipPolicy { perm = PackageManager.PERMISSION_DENIED; } if (perm != PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Permission Denial: receiving " + return "Permission Denial: receiving " + r.intent + " to " + component.flattenToShortString() + " requires " + requiredPermission + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } int appOp = AppOpsManager.permissionToOpCode(requiredPermission); if (appOp != AppOpsManager.OP_NONE && appOp != r.appOp) { if (!noteOpForManifestReceiver(appOp, r, info, component)) { - return true; + return "Skipping delivery to " + info.activityInfo.packageName + + " due to required appop " + appOp; } } } } if (r.appOp != AppOpsManager.OP_NONE) { if (!noteOpForManifestReceiver(r.appOp, r, info, component)) { - return true; + return "Skipping delivery to " + info.activityInfo.packageName + + " due to required appop " + r.appOp; } } - return false; + return null; } /** * Determine if the given {@link BroadcastRecord} is eligible to be sent to * the given {@link BroadcastFilter}. + * + * @return message indicating why the argument should be skipped, otherwise + * {@code null} if it can proceed. */ - public boolean shouldSkip(@NonNull BroadcastRecord r, @NonNull BroadcastFilter filter) { + private @Nullable String shouldSkipMessage(@NonNull BroadcastRecord r, + @NonNull BroadcastFilter filter) { if (r.options != null && !r.options.testRequireCompatChange(filter.owningUid)) { - Slog.w(TAG, "Compat change filtered: broadcasting " + r.intent.toString() + return "Compat change filtered: broadcasting " + r.intent.toString() + " to uid " + filter.owningUid + " due to compat change " - + r.options.getRequireCompatChangeId()); - return true; + + r.options.getRequireCompatChangeId(); } if (!mService.validateAssociationAllowedLocked(r.callerPackage, r.callingUid, filter.packageName, filter.owningUid)) { - Slog.w(TAG, "Association not allowed: broadcasting " + return "Association not allowed: broadcasting " + r.intent.toString() + " from " + r.callerPackage + " (pid=" + r.callingPid + ", uid=" + r.callingUid + ") to " + filter.packageName + " through " - + filter); - return true; + + filter; } if (!mService.mIntentFirewall.checkBroadcast(r.intent, r.callingUid, r.callingPid, r.resolvedType, filter.receiverList.uid)) { - Slog.w(TAG, "Firewall blocked: broadcasting " + return "Firewall blocked: broadcasting " + r.intent.toString() + " from " + r.callerPackage + " (pid=" + r.callingPid + ", uid=" + r.callingUid + ") to " + filter.packageName + " through " - + filter); - return true; + + filter; } // Check that the sender has permission to send to this receiver if (filter.requiredPermission != null) { int perm = checkComponentPermission(filter.requiredPermission, r.callingPid, r.callingUid, -1, true); if (perm != PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Permission Denial: broadcasting " + return "Permission Denial: broadcasting " + r.intent.toString() + " from " + r.callerPackage + " (pid=" + r.callingPid + ", uid=" + r.callingUid + ")" + " requires " + filter.requiredPermission - + " due to registered receiver " + filter); - return true; + + " due to registered receiver " + filter; } else { final int opCode = AppOpsManager.permissionToOpCode(filter.requiredPermission); if (opCode != AppOpsManager.OP_NONE && mService.getAppOpsManager().noteOpNoThrow(opCode, r.callingUid, r.callerPackage, r.callerFeatureId, "Broadcast sent to protected receiver") != AppOpsManager.MODE_ALLOWED) { - Slog.w(TAG, "Appop Denial: broadcasting " + return "Appop Denial: broadcasting " + r.intent.toString() + " from " + r.callerPackage + " (pid=" + r.callingPid + ", uid=" + r.callingUid + ")" + " requires appop " + AppOpsManager.permissionToOp( filter.requiredPermission) - + " due to registered receiver " + filter); - return true; + + " due to registered receiver " + filter; } } } if ((filter.receiverList.app == null || filter.receiverList.app.isKilled() || filter.receiverList.app.mErrorState.isCrashing())) { - Slog.w(TAG, "Skipping deliver [" + r.queue.toString() + "] " + r - + " to " + filter.receiverList + ": process gone or crashing"); - return true; + return "Skipping deliver [" + r.queue.toString() + "] " + r + + " to " + filter.receiverList + ": process gone or crashing"; } // Ensure that broadcasts are only sent to other Instant Apps if they are marked as @@ -405,28 +411,26 @@ public class BroadcastSkipPolicy { if (!visibleToInstantApps && filter.instantApp && filter.receiverList.uid != r.callingUid) { - Slog.w(TAG, "Instant App Denial: receiving " + return "Instant App Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " due to sender " + r.callerPackage + " (uid " + r.callingUid + ")" - + " not specifying FLAG_RECEIVER_VISIBLE_TO_INSTANT_APPS"); - return true; + + " not specifying FLAG_RECEIVER_VISIBLE_TO_INSTANT_APPS"; } if (!filter.visibleToInstantApp && r.callerInstantApp && filter.receiverList.uid != r.callingUid) { - Slog.w(TAG, "Instant App Denial: receiving " + return "Instant App Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " requires receiver be visible to instant apps" + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } // Check that the receiver has the required permission(s) to receive this broadcast. @@ -436,15 +440,14 @@ public class BroadcastSkipPolicy { int perm = checkComponentPermission(requiredPermission, filter.receiverList.pid, filter.receiverList.uid, -1, true); if (perm != PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Permission Denial: receiving " + return "Permission Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " requires " + requiredPermission + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } int appOp = AppOpsManager.permissionToOpCode(requiredPermission); if (appOp != AppOpsManager.OP_NONE && appOp != r.appOp @@ -452,7 +455,7 @@ public class BroadcastSkipPolicy { filter.receiverList.uid, filter.packageName, filter.featureId, "Broadcast delivered to registered receiver " + filter.receiverId) != AppOpsManager.MODE_ALLOWED) { - Slog.w(TAG, "Appop Denial: receiving " + return "Appop Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid @@ -460,8 +463,7 @@ public class BroadcastSkipPolicy { + " requires appop " + AppOpsManager.permissionToOp( requiredPermission) + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } } } @@ -469,14 +471,13 @@ public class BroadcastSkipPolicy { int perm = checkComponentPermission(null, filter.receiverList.pid, filter.receiverList.uid, -1, true); if (perm != PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Permission Denial: security check failed when receiving " + return "Permission Denial: security check failed when receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } } // Check that the receiver does *not* have any excluded permissions @@ -496,7 +497,7 @@ public class BroadcastSkipPolicy { filter.receiverList.uid, filter.packageName) == AppOpsManager.MODE_ALLOWED)) { - Slog.w(TAG, "Appop Denial: receiving " + return "Appop Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid @@ -504,22 +505,20 @@ public class BroadcastSkipPolicy { + " excludes appop " + AppOpsManager.permissionToOp( excludedPermission) + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } } else { // When there is no app op associated with the permission, // skip when permission is granted. if (perm == PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Permission Denial: receiving " + return "Permission Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " excludes " + excludedPermission + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } } } @@ -528,15 +527,14 @@ public class BroadcastSkipPolicy { // Check that the receiver does *not* belong to any of the excluded packages if (r.excludedPackages != null && r.excludedPackages.length > 0) { if (ArrayUtils.contains(r.excludedPackages, filter.packageName)) { - Slog.w(TAG, "Skipping delivery of excluded package " + return "Skipping delivery of excluded package " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " excludes package " + filter.packageName + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } } @@ -546,15 +544,14 @@ public class BroadcastSkipPolicy { filter.receiverList.uid, filter.packageName, filter.featureId, "Broadcast delivered to registered receiver " + filter.receiverId) != AppOpsManager.MODE_ALLOWED) { - Slog.w(TAG, "Appop Denial: receiving " + return "Appop Denial: receiving " + r.intent.toString() + " to " + filter.receiverList.app + " (pid=" + filter.receiverList.pid + ", uid=" + filter.receiverList.uid + ")" + " requires appop " + AppOpsManager.opToName(r.appOp) + " due to sender " + r.callerPackage - + " (uid " + r.callingUid + ")"); - return true; + + " (uid " + r.callingUid + ")"; } // Ensure that broadcasts are only sent to other apps if they are explicitly marked as @@ -562,15 +559,14 @@ public class BroadcastSkipPolicy { if (!filter.exported && checkComponentPermission(null, r.callingPid, r.callingUid, filter.receiverList.uid, filter.exported) != PackageManager.PERMISSION_GRANTED) { - Slog.w(TAG, "Exported Denial: sending " + return "Exported Denial: sending " + r.intent.toString() + ", action: " + r.intent.getAction() + " from " + r.callerPackage + " (uid=" + r.callingUid + ")" + " due to receiver " + filter.receiverList.app + " (uid " + filter.receiverList.uid + ")" - + " not specifying RECEIVER_EXPORTED"); - return true; + + " not specifying RECEIVER_EXPORTED"; } // If permissions need a review before any of the app components can run, we drop @@ -579,10 +575,10 @@ public class BroadcastSkipPolicy { // broadcast. if (!requestStartTargetPermissionsReviewIfNeededLocked(r, filter.packageName, filter.owningUserId)) { - return true; + return "Skipping delivery to " + filter.packageName + " due to permissions review"; } - return false; + return null; } private static String broadcastDescription(BroadcastRecord r, ComponentName component) { 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 e1a4c1dd72566..de5960363fa5e 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java @@ -280,13 +280,13 @@ public class BroadcastQueueTest { constants.TIMEOUT = 100; constants.ALLOW_BG_ACTIVITY_START_TIMEOUT = 0; final BroadcastSkipPolicy emptySkipPolicy = new BroadcastSkipPolicy(mAms) { - public boolean shouldSkip(BroadcastRecord r, ResolveInfo info) { + public boolean shouldSkip(BroadcastRecord r, Object o) { // Ignored return false; } - public boolean shouldSkip(BroadcastRecord r, BroadcastFilter filter) { + public String shouldSkipMessage(BroadcastRecord r, Object o) { // Ignored - return false; + return null; } }; final BroadcastHistory emptyHistory = new BroadcastHistory(constants) { From 48ffbabcb973d6648e478482770eb98c18d7b3ae Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 28 Oct 2022 10:07:58 -0600 Subject: [PATCH 3/5] BroadcastQueue: better state transition logging. To aid future debugging of the modern stack, include a "reason" behind each state transition. Remove outdated TODOs that are either finished or tracked elsewhere, add misc annotations. Bug: 254521331 Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Change-Id: I307dc86b1935d5ca88a8ca20869cb9120e05bf12 --- .../android/server/am/BroadcastLoopers.java | 3 + .../server/am/BroadcastQueueModernImpl.java | 60 ++++++++++--------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastLoopers.java b/services/core/java/com/android/server/am/BroadcastLoopers.java index bebb48473fc3c..b828720c9162e 100644 --- a/services/core/java/com/android/server/am/BroadcastLoopers.java +++ b/services/core/java/com/android/server/am/BroadcastLoopers.java @@ -25,6 +25,8 @@ import android.os.SystemClock; import android.util.ArraySet; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; + import java.io.PrintWriter; import java.util.Objects; import java.util.concurrent.CountDownLatch; @@ -37,6 +39,7 @@ import java.util.concurrent.CountDownLatch; public class BroadcastLoopers { private static final String TAG = "BroadcastLoopers"; + @GuardedBy("sLoopers") private static final ArraySet sLoopers = new ArraySet<>(); /** diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index b7a476e8e4a0b..95cefea532a6e 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -144,14 +144,6 @@ class BroadcastQueueModernImpl extends BroadcastQueue { mRunning = new BroadcastProcessQueue[mConstants.MAX_RUNNING_PROCESS_QUEUES]; } - // TODO: add support for replacing pending broadcasts - // TODO: add support for merging pending broadcasts - - // TODO: consider reordering foreground broadcasts within queue - - // TODO: pause queues when background services are running - // TODO: pause queues when processes are frozen - /** * Map from UID to per-process broadcast queues. If a UID hosts more than * one process, each additional process is stored as a linked list using @@ -516,7 +508,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (queue != null) { // If queue was running a broadcast, fail it if (queue.isActive()) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, + "onApplicationCleanupLocked"); } // Skip any pending registered receivers, since the old process @@ -668,7 +661,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // Ignore registered receivers from a previous PID if (receiver instanceof BroadcastFilter) { mRunningColdStart = null; - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, + "BroadcastFilter for cold app"); return; } @@ -690,7 +684,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { hostingRecord, zygotePolicyFlags, allowWhileBooting, false); if (queue.app == null) { mRunningColdStart = null; - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, + "startProcessLocked failed"); return; } } @@ -721,29 +716,30 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // If someone already finished this broadcast, finish immediately final int oldDeliveryState = getDeliveryState(r, index); if (isDeliveryStateTerminal(oldDeliveryState)) { - finishReceiverLocked(queue, oldDeliveryState); + finishReceiverLocked(queue, oldDeliveryState, "already terminal state"); return; } // Consider additional cases where we'd want to finish immediately if (app.isInFullBackup()) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, "isInFullBackup"); return; } if (mSkipPolicy.shouldSkip(r, receiver)) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, "mSkipPolicy"); return; } final Intent receiverIntent = r.getReceiverIntent(receiver); if (receiverIntent == null) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, "isInFullBackup"); return; } // Ignore registered receivers from a previous PID if ((receiver instanceof BroadcastFilter) && ((BroadcastFilter) receiver).receiverList.pid != app.getPid()) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, + "BroadcastFilter for mismatched PID"); return; } @@ -778,7 +774,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } if (DEBUG_BROADCAST) logv("Scheduling " + r + " to warm " + app); - setDeliveryState(queue, app, r, index, receiver, BroadcastRecord.DELIVERY_SCHEDULED); + setDeliveryState(queue, app, r, index, receiver, BroadcastRecord.DELIVERY_SCHEDULED, + "scheduleReceiverWarmLocked"); final IApplicationThread thread = app.getOnewayThread(); if (thread != null) { @@ -793,7 +790,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // TODO: consider making registered receivers of unordered // broadcasts report results to detect ANRs if (assumeDelivered) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_DELIVERED); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_DELIVERED, + "assuming delivered"); } } else { notifyScheduleReceiver(app, r, (ResolveInfo) receiver); @@ -807,10 +805,11 @@ class BroadcastQueueModernImpl extends BroadcastQueue { logw(msg); app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, null); app.setKilled(true); - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, "remote app"); } } else { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, + "missing IApplicationThread"); } } @@ -854,7 +853,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } private void deliveryTimeoutHardLocked(@NonNull BroadcastProcessQueue queue) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_TIMEOUT); + finishReceiverLocked(queue, BroadcastRecord.DELIVERY_TIMEOUT, + "deliveryTimeoutHardLocked"); } @Override @@ -881,16 +881,16 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (r.resultAbort) { for (int i = r.terminalCount + 1; i < r.receivers.size(); i++) { setDeliveryState(null, null, r, i, r.receivers.get(i), - BroadcastRecord.DELIVERY_SKIPPED); + BroadcastRecord.DELIVERY_SKIPPED, "resultAbort"); } } } - return finishReceiverLocked(queue, BroadcastRecord.DELIVERY_DELIVERED); + return finishReceiverLocked(queue, BroadcastRecord.DELIVERY_DELIVERED, "remote app"); } private boolean finishReceiverLocked(@NonNull BroadcastProcessQueue queue, - @DeliveryState int deliveryState) { + @DeliveryState int deliveryState, @NonNull String reason) { checkState(queue.isActive(), "isActive"); final ProcessRecord app = queue.app; @@ -898,7 +898,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { final int index = queue.getActiveIndex(); final Object receiver = r.receivers.get(index); - setDeliveryState(queue, app, r, index, receiver, deliveryState); + setDeliveryState(queue, app, r, index, receiver, deliveryState, reason); if (deliveryState == BroadcastRecord.DELIVERY_TIMEOUT) { r.anrCount++; @@ -945,7 +945,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { */ private void setDeliveryState(@Nullable BroadcastProcessQueue queue, @Nullable ProcessRecord app, @NonNull BroadcastRecord r, int index, - @NonNull Object receiver, @DeliveryState int newDeliveryState) { + @NonNull Object receiver, @DeliveryState int newDeliveryState, String reason) { final int oldDeliveryState = getDeliveryState(r, index); // Only apply state when we haven't already reached a terminal state; @@ -973,7 +973,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { logw("Delivery state of " + r + " to " + receiver + " via " + app + " changed from " + deliveryStateToString(oldDeliveryState) + " to " - + deliveryStateToString(newDeliveryState)); + + deliveryStateToString(newDeliveryState) + " because " + reason); } r.terminalCount++; @@ -1063,7 +1063,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { * of it matching a predicate. */ private final BroadcastConsumer mBroadcastConsumerSkip = (r, i) -> { - setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED); + setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED, + "mBroadcastConsumerSkip"); }; /** @@ -1071,7 +1072,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { * cancelled, usually as a result of it matching a predicate. */ private final BroadcastConsumer mBroadcastConsumerSkipAndCanceled = (r, i) -> { - setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED); + setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_SKIPPED, + "mBroadcastConsumerSkipAndCanceled"); r.resultCode = Activity.RESULT_CANCELED; r.resultData = null; r.resultExtras = null; From f0264d791f21b3c176749bb66934737ebcd2a22d Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 1 Nov 2022 08:58:01 -0600 Subject: [PATCH 4/5] BroadcastQueue: post finishReceiver() as async. In the typical case, the remote process calls back around to finishReceiver() once it's finished. In cases were we shortcutted delivery, we had been calling finishReceiver() directly, but that results in holding the AMS lock for a long period of time as we recursively deliver the next pending broadcast. This change adjusts that shortcutting behavior to post through a message, so that we release the AMS lock, giving other pending operations a chance to run. Bug: 255936508 Test: ./frameworks/base/libs/hwui/tests/scripts/prep_generic.sh little && atest ActivityManagerPerfTests:BroadcastPerfTest Change-Id: Ic7094df0a1d2aa1e2ac1e22c1fd711c245d8eb86 --- .../server/am/BroadcastQueueModernImpl.java | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 95cefea532a6e..a1686ad986fc5 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -214,12 +214,22 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private static final int MSG_DELIVERY_TIMEOUT_HARD = 3; private static final int MSG_BG_ACTIVITY_START_TIMEOUT = 4; private static final int MSG_CHECK_HEALTH = 5; + private static final int MSG_FINISH_RECEIVER = 6; private void enqueueUpdateRunningList() { mLocalHandler.removeMessages(MSG_UPDATE_RUNNING_LIST); mLocalHandler.sendEmptyMessage(MSG_UPDATE_RUNNING_LIST); } + private void enqueueFinishReceiver(@NonNull BroadcastProcessQueue queue, + @DeliveryState int deliveryState, @NonNull String reason) { + final SomeArgs args = SomeArgs.obtain(); + args.arg1 = queue; + args.argi1 = deliveryState; + args.arg2 = reason; + mLocalHandler.sendMessage(Message.obtain(mLocalHandler, MSG_FINISH_RECEIVER, args)); + } + private final Handler mLocalHandler; private final Handler.Callback mLocalCallback = (msg) -> { @@ -258,6 +268,17 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } return true; } + case MSG_FINISH_RECEIVER: { + synchronized (mService) { + final SomeArgs args = (SomeArgs) msg.obj; + final BroadcastProcessQueue queue = (BroadcastProcessQueue) args.arg1; + final int deliveryState = args.argi1; + final String reason = (String) args.arg2; + args.recycle(); + finishReceiverLocked(queue, deliveryState, reason); + } + return true; + } } return false; }; @@ -661,7 +682,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // Ignore registered receivers from a previous PID if (receiver instanceof BroadcastFilter) { mRunningColdStart = null; - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_SKIPPED, "BroadcastFilter for cold app"); return; } @@ -684,7 +705,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { hostingRecord, zygotePolicyFlags, allowWhileBooting, false); if (queue.app == null) { mRunningColdStart = null; - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_FAILURE, "startProcessLocked failed"); return; } @@ -716,29 +737,29 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // If someone already finished this broadcast, finish immediately final int oldDeliveryState = getDeliveryState(r, index); if (isDeliveryStateTerminal(oldDeliveryState)) { - finishReceiverLocked(queue, oldDeliveryState, "already terminal state"); + enqueueFinishReceiver(queue, oldDeliveryState, "already terminal state"); return; } // Consider additional cases where we'd want to finish immediately if (app.isInFullBackup()) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, "isInFullBackup"); + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_SKIPPED, "isInFullBackup"); return; } if (mSkipPolicy.shouldSkip(r, receiver)) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, "mSkipPolicy"); + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_SKIPPED, "mSkipPolicy"); return; } final Intent receiverIntent = r.getReceiverIntent(receiver); if (receiverIntent == null) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, "isInFullBackup"); + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_SKIPPED, "isInFullBackup"); return; } // Ignore registered receivers from a previous PID if ((receiver instanceof BroadcastFilter) && ((BroadcastFilter) receiver).receiverList.pid != app.getPid()) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_SKIPPED, + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_SKIPPED, "BroadcastFilter for mismatched PID"); return; } @@ -790,7 +811,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // TODO: consider making registered receivers of unordered // broadcasts report results to detect ANRs if (assumeDelivered) { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_DELIVERED, + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_DELIVERED, "assuming delivered"); } } else { @@ -805,10 +826,10 @@ class BroadcastQueueModernImpl extends BroadcastQueue { logw(msg); app.scheduleCrashLocked(msg, CannotDeliverBroadcastException.TYPE_ID, null); app.setKilled(true); - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, "remote app"); + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_FAILURE, "remote app"); } } else { - finishReceiverLocked(queue, BroadcastRecord.DELIVERY_FAILURE, + enqueueFinishReceiver(queue, BroadcastRecord.DELIVERY_FAILURE, "missing IApplicationThread"); } } From c16955aa41551009ea8e70fd0b73114f3de6bf0c Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 1 Nov 2022 10:31:00 -0600 Subject: [PATCH 5/5] BroadcastQueue: add more performance tracing. To better understand what the modern stack is doing, add tracing events for common operations. Bug: 255936508 Test: ./frameworks/base/libs/hwui/tests/scripts/prep_generic.sh little && atest ActivityManagerPerfTests:BroadcastPerfTest Change-Id: Iad3f83e85a79a1d134ffe74291ecc631f3fac66c --- .../server/am/ActivityManagerService.java | 25 ++++++++++++++ .../com/android/server/am/BroadcastQueue.java | 13 +++++++ .../server/am/BroadcastQueueModernImpl.java | 34 +++++++++---------- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 2eaddb1c37f1d..062afe93e63d1 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -13874,6 +13874,29 @@ public class ActivityManagerService extends IActivityManager.Stub @Nullable IBinder backgroundActivityStartsToken, @Nullable int[] broadcastAllowList, @Nullable BiFunction filterExtrasForReceiver) { + final int cookie = BroadcastQueue.traceBegin("broadcastIntentLockedTraced"); + final int res = broadcastIntentLockedTraced(callerApp, callerPackage, callerFeatureId, + intent, resolvedType, resultToApp, resultTo, resultCode, resultData, resultExtras, + requiredPermissions, excludedPermissions, excludedPackages, appOp, bOptions, + ordered, sticky, callingPid, callingUid, realCallingUid, realCallingPid, userId, + allowBackgroundActivityStarts, backgroundActivityStartsToken, broadcastAllowList, + filterExtrasForReceiver); + BroadcastQueue.traceEnd(cookie); + return res; + } + + @GuardedBy("this") + final int broadcastIntentLockedTraced(ProcessRecord callerApp, String callerPackage, + @Nullable String callerFeatureId, Intent intent, String resolvedType, + ProcessRecord resultToApp, IIntentReceiver resultTo, int resultCode, String resultData, + Bundle resultExtras, String[] requiredPermissions, + String[] excludedPermissions, String[] excludedPackages, int appOp, Bundle bOptions, + boolean ordered, boolean sticky, int callingPid, int callingUid, + int realCallingUid, int realCallingPid, int userId, + boolean allowBackgroundActivityStarts, + @Nullable IBinder backgroundActivityStartsToken, + @Nullable int[] broadcastAllowList, + @Nullable BiFunction filterExtrasForReceiver) { // Ensure all internal loopers are registered for idle checks BroadcastLoopers.addMyLooper(); @@ -14425,6 +14448,7 @@ public class ActivityManagerService extends IActivityManager.Stub } // Figure out who all will receive this broadcast. + final int cookie = BroadcastQueue.traceBegin("queryReceivers"); List receivers = null; List registeredReceivers = null; // Need to resolve the intent to interested receivers... @@ -14455,6 +14479,7 @@ public class ActivityManagerService extends IActivityManager.Stub resolvedType, false /*defaultOnly*/, userId); } } + BroadcastQueue.traceEnd(cookie); final boolean replacePending = (intent.getFlags()&Intent.FLAG_RECEIVER_REPLACE_PENDING) != 0; diff --git a/services/core/java/com/android/server/am/BroadcastQueue.java b/services/core/java/com/android/server/am/BroadcastQueue.java index 1e172fc92f40d..e0fab2cfec21c 100644 --- a/services/core/java/com/android/server/am/BroadcastQueue.java +++ b/services/core/java/com/android/server/am/BroadcastQueue.java @@ -24,6 +24,7 @@ import android.content.Intent; import android.os.Bundle; import android.os.DropBoxManager; import android.os.Handler; +import android.os.Trace; import android.util.Slog; import android.util.proto.ProtoOutputStream; @@ -76,6 +77,18 @@ public abstract class BroadcastQueue { } } + static int traceBegin(@NonNull String methodName) { + final int cookie = methodName.hashCode(); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, + TAG, methodName, cookie); + return cookie; + } + + static void traceEnd(int cookie) { + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER, + TAG, cookie); + } + @Override public String toString() { return mQueueName; diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index a1686ad986fc5..af2a97e626171 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -64,7 +64,6 @@ import android.os.Message; import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; -import android.os.Trace; import android.os.UserHandle; import android.text.format.DateUtils; import android.util.IndentingPrintWriter; @@ -322,6 +321,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return; } + final int cookie = traceBegin("updateRunnableList"); final boolean wantQueue = queue.isRunnable(); final boolean inQueue = (queue == mRunnableHead) || (queue.runnableAtPrev != null) || (queue.runnableAtNext != null); @@ -348,6 +348,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (queue.isEmpty() && !queue.isActive() && !queue.isProcessWarm()) { removeProcessQueue(queue.processName, queue.uid); } + + traceEnd(cookie); } /** @@ -362,7 +364,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { int avail = mRunning.length - getRunningSize(); if (avail == 0) return; - final int cookie = traceBegin(TAG, "updateRunningList"); + final int cookie = traceBegin("updateRunningList"); final long now = SystemClock.uptimeMillis(); // If someone is waiting for a state, everything is runnable now @@ -462,7 +464,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { }); } - traceEnd(TAG, cookie); + traceEnd(cookie); } @Override @@ -558,6 +560,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { public void enqueueBroadcastLocked(@NonNull BroadcastRecord r) { if (DEBUG_BROADCAST) logv("Enqueuing " + r + " for " + r.receivers.size() + " receivers"); + final int cookie = traceBegin("enqueueBroadcast"); r.applySingletonPolicy(mService); final IntentFilter removeMatchingFilter = (r.options != null) @@ -627,6 +630,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (r.receivers.isEmpty()) { scheduleResultTo(r); } + + traceEnd(cookie); } private void applyDeliveryGroupPolicy(@NonNull BroadcastRecord r) { @@ -912,6 +917,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private boolean finishReceiverLocked(@NonNull BroadcastProcessQueue queue, @DeliveryState int deliveryState, @NonNull String reason) { + final int cookie = traceBegin("finishReceiver"); checkState(queue.isActive(), "isActive"); final ProcessRecord app = queue.app; @@ -938,11 +944,12 @@ class BroadcastQueueModernImpl extends BroadcastQueue { final boolean shouldRetire = (queue.getActiveCountSinceIdle() >= mConstants.MAX_RUNNING_ACTIVE_BROADCASTS); + final boolean res; if (queue.isRunnable() && queue.isProcessWarm() && !shouldRetire) { // We're on a roll; move onto the next broadcast for this process queue.makeActiveNextPending(); scheduleReceiverWarmLocked(queue); - return true; + res = true; } else { // We've drained running broadcasts; maybe move back to runnable queue.makeActiveIdle(); @@ -956,8 +963,10 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // Tell other OS components that app is not actively running, giving // a chance to update OOM adjustment notifyStoppedRunning(queue); - return false; + res = false; } + traceEnd(cookie); + return res; } /** @@ -967,6 +976,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private void setDeliveryState(@Nullable BroadcastProcessQueue queue, @Nullable ProcessRecord app, @NonNull BroadcastRecord r, int index, @NonNull Object receiver, @DeliveryState int newDeliveryState, String reason) { + final int cookie = traceBegin("setDeliveryState"); final int oldDeliveryState = getDeliveryState(r, index); // Only apply state when we haven't already reached a terminal state; @@ -1024,6 +1034,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue { enqueueUpdateRunningList(); } } + + traceEnd(cookie); } private @DeliveryState int getDeliveryState(@NonNull BroadcastRecord r, int index) { @@ -1286,18 +1298,6 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } } - private int traceBegin(String trackName, String methodName) { - final int cookie = methodName.hashCode(); - Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, - trackName, methodName, cookie); - return cookie; - } - - private void traceEnd(String trackName, int cookie) { - Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER, - trackName, cookie); - } - private void updateWarmProcess(@NonNull BroadcastProcessQueue queue) { if (!queue.isProcessWarm()) { queue.setProcess(mService.getProcessRecordLocked(queue.processName, queue.uid));