Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3b8c41ab authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Reland: More robust updating of "runnable" list.

Whenever BroadcastProcessQueue.getRunnableAt() changes, we need to
check its position in the "runnable" list to ensure it stays sorted.

We've been returning boolean values to indicate when a caller should
invoke updateRunnableList(), so we expand this to more methods that
can have side-effects, and use the "@CheckResult" annotation to
ensure callers don't forget their duty.

It's okay to err on the side of calling updateRunnableList() when
unsure, since it has a fast-path that returns quickly if a queue is
already sorted correctly.

Bug: 274681945
Test: atest FrameworksMockingServicesTests:BroadcastQueueTest
Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest
Test: atest FrameworksMockingServicesTests:BroadcastRecordTest
Change-Id: I877c5f179ba010c53e52d54c75c47583a608ec95
parent d5b0d8c5
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -353,6 +353,15 @@ public class TimeUtils {
        formatDuration(duration, pw, 0);
    }

    /** @hide Just for debugging; not internationalized. */
    public static void formatDuration(long time, long now, StringBuilder sb) {
        if (time == 0) {
            sb.append("--");
            return;
        }
        formatDuration(time-now, sb, 0);
    }

    /** @hide Just for debugging; not internationalized. */
    public static void formatDuration(long time, long now, PrintWriter pw) {
        if (time == 0) {
+141 −28
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static com.android.internal.util.Preconditions.checkState;
import static com.android.server.am.BroadcastRecord.deliveryStateToString;
import static com.android.server.am.BroadcastRecord.isReceiverEquals;

import android.annotation.CheckResult;
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -187,6 +188,12 @@ class BroadcastProcessQueue {
    private @Reason int mRunnableAtReason = REASON_EMPTY;
    private boolean mRunnableAtInvalidated;

    /**
     * Last state applied by {@link #updateDeferredStates}, used to quickly
     * determine if a state transition is occurring.
     */
    private boolean mLastDeferredStates;

    private boolean mUidCached;
    private boolean mProcessInstrumented;
    private boolean mProcessPersistent;
@@ -235,7 +242,15 @@ class BroadcastProcessQueue {
     */
    @Nullable
    public BroadcastRecord enqueueOrReplaceBroadcast(@NonNull BroadcastRecord record,
            int recordIndex, boolean wouldBeSkipped) {
            int recordIndex, boolean wouldBeSkipped,
            @NonNull BroadcastConsumer deferredStatesApplyConsumer) {
        // When updateDeferredStates() has already applied a deferred state to
        // all pending items, apply to this new broadcast too
        if (mLastDeferredStates && record.deferUntilActive
                && (record.getDeliveryState(recordIndex) == BroadcastRecord.DELIVERY_PENDING)) {
            deferredStatesApplyConsumer.accept(record, recordIndex);
        }

        if (record.isReplacePending()) {
            final BroadcastRecord replacedBroadcastRecord = replaceBroadcast(record, recordIndex,
                    wouldBeSkipped);
@@ -340,7 +355,12 @@ class BroadcastProcessQueue {
     * Predicates that choose to remove a broadcast <em>must</em> 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;
@@ -353,6 +373,7 @@ class BroadcastProcessQueue {
        return didSomething;
    }

    @CheckResult
    private boolean forEachMatchingBroadcastInQueue(@NonNull ArrayDeque<SomeArgs> queue,
            @NonNull BroadcastPredicate predicate, @NonNull BroadcastConsumer consumer,
            boolean andRemove) {
@@ -369,6 +390,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;
            }
@@ -380,32 +405,44 @@ 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 setProcessAndUidCached(@Nullable ProcessRecord app, boolean uidCached) {
    @CheckResult
    public boolean setProcessAndUidCached(@Nullable ProcessRecord app, boolean uidCached) {
        this.app = app;
        if (app != null) {
            setUidCached(uidCached);
            setProcessInstrumented(app.getActiveInstrumentation() != null);
            setProcessPersistent(app.isPersistent());
        } else {
            setUidCached(uidCached);
            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 |= setUidCached(uidCached);
            didSomething |= setProcessInstrumented(app.getActiveInstrumentation() != null);
            didSomething |= setProcessPersistent(app.isPersistent());
        } else {
            didSomething |= setUidCached(uidCached);
            didSomething |= setProcessInstrumented(false);
            didSomething |= setProcessPersistent(false);
        }
        return didSomething;
    }

    /**
     * Update if this process is in the "cached" state, typically signaling that
     * broadcast dispatch should be paused or delayed.
     */
    private void setUidCached(boolean uidCached) {
    @CheckResult
    private boolean setUidCached(boolean uidCached) {
        if (mUidCached != uidCached) {
            mUidCached = uidCached;
            invalidateRunnableAt();
            return true;
        } else {
            return false;
        }
    }

@@ -414,10 +451,14 @@ class BroadcastProcessQueue {
     * signaling that broadcast dispatch should bypass all pauses or delays, to
     * avoid holding up test suites.
     */
    private void setProcessInstrumented(boolean instrumented) {
    @CheckResult
    private boolean setProcessInstrumented(boolean instrumented) {
        if (mProcessInstrumented != instrumented) {
            mProcessInstrumented = instrumented;
            invalidateRunnableAt();
            return true;
        } else {
            return false;
        }
    }

@@ -425,10 +466,14 @@ 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.
     */
    private void setProcessPersistent(boolean persistent) {
    @CheckResult
    private boolean setProcessPersistent(boolean persistent) {
        if (mProcessPersistent != persistent) {
            mProcessPersistent = persistent;
            invalidateRunnableAt();
            return true;
        } else {
            return false;
        }
    }

@@ -648,8 +693,20 @@ class BroadcastProcessQueue {
        return mActive != null;
    }

    void forceDelayBroadcastDelivery(long 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;
        }
    }

    /**
@@ -721,10 +778,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) {
    boolean setPrioritizeEarliest(boolean prioritizeEarliest) {
        if (mPrioritizeEarliest != prioritizeEarliest) {
            mPrioritizeEarliest = prioritizeEarliest;
            invalidateRunnableAt();
            return true;
        } else {
            return false;
        }
    }

    /**
@@ -912,9 +980,9 @@ class BroadcastProcessQueue {
    }

    /**
     * Update {@link #getRunnableAt()} if it's currently invalidated.
     * Update {@link #getRunnableAt()}, when needed.
     */
    private void updateRunnableAt() {
    void updateRunnableAt() {
        if (!mRunnableAtInvalidated) return;
        mRunnableAtInvalidated = false;

@@ -1026,11 +1094,45 @@ class BroadcastProcessQueue {
        }
    }

    /**
     * Update {@link BroadcastRecord.DELIVERY_DEFERRED} states of all our
     * pending broadcasts, when needed.
     */
    void updateDeferredStates(@NonNull BroadcastConsumer applyConsumer,
            @NonNull BroadcastConsumer clearConsumer) {
        // When all we have pending is deferred broadcasts, and we're cached,
        // then we want everything to be marked deferred
        final boolean wantDeferredStates = (mCountDeferred > 0)
                && (mCountDeferred == mCountEnqueued) && mUidCached;

        if (mLastDeferredStates != wantDeferredStates) {
            mLastDeferredStates = wantDeferredStates;
            if (wantDeferredStates) {
                forEachMatchingBroadcast((r, i) -> {
                    return r.deferUntilActive
                            && (r.getDeliveryState(i) == BroadcastRecord.DELIVERY_PENDING);
                }, applyConsumer, false);
            } else {
                forEachMatchingBroadcast((r, i) -> {
                    return r.deferUntilActive
                            && (r.getDeliveryState(i) == BroadcastRecord.DELIVERY_DEFERRED);
                }, clearConsumer, false);
            }
        }
    }

    /**
     * Check overall health, confirming things are in a reasonable state and
     * that we're not wedged.
     */
    public void assertHealthLocked() {
        // If we're not actively running, we should be sorted into the runnable
        // list, and if we're invalidated then someone likely forgot to invoke
        // updateRunnableList() to re-sort us into place
        if (!isActive()) {
            checkState(!mRunnableAtInvalidated, "mRunnableAtInvalidated");
        }

        assertHealthLocked(mPending);
        assertHealthLocked(mPendingUrgent);
        assertHealthLocked(mPendingOffload);
@@ -1133,19 +1235,30 @@ class BroadcastProcessQueue {
        return mCachedToShortString;
    }

    public String describeStateLocked() {
        return describeStateLocked(SystemClock.uptimeMillis());
    }

    public String describeStateLocked(@UptimeMillisLong long now) {
        final StringBuilder sb = new StringBuilder();
        if (isRunnable()) {
            sb.append("runnable at ");
            TimeUtils.formatDuration(getRunnableAt(), now, sb);
        } else {
            sb.append("not runnable");
        }
        sb.append(" because ");
        sb.append(reasonToString(mRunnableAtReason));
        return sb.toString();
    }

    @NeverCompile
    public void dumpLocked(@UptimeMillisLong long now, @NonNull IndentingPrintWriter pw) {
        if ((mActive == null) && isEmpty()) return;

        pw.print(toShortString());
        if (isRunnable()) {
            pw.print(" runnable at ");
            TimeUtils.formatDuration(getRunnableAt(), now, pw);
        } else {
            pw.print(" not runnable");
        }
        pw.print(" because ");
        pw.print(reasonToString(mRunnableAtReason));
        pw.print(" ");
        pw.print(describeStateLocked(now));
        pw.println();

        pw.increaseIndent();
+34 −32
Original line number Diff line number Diff line
@@ -327,6 +327,12 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
            return;
        }

        // To place ourselves correctly in the runnable list, we may need to
        // update internals that may have been invalidated; we wait until now at
        // the last possible moment to avoid duplicated work
        queue.updateDeferredStates(mBroadcastConsumerDeferApply, mBroadcastConsumerDeferClear);
        queue.updateRunnableAt();

        final boolean wantQueue = queue.isRunnable();
        final boolean inQueue = (queue == mRunnableHead) || (queue.runnableAtPrev != null)
                || (queue.runnableAtNext != null);
@@ -352,8 +358,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);
        }
    }

@@ -619,14 +623,10 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
            }
            enqueuedBroadcast = true;
            final BroadcastRecord replacedBroadcast = queue.enqueueOrReplaceBroadcast(
                    r, i, wouldBeSkipped);
                    r, i, wouldBeSkipped, mBroadcastConsumerDeferApply);
            if (replacedBroadcast != null) {
                replacedBroadcasts.add(replacedBroadcast);
            }
            if (r.isDeferUntilActive() && queue.isDeferredUntilActive()) {
                setDeliveryState(queue, null, r, i, receiver, BroadcastRecord.DELIVERY_DEFERRED,
                        "deferred at enqueue time");
            }
            updateRunnableList(queue);
            enqueueUpdateRunningList();
        }
@@ -1249,14 +1249,14 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
        r.resultExtras = null;
    };

    private final BroadcastConsumer mBroadcastConsumerDefer = (r, i) -> {
    private final BroadcastConsumer mBroadcastConsumerDeferApply = (r, i) -> {
        setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_DEFERRED,
                "mBroadcastConsumerDefer");
                "mBroadcastConsumerDeferApply");
    };

    private final BroadcastConsumer mBroadcastConsumerUndoDefer = (r, i) -> {
    private final BroadcastConsumer mBroadcastConsumerDeferClear = (r, i) -> {
        setDeliveryState(null, null, r, i, r.receivers.get(i), BroadcastRecord.DELIVERY_PENDING,
                "mBroadcastConsumerUndoDefer");
                "mBroadcastConsumerDeferClear");
    };

    /**
@@ -1272,7 +1272,8 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
                    final long now = SystemClock.uptimeMillis();
                    if (now > mLastTestFailureTime + DateUtils.SECOND_IN_MILLIS) {
                        mLastTestFailureTime = now;
                        pw.println("Test " + label + " failed due to " + leaf.toShortString());
                        pw.println("Test " + label + " failed due to " + leaf.toShortString() + " "
                                + leaf.describeStateLocked());
                        pw.flush();
                    }
                    return false;
@@ -1309,34 +1310,25 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
        return didSomething;
    }

    private void forEachMatchingQueue(
    private boolean forEachMatchingQueue(
            @NonNull Predicate<BroadcastProcessQueue> queuePredicate,
            @NonNull Consumer<BroadcastProcessQueue> 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);
                    updateRunnableList(leaf);
                    didSomething = true;
                }
                leaf = leaf.processNameNext;
            }
        }
        if (didSomething) {
            enqueueUpdateRunningList();
        }

    private void updateQueueDeferred(
            @NonNull BroadcastProcessQueue leaf) {
        if (leaf.isDeferredUntilActive()) {
            leaf.forEachMatchingBroadcast((r, i) -> {
                return r.deferUntilActive && (r.getDeliveryState(i)
                        == BroadcastRecord.DELIVERY_PENDING);
            }, mBroadcastConsumerDefer, false);
        } else if (leaf.hasDeferredBroadcasts()) {
            leaf.forEachMatchingBroadcast((r, i) -> {
                return r.deferUntilActive && (r.getDeliveryState(i)
                        == BroadcastRecord.DELIVERY_DEFERRED);
            }, mBroadcastConsumerUndoDefer, false);
        }
        return didSomething;
    }

    @Override
@@ -1359,8 +1351,6 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
                        // Update internal state by refreshing values previously
                        // read from any known running process
                        setQueueProcess(leaf, leaf.app);
                        updateQueueDeferred(leaf);
                        updateRunnableList(leaf);
                        leaf = leaf.processNameNext;
                    }
                    enqueueUpdateRunningList();
@@ -1529,19 +1519,31 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
        }
    }

    @SuppressWarnings("CheckResult")
    private void updateWarmProcess(@NonNull BroadcastProcessQueue queue) {
        if (!queue.isProcessWarm()) {
            setQueueProcess(queue, mService.getProcessRecordLocked(queue.processName, queue.uid));
            // This is a bit awkward; we're in the middle of traversing the
            // runnable queue, so we can't reorder that list if the runnable
            // time changes here. However, if this process was just found to be
            // warm via this operation, we're going to immediately promote it to
            // be running, and any side effect of this operation will then apply
            // after it's finished and is returned to the runnable list.
            queue.setProcessAndUidCached(
                    mService.getProcessRecordLocked(queue.processName, queue.uid),
                    mUidCached.get(queue.uid, false));
        }
    }

    /**
     * Update the {@link ProcessRecord} associated with the given
     * {@link BroadcastProcessQueue}.
     * {@link BroadcastProcessQueue}. Also updates any runnable status that
     * might have changed as a side-effect.
     */
    private void setQueueProcess(@NonNull BroadcastProcessQueue queue,
            @Nullable ProcessRecord app) {
        queue.setProcessAndUidCached(app, mUidCached.get(queue.uid, false));
        if (queue.setProcessAndUidCached(app, mUidCached.get(queue.uid, false))) {
            updateRunnableList(queue);
        }
    }

    /**
+41 −27

File changed.

Preview size limit exceeded, changes collapsed.