Fix user cleanup in modern broadcast queue.
When a user is removed, we iterate through all
existing processes in that user and mark any pending
receivers as "skipped" but the way we are currently
iterating and removing the process queues at the same
time could result in skipping over some process queues.
Bug: 259529372
Bug: 260158381
Test: atest services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
Test: atest services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
Change-Id: I3487756ca4c230d3a67fa2f85751aefeb055a66e
diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java
index 8f241b2..15dfd72 100644
--- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java
+++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java
@@ -1341,7 +1341,7 @@
@NonNull BroadcastPredicate broadcastPredicate,
@NonNull BroadcastConsumer broadcastConsumer, boolean andRemove) {
boolean didSomething = false;
- for (int i = 0; i < mProcessQueues.size(); i++) {
+ for (int i = mProcessQueues.size() - 1; i >= 0; i--) {
BroadcastProcessQueue leaf = mProcessQueues.valueAt(i);
while (leaf != null) {
if (queuePredicate.test(leaf)) {
@@ -1363,7 +1363,7 @@
private void forEachMatchingQueue(
@NonNull Predicate<BroadcastProcessQueue> queuePredicate,
@NonNull Consumer<BroadcastProcessQueue> queueConsumer) {
- for (int i = 0; i < mProcessQueues.size(); i++) {
+ for (int i = mProcessQueues.size() - 1; i >= 0; i--) {
BroadcastProcessQueue leaf = mProcessQueues.valueAt(i);
while (leaf != null) {
if (queuePredicate.test(leaf)) {
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
index 89077e3..77127c5 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
@@ -25,7 +25,10 @@
import static com.android.server.am.BroadcastProcessQueue.REASON_CONTAINS_RESULT_TO;
import static com.android.server.am.BroadcastProcessQueue.insertIntoRunnableList;
import static com.android.server.am.BroadcastProcessQueue.removeFromRunnableList;
+import static com.android.server.am.BroadcastQueueTest.CLASS_BLUE;
import static com.android.server.am.BroadcastQueueTest.CLASS_GREEN;
+import static com.android.server.am.BroadcastQueueTest.CLASS_RED;
+import static com.android.server.am.BroadcastQueueTest.CLASS_YELLOW;
import static com.android.server.am.BroadcastQueueTest.PACKAGE_BLUE;
import static com.android.server.am.BroadcastQueueTest.PACKAGE_GREEN;
import static com.android.server.am.BroadcastQueueTest.PACKAGE_RED;
@@ -981,6 +984,40 @@
assertTrue(queue.isEmpty());
}
+ @Test
+ public void testCleanupDisabledPackageReceiversLocked() {
+ final Intent userPresent = new Intent(Intent.ACTION_USER_PRESENT);
+ final Intent timeTick = new Intent(Intent.ACTION_TIME_TICK);
+
+ final BroadcastRecord record1 = makeBroadcastRecord(userPresent, List.of(
+ makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN),
+ makeManifestReceiver(PACKAGE_RED, CLASS_BLUE),
+ makeManifestReceiver(PACKAGE_YELLOW, CLASS_RED),
+ makeManifestReceiver(PACKAGE_BLUE, CLASS_GREEN)
+ ));
+ final BroadcastRecord record2 = makeBroadcastRecord(timeTick, List.of(
+ makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN),
+ makeManifestReceiver(PACKAGE_RED, CLASS_RED),
+ makeManifestReceiver(PACKAGE_YELLOW, CLASS_YELLOW)
+ ));
+
+ // Halt all processing so that we get a consistent view
+ mHandlerThread.getLooper().getQueue().postSyncBarrier();
+ mImpl.enqueueBroadcastLocked(record1);
+ mImpl.enqueueBroadcastLocked(record2);
+
+ mImpl.cleanupDisabledPackageReceiversLocked(null, null, UserHandle.USER_SYSTEM);
+
+ // Verify that all receivers have been marked as "skipped".
+ for (BroadcastRecord record : new BroadcastRecord[] {record1, record2}) {
+ for (int i = 0; i < record.receivers.size(); ++i) {
+ final String errMsg = "Unexpected delivery state for record:" + record
+ + "; receiver=" + record.receivers.get(i);
+ assertEquals(errMsg, BroadcastRecord.DELIVERY_SKIPPED, record.getDeliveryState(i));
+ }
+ }
+ }
+
private Intent createPackageChangedIntent(int uid, List<String> componentNameList) {
final Intent packageChangedIntent = new Intent(Intent.ACTION_PACKAGE_CHANGED);
packageChangedIntent.putExtra(Intent.EXTRA_UID, uid);
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 6800d52..b6a53ac 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
@@ -25,6 +25,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -805,14 +806,27 @@
null, null, null, null, null, null, userId, null));
}
+ private void verifyScheduleReceiver(VerificationMode mode, ProcessRecord app,
+ int userId) throws Exception {
+ verify(app.getThread(), mode).scheduleReceiverList(
+ manifestReceiver(null,
+ null, null, null, null, null, null, userId, null));
+ }
+
private void verifyScheduleRegisteredReceiver(ProcessRecord app,
- Intent intent)
- throws Exception {
+ Intent intent) throws Exception {
verify(app.getThread()).scheduleReceiverList(
registeredReceiver(null, filterEqualsIgnoringComponent(intent),
null, null, null, null, null, UserHandle.USER_SYSTEM, null));
}
+ private void verifyScheduleRegisteredReceiver(VerificationMode mode, ProcessRecord app,
+ int userId) throws Exception {
+ verify(app.getThread(), mode).scheduleReceiverList(
+ registeredReceiver(null, null,
+ null, null, null, null, null, userId, null));
+ }
+
static final int USER_GUEST = 11;
static final String PACKAGE_ANDROID = "android";
@@ -1222,6 +1236,45 @@
new ComponentName(PACKAGE_GREEN, CLASS_BLUE));
}
+ @Test
+ public void testCleanup_userRemoved() throws Exception {
+ final ProcessRecord callerApp = makeActiveProcessRecord(
+ PACKAGE_RED, PACKAGE_RED, ProcessBehavior.NORMAL,
+ USER_GUEST);
+
+ final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
+ final Intent timeZone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);
+ try (SyncBarrier b = new SyncBarrier()) {
+ enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, USER_GUEST, new ArrayList<>(
+ List.of(makeRegisteredReceiver(callerApp),
+ makeManifestReceiver(PACKAGE_GREEN, CLASS_RED, USER_GUEST),
+ makeManifestReceiver(PACKAGE_BLUE, CLASS_GREEN, USER_GUEST),
+ makeManifestReceiver(PACKAGE_YELLOW, CLASS_BLUE, USER_GUEST)))));
+ enqueueBroadcast(makeBroadcastRecord(timeZone, callerApp, USER_GUEST, new ArrayList<>(
+ List.of(makeRegisteredReceiver(callerApp),
+ makeManifestReceiver(PACKAGE_GREEN, CLASS_RED, USER_GUEST),
+ makeManifestReceiver(PACKAGE_BLUE, CLASS_GREEN, USER_GUEST),
+ makeManifestReceiver(PACKAGE_YELLOW, CLASS_BLUE, USER_GUEST)))));
+
+ synchronized (mAms) {
+ mQueue.cleanupDisabledPackageReceiversLocked(null, null, USER_GUEST);
+ }
+ }
+
+ waitForIdle();
+ // Legacy stack does not remove registered receivers as part of
+ // cleanUpDisabledPackageReceiversLocked() call, so verify this only on modern queue.
+ if (mImpl == Impl.MODERN) {
+ verifyScheduleReceiver(never(), callerApp, USER_GUEST);
+ verifyScheduleRegisteredReceiver(never(), callerApp, USER_GUEST);
+ }
+ for (String pkg : new String[] {
+ PACKAGE_GREEN, PACKAGE_BLUE, PACKAGE_YELLOW
+ }) {
+ assertNull(mAms.getProcessRecordLocked(pkg, getUidForPackage(pkg, USER_GUEST)));
+ }
+ }
+
/**
* Verify that killing a running process skips registered receivers.
*/