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

Commit b57c8658 authored by Sudheer Shanka's avatar Sudheer Shanka
Browse files

Ignore older UID state change callbacks in NPMS. 

Since the onUidStateChanged() callbacks will be
happening from more than one thread, we need to make
sure we only handle the latest uid state change
callbacks and ignore any older callbacks that could
override the latest uid states in NPMS. In order to
this, every uid state change will be associated with
a unique seq number.

Also, remove the UidRecord.lastDispatchedProcStateSeq.
This was added as a safety measure to make sure we
don't unnecessarily end up waiting but it isn't
necessary and if we need to keep it, we need to add
a way to access it without holding global AMS lock,
which isn't worth the complexity.

Bug: 226299593
Test: atest tests/cts/hostside/src/com/android/cts/net/HostsideRestrictBackgroundNetworkTests.java
Change-Id: I631f5e01f6627916a96c930c22849a1d11eab636
parent dd7c5737

core/java/android/net/NetworkPolicyManager.java

+5 −1
Original line number Diff line number Diff line
@@ -817,11 +817,13 @@ public class NetworkPolicyManager { 
    public static final class UidState {

        public int uid;

        public int procState;

        public long procStateSeq;

        public int capability;



        public UidState(int uid, int procState, int capability) {

        public UidState(int uid, int procState, long procStateSeq, int capability) {

            this.uid = uid;

            this.procState = procState;

            this.procStateSeq = procStateSeq;

            this.capability = capability;

        }
@@ -830,6 +832,8 @@ public class NetworkPolicyManager { 
            final StringBuilder sb = new StringBuilder();

            sb.append("{procState=");

            sb.append(procStateToString(procState));

            sb.append(",seq=");

            sb.append(procStateSeq);

            sb.append(",cap=");

            ActivityManager.printCapabilitiesSummary(sb, capability);

            sb.append("}");

services/core/java/com/android/server/am/ActivityManagerService.java

+8 −31
Original line number Diff line number Diff line
@@ -15257,7 +15257,6 @@ public class ActivityManagerService extends IActivityManager.Stub 
                uid, change, procState, procStateSeq, capability, ephemeral);

        if (uidRec != null) {

            uidRec.setLastReportedChange(enqueuedChange);

            uidRec.updateLastDispatchedProcStateSeq(enqueuedChange);

        }















        // Directly update the power manager, since we sit on top of it and it is critical










@@ -16523,18 +16522,13 @@ public class ActivityManagerService extends IActivityManager.Stub













                    return;















                }















                record.lastNetworkUpdatedProcStateSeq = procStateSeq;













                if (record.curProcStateSeq > procStateSeq) {













                    if (DEBUG_NETWORK) {













                        Slog.d(TAG_NETWORK, "No need to handle older seq no., Uid: " + uid













                                + ", curProcstateSeq: " + record.curProcStateSeq













                                + ", procStateSeq: " + procStateSeq);













                    }













                    return;













                }













                if (record.waitingForNetwork) {













                if (record.procStateSeqWaitingForNetwork != 0













                        && procStateSeq >= record.procStateSeqWaitingForNetwork) {















                    if (DEBUG_NETWORK) {















                        Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid













                                + ", procStateSeq: " + procStateSeq);













                                + ", procStateSeq: " + procStateSeq













                                + ", procStateSeqWaitingForNetwork: "













                                + record.procStateSeqWaitingForNetwork);















                    }















                    record.networkStateLock.notifyAll();















                }










@@ -17279,7 +17273,7 @@ public class ActivityManagerService extends IActivityManager.Stub













            if (mNetworkPolicyUidObserver != null) {















                try {















                    mNetworkPolicyUidObserver.onUidStateChanged(uid, PROCESS_STATE_TOP,













                            mProcessList.getProcStateSeqCounter(), PROCESS_CAPABILITY_ALL);













                            mProcessList.getNextProcStateSeq(), PROCESS_CAPABILITY_ALL);















                } catch (RemoteException e) {















                    // Should not happen; call is within the same process















                }










@@ -17568,23 +17562,6 @@ public class ActivityManagerService extends IActivityManager.Stub













            }















        }















        synchronized (record.networkStateLock) {













            if (record.lastDispatchedProcStateSeq < procStateSeq) {













                if (DEBUG_NETWORK) {













                    Slog.d(TAG_NETWORK, "Uid state change for seq no. " + procStateSeq + " is not "













                            + "dispatched to NPMS yet, so don't wait. Uid: " + callingUid













                            + " lastProcStateSeqDispatchedToObservers: "













                            + record.lastDispatchedProcStateSeq);













                }













                return;













            }













            if (record.curProcStateSeq > procStateSeq) {













                if (DEBUG_NETWORK) {













                    Slog.d(TAG_NETWORK, "Ignore the wait requests for older seq numbers. Uid: "













                            + callingUid + ", curProcStateSeq: " + record.curProcStateSeq













                            + ", procStateSeq: " + procStateSeq);













                }













                return;













            }















            if (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) {















                if (DEBUG_NETWORK) {















                    Slog.d(TAG_NETWORK, "Network rules have been already updated for seq no. "









@@ -17600,9 +17577,9 @@ public class ActivityManagerService extends IActivityManager.Stub













                        + " Uid: " + callingUid + " procStateSeq: " + procStateSeq);















                }















                final long startTime = SystemClock.uptimeMillis();













                record.waitingForNetwork = true;













                record.procStateSeqWaitingForNetwork = procStateSeq;















                record.networkStateLock.wait(mWaitForNetworkTimeoutMs);













                record.waitingForNetwork = false;













                record.procStateSeqWaitingForNetwork = 0;















                final long totalTime = SystemClock.uptimeMillis() - startTime;















                if (totalTime >= mWaitForNetworkTimeoutMs || DEBUG_NETWORK) {















                    Slog.w(TAG_NETWORK, "Total time waited for network rules to get updated: "

























services/core/java/com/android/server/am/ProcessList.java



+10
−7




























Original line number
Diff line number
Diff line








@@ -4781,13 +4781,17 @@ public final class ProcessList {













    }































    /**













     * Checks if any uid is coming from background to foreground or vice versa and if so, increments













     * the {@link UidRecord#curProcStateSeq} corresponding to that uid using global seq counter













     * {@link ProcessList#mProcStateSeqCounter} and notifies the app if it needs to block.













     * Increments the {@link UidRecord#curProcStateSeq} for all uids using global seq counter













     * {@link ProcessList#mProcStateSeqCounter} and checks if any uid is coming













     * from background to foreground or vice versa and if so, notifies the app if it needs to block.















     */















    @VisibleForTesting















    @GuardedBy(anyOf = {"mService", "mProcLock"})















    void incrementProcStateSeqAndNotifyAppsLOSP(ActiveUids activeUids) {













        for (int i = activeUids.size() - 1; i >= 0; --i) {













            final UidRecord uidRec = activeUids.valueAt(i);













            uidRec.curProcStateSeq = getNextProcStateSeq();













        }















        if (mService.mWaitForNetworkTimeoutMs <= 0) {















            return;















        }









@@ -4814,7 +4818,6 @@ public final class ProcessList {













                continue;















            }















            synchronized (uidRec.networkStateLock) {













                uidRec.curProcStateSeq = ++mProcStateSeqCounter; // TODO: use method















                if (blockState == NETWORK_STATE_BLOCK) {















                    if (blockingUids == null) {















                        blockingUids = new ArrayList<>();









@@ -4825,7 +4828,7 @@ public final class ProcessList {













                        Slog.d(TAG_NETWORK, "uid going to background, notifying all blocking"















                                + " threads for uid: " + uidRec);















                    }













                    if (uidRec.waitingForNetwork) {













                    if (uidRec.procStateSeqWaitingForNetwork != 0) {















                        uidRec.networkStateLock.notifyAll();















                    }















                }










@@ -4859,8 +4862,8 @@ public final class ProcessList {













        }















    }





























    long getProcStateSeqCounter() {













        return mProcStateSeqCounter;













    long getNextProcStateSeq() {













        return ++mProcStateSeqCounter;















    }































    /**

































services/core/java/com/android/server/am/UidObserverController.java



+0
−1




























Original line number
Diff line number
Diff line








@@ -229,7 +229,6 @@ public class UidObserverController {













                    validateUid.setCurProcState(item.procState);















                    validateUid.setSetCapability(item.capability);















                    validateUid.setCurCapability(item.capability);













                    validateUid.lastDispatchedProcStateSeq = item.procStateSeq;















                }















            }















        }

































services/core/java/com/android/server/am/UidRecord.java



+2
−24




























Original line number
Diff line number
Diff line








@@ -99,16 +99,9 @@ public final class UidRecord {













    long lastNetworkUpdatedProcStateSeq;































    /**













     * Last seq number for which AcitivityManagerService dispatched uid state change to













     * NetworkPolicyManagerService.













     * Indicates if any thread is waiting for network rules to get updated for {@link #mUid}.















     */













    @GuardedBy("networkStateUpdate")













    long lastDispatchedProcStateSeq;



























    /**













     * Indicates if any thread is waiting for network rules to get updated for {@link #uid}.













     */













    volatile boolean waitingForNetwork;













    volatile long procStateSeqWaitingForNetwork;































    /**















     * Indicates whether this uid has internet permission or not.










@@ -345,18 +338,6 @@ public final class UidRecord {













                mUid) == PackageManager.PERMISSION_GRANTED;















    }





























    /**













     * If the change being dispatched is not CHANGE_GONE (not interested in













     * these changes), then update the {@link #lastDispatchedProcStateSeq} with













     * {@link #curProcStateSeq}.













     */













    public void updateLastDispatchedProcStateSeq(int changeToDispatch) {













        if ((changeToDispatch & CHANGE_GONE) == 0) {













            lastDispatchedProcStateSeq = curProcStateSeq;













        }













    }











































    void dumpDebug(ProtoOutputStream proto, long fieldId) {















        long token = proto.start(fieldId);















        proto.write(UidRecordProto.UID, mUid);









@@ -377,7 +358,6 @@ public final class UidRecord {













        proto.write(UidRecordProto.ProcStateSequence.CURURENT, curProcStateSeq);















        proto.write(UidRecordProto.ProcStateSequence.LAST_NETWORK_UPDATED,















                lastNetworkUpdatedProcStateSeq);













        proto.write(UidRecordProto.ProcStateSequence.LAST_DISPATCHED, lastDispatchedProcStateSeq);















        proto.end(seqToken);































        proto.end(token);










@@ -460,8 +440,6 @@ public final class UidRecord {













        sb.append(curProcStateSeq);















        sb.append(",");















        sb.append(lastNetworkUpdatedProcStateSeq);













        sb.append(",");













        sb.append(lastDispatchedProcStateSeq);















        sb.append(")}");















        return sb.toString();















    }