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
This commit is contained in:
Sudheer Shanka
2023-01-04 22:28:03 -08:00
parent e6342bec18
commit 5cb9f358a9
3 changed files with 94 additions and 4 deletions

View File

@@ -1341,7 +1341,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
@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 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
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)) {

View File

@@ -25,7 +25,10 @@ import static com.android.server.am.BroadcastProcessQueue.REASON_CONTAINS_PRIORI
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 @@ public class BroadcastQueueModernImplTest {
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);

View File

@@ -25,6 +25,7 @@ import static org.junit.Assert.assertEquals;
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 @@ public class BroadcastQueueTest {
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 @@ public class BroadcastQueueTest {
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.
*/