diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 8aeae5aff3ccf..fa3d4caa860b1 100644 --- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java +++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java @@ -21,6 +21,7 @@ import static com.android.server.am.BroadcastRecord.deliveryStateToString; import static com.android.server.am.BroadcastRecord.isDeliveryStateTerminal; import static com.android.server.am.BroadcastRecord.isReceiverEquals; +import android.annotation.CheckResult; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -341,7 +342,12 @@ class BroadcastProcessQueue { * Predicates that choose to remove a broadcast must finish * delivery of the matched broadcast, to ensure that situations like ordered * broadcasts are handled consistently. + * + * @return if this operation may have changed internal state, indicating + * that the caller is responsible for invoking + * {@link BroadcastQueueModernImpl#updateRunnableList} */ + @CheckResult public boolean forEachMatchingBroadcast(@NonNull BroadcastPredicate predicate, @NonNull BroadcastConsumer consumer, boolean andRemove) { boolean didSomething = false; @@ -354,6 +360,7 @@ class BroadcastProcessQueue { return didSomething; } + @CheckResult private boolean forEachMatchingBroadcastInQueue(@NonNull ArrayDeque queue, @NonNull BroadcastPredicate predicate, @NonNull BroadcastConsumer consumer, boolean andRemove) { @@ -370,6 +377,10 @@ class BroadcastProcessQueue { args.recycle(); it.remove(); onBroadcastDequeued(record, recordIndex, recordWouldBeSkipped); + } else { + // Even if we're leaving broadcast in queue, it may have + // been mutated in such a way to change our runnable time + invalidateRunnableAt(); } didSomething = true; } @@ -381,30 +392,43 @@ class BroadcastProcessQueue { /** * Update the actively running "warm" process for this process. + * + * @return if this operation may have changed internal state, indicating + * that the caller is responsible for invoking + * {@link BroadcastQueueModernImpl#updateRunnableList} */ - public void setProcess(@Nullable ProcessRecord app) { + @CheckResult + public boolean setProcessAndUidFrozen(@Nullable ProcessRecord app, boolean uidFrozen) { this.app = app; - if (app != null) { - setProcessInstrumented(app.getActiveInstrumentation() != null); - setProcessPersistent(app.isPersistent()); - } else { - setProcessInstrumented(false); - setProcessPersistent(false); - } // Since we may have just changed our PID, invalidate cached strings mCachedToString = null; mCachedToShortString = null; + + boolean didSomething = false; + if (app != null) { + didSomething |= setProcessInstrumented(app.getActiveInstrumentation() != null); + didSomething |= setProcessPersistent(app.isPersistent()); + didSomething |= setUidFrozen(uidFrozen); + } else { + didSomething |= setProcessInstrumented(false); + didSomething |= setProcessPersistent(false); + didSomething |= setUidFrozen(uidFrozen); + } + return didSomething; } /** * Update if this UID is in the "frozen" state, typically signaling that * broadcast dispatch should be paused or delayed. */ - public void setUidFrozen(boolean frozen) { + private boolean setUidFrozen(boolean frozen) { if (mUidFrozen != frozen) { mUidFrozen = frozen; invalidateRunnableAt(); + return true; + } else { + return false; } } @@ -413,10 +437,13 @@ class BroadcastProcessQueue { * signaling that broadcast dispatch should bypass all pauses or delays, to * avoid holding up test suites. */ - public void setProcessInstrumented(boolean instrumented) { + private boolean setProcessInstrumented(boolean instrumented) { if (mProcessInstrumented != instrumented) { mProcessInstrumented = instrumented; invalidateRunnableAt(); + return true; + } else { + return false; } } @@ -424,10 +451,13 @@ class BroadcastProcessQueue { * Update if this process is in the "persistent" state, which signals broadcast dispatch should * bypass all pauses or delays to prevent the system from becoming out of sync with itself. */ - public void setProcessPersistent(boolean persistent) { + private boolean setProcessPersistent(boolean persistent) { if (mProcessPersistent != persistent) { mProcessPersistent = persistent; invalidateRunnableAt(); + return true; + } else { + return false; } } @@ -647,8 +677,20 @@ class BroadcastProcessQueue { return mActive != null; } - void forceDelayBroadcastDelivery(long delayedDurationMs) { - mForcedDelayedDurationMs = delayedDurationMs; + /** + * @return if this operation may have changed internal state, indicating + * that the caller is responsible for invoking + * {@link BroadcastQueueModernImpl#updateRunnableList} + */ + @CheckResult + boolean forceDelayBroadcastDelivery(long delayedDurationMs) { + if (mForcedDelayedDurationMs != delayedDurationMs) { + mForcedDelayedDurationMs = delayedDurationMs; + invalidateRunnableAt(); + return true; + } else { + return false; + } } /** @@ -720,10 +762,21 @@ class BroadcastProcessQueue { * broadcasts would be prioritized for dispatching, even if there are urgent broadcasts * waiting. This is typically used in case there are callers waiting for "barrier" to be * reached. + * + * @return if this operation may have changed internal state, indicating + * that the caller is responsible for invoking + * {@link BroadcastQueueModernImpl#updateRunnableList} */ + @CheckResult @VisibleForTesting - void setPrioritizeEarliest(boolean prioritizeEarliest) { - mPrioritizeEarliest = prioritizeEarliest; + boolean setPrioritizeEarliest(boolean prioritizeEarliest) { + if (mPrioritizeEarliest != prioritizeEarliest) { + mPrioritizeEarliest = prioritizeEarliest; + invalidateRunnableAt(); + return true; + } else { + return false; + } } /** diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index b11599ede3b7c..f31c017cd2e87 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -360,8 +360,6 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // If app isn't running, and there's nothing in the queue, clean up if (queue.isEmpty() && !queue.isActive() && !queue.isProcessWarm()) { removeProcessQueue(queue.processName, queue.uid); - } else { - updateQueueDeferred(queue); } } @@ -496,8 +494,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // relevant per-process queue final BroadcastProcessQueue queue = getProcessQueue(app); if (queue != null) { - queue.setProcess(app); - queue.setUidFrozen(mUidFrozen.get(queue.uid, false)); + setQueueProcess(queue, app); } boolean didSomething = false; @@ -538,8 +535,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { // relevant per-process queue final BroadcastProcessQueue queue = getProcessQueue(app); if (queue != null) { - queue.setProcess(null); - queue.setUidFrozen(mUidFrozen.get(queue.uid, false)); + setQueueProcess(queue, null); } if ((mRunningColdStart != null) && (mRunningColdStart == queue)) { @@ -566,6 +562,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return (r.receivers.get(i) instanceof BroadcastFilter); }, mBroadcastConsumerSkip, true); if (didSomething || queue.isEmpty()) { + updateQueueDeferred(queue); updateRunnableList(queue); enqueueUpdateRunningList(); } @@ -631,6 +628,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { setDeliveryState(queue, null, r, i, receiver, BroadcastRecord.DELIVERY_DEFERRED, "deferred at enqueue time"); } + updateQueueDeferred(queue); updateRunnableList(queue); enqueueUpdateRunningList(); } @@ -1079,6 +1077,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { final int queueIndex = getRunningIndexOf(queue); mRunning[queueIndex] = null; + updateQueueDeferred(queue); updateRunnableList(queue); enqueueUpdateRunningList(); @@ -1161,6 +1160,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { getReceiverUid(otherReceiver)); if (otherQueue != null) { otherQueue.invalidateRunnableAt(); + updateQueueDeferred(otherQueue); updateRunnableList(otherQueue); } } @@ -1291,6 +1291,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (queuePredicate.test(leaf)) { if (leaf.forEachMatchingBroadcast(broadcastPredicate, broadcastConsumer, andRemove)) { + updateQueueDeferred(leaf); updateRunnableList(leaf); didSomething = true; } @@ -1304,29 +1305,40 @@ class BroadcastQueueModernImpl extends BroadcastQueue { return didSomething; } - private void forEachMatchingQueue( + private boolean forEachMatchingQueue( @NonNull Predicate queuePredicate, @NonNull Consumer queueConsumer) { + boolean didSomething = false; for (int i = mProcessQueues.size() - 1; i >= 0; i--) { BroadcastProcessQueue leaf = mProcessQueues.valueAt(i); while (leaf != null) { if (queuePredicate.test(leaf)) { queueConsumer.accept(leaf); + updateQueueDeferred(leaf); updateRunnableList(leaf); + didSomething = true; } leaf = leaf.processNameNext; } } + if (didSomething) { + enqueueUpdateRunningList(); + } + return didSomething; } - private void updateQueueDeferred( - @NonNull BroadcastProcessQueue leaf) { + @SuppressWarnings("CheckResult") + private void updateQueueDeferred(@NonNull BroadcastProcessQueue leaf) { if (leaf.isDeferredUntilActive()) { + // We ignore the returned value here since callers are invoking us + // just before updateRunnableList() leaf.forEachMatchingBroadcast((r, i) -> { return r.deferUntilActive && (r.getDeliveryState(i) == BroadcastRecord.DELIVERY_PENDING); }, mBroadcastConsumerDefer, false); } else if (leaf.hasDeferredBroadcasts()) { + // We ignore the returned value here since callers are invoking us + // just before updateRunnableList() leaf.forEachMatchingBroadcast((r, i) -> { return r.deferUntilActive && (r.getDeliveryState(i) == BroadcastRecord.DELIVERY_DEFERRED); @@ -1356,10 +1368,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { while (leaf != null) { // Update internal state by refreshing values previously // read from any known running process - leaf.setProcess(leaf.app); - leaf.setUidFrozen(frozen); - updateQueueDeferred(leaf); - updateRunnableList(leaf); + setQueueProcess(leaf, leaf.app); leaf = leaf.processNameNext; } enqueueUpdateRunningList(); @@ -1520,8 +1529,20 @@ class BroadcastQueueModernImpl extends BroadcastQueue { private void updateWarmProcess(@NonNull BroadcastProcessQueue queue) { if (!queue.isProcessWarm()) { - queue.setProcess(mService.getProcessRecordLocked(queue.processName, queue.uid)); - queue.setUidFrozen(mUidFrozen.get(queue.uid, false)); + setQueueProcess(queue, mService.getProcessRecordLocked(queue.processName, queue.uid)); + } + } + + /** + * Update the {@link ProcessRecord} associated with the given + * {@link BroadcastProcessQueue}. Also updates any runnable status that + * might have changed as a side-effect. + */ + private void setQueueProcess(@NonNull BroadcastProcessQueue queue, + @Nullable ProcessRecord app) { + if (queue.setProcessAndUidFrozen(app, mUidFrozen.get(queue.uid, false))) { + updateQueueDeferred(queue); + updateRunnableList(queue); } } @@ -1715,8 +1736,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } BroadcastProcessQueue created = new BroadcastProcessQueue(mConstants, processName, uid); - created.setProcess(mService.getProcessRecordLocked(processName, uid)); - created.setUidFrozen(mUidFrozen.get(uid, false)); + setQueueProcess(created, mService.getProcessRecordLocked(processName, uid)); if (leaf == null) { mProcessQueues.put(uid, created); 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 c68db4f673372..74b68e7f3e0fd 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java @@ -371,9 +371,9 @@ public final class BroadcastQueueModernImplTest { List.of(makeMockRegisteredReceiver()), false); queue.enqueueOrReplaceBroadcast(airplaneRecord, 0, false); - queue.setUidFrozen(false); + queue.setProcessAndUidFrozen(null, false); final long notCachedRunnableAt = queue.getRunnableAt(); - queue.setUidFrozen(true); + queue.setProcessAndUidFrozen(null, true); final long cachedRunnableAt = queue.getRunnableAt(); assertThat(cachedRunnableAt).isGreaterThan(notCachedRunnableAt); assertFalse(queue.isRunnable()); @@ -398,9 +398,9 @@ public final class BroadcastQueueModernImplTest { List.of(makeMockRegisteredReceiver()), false); queue.enqueueOrReplaceBroadcast(airplaneRecord, 0, false); - queue.setUidFrozen(false); + queue.setProcessAndUidFrozen(null, false); final long notCachedRunnableAt = queue.getRunnableAt(); - queue.setUidFrozen(true); + queue.setProcessAndUidFrozen(null, true); final long cachedRunnableAt = queue.getRunnableAt(); assertThat(cachedRunnableAt).isGreaterThan(notCachedRunnableAt); assertTrue(queue.isRunnable()); @@ -430,13 +430,13 @@ public final class BroadcastQueueModernImplTest { // verify that: // (a) the queue is immediately runnable by existence of a fg-priority broadcast // (b) the next one up is the fg-priority broadcast despite its later enqueue time - queue.setUidFrozen(false); + queue.setProcessAndUidFrozen(null, false); assertTrue(queue.isRunnable()); assertThat(queue.getRunnableAt()).isAtMost(airplaneRecord.enqueueClockTime); assertEquals(ProcessList.SCHED_GROUP_DEFAULT, queue.getPreferredSchedulingGroupLocked()); assertEquals(queue.peekNextBroadcastRecord(), airplaneRecord); - queue.setUidFrozen(true); + queue.setProcessAndUidFrozen(null, true); assertTrue(queue.isRunnable()); assertThat(queue.getRunnableAt()).isAtMost(airplaneRecord.enqueueClockTime); assertEquals(ProcessList.SCHED_GROUP_DEFAULT, queue.getPreferredSchedulingGroupLocked()); @@ -499,7 +499,7 @@ public final class BroadcastQueueModernImplTest { private void doRunnableAt_Frozen(BroadcastRecord testRecord, int testRunnableAtReason) { final BroadcastProcessQueue queue = new BroadcastProcessQueue(mConstants, PACKAGE_GREEN, getUidForPackage(PACKAGE_GREEN)); - queue.setUidFrozen(true); + queue.setProcessAndUidFrozen(null, true); final BroadcastRecord lazyRecord = makeBroadcastRecord( new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED),