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

Commit efa4f742 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
Merged-In: I631f5e01f6627916a96c930c22849a1d11eab636
parent 3d9f5e63
Loading
Loading
Loading
Loading
+5 −1
Original line number Original line Diff line number Diff line
@@ -817,11 +817,13 @@ public class NetworkPolicyManager {
    public static final class UidState {
    public static final class UidState {
        public int uid;
        public int uid;
        public int procState;
        public int procState;
        public long procStateSeq;
        public int capability;
        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.uid = uid;
            this.procState = procState;
            this.procState = procState;
            this.procStateSeq = procStateSeq;
            this.capability = capability;
            this.capability = capability;
        }
        }


@@ -830,6 +832,8 @@ public class NetworkPolicyManager {
            final StringBuilder sb = new StringBuilder();
            final StringBuilder sb = new StringBuilder();
            sb.append("{procState=");
            sb.append("{procState=");
            sb.append(procStateToString(procState));
            sb.append(procStateToString(procState));
            sb.append(",seq=");
            sb.append(procStateSeq);
            sb.append(",cap=");
            sb.append(",cap=");
            ActivityManager.printCapabilitiesSummary(sb, capability);
            ActivityManager.printCapabilitiesSummary(sb, capability);
            sb.append("}");
            sb.append("}");
+8 −31
Original line number Original line Diff line number Diff line
@@ -14449,7 +14449,6 @@ public class ActivityManagerService extends IActivityManager.Stub
                uid, change, procState, procStateSeq, capability, ephemeral);
                uid, change, procState, procStateSeq, capability, ephemeral);
        if (uidRec != null) {
        if (uidRec != null) {
            uidRec.setLastReportedChange(enqueuedChange);
            uidRec.setLastReportedChange(enqueuedChange);
            uidRec.updateLastDispatchedProcStateSeq(enqueuedChange);
        }
        }
        // Directly update the power manager, since we sit on top of it and it is critical
        // Directly update the power manager, since we sit on top of it and it is critical
@@ -15587,18 +15586,13 @@ public class ActivityManagerService extends IActivityManager.Stub
                    return;
                    return;
                }
                }
                record.lastNetworkUpdatedProcStateSeq = procStateSeq;
                record.lastNetworkUpdatedProcStateSeq = procStateSeq;
                if (record.curProcStateSeq > procStateSeq) {
                if (record.procStateSeqWaitingForNetwork != 0
                    if (DEBUG_NETWORK) {
                        && procStateSeq >= record.procStateSeqWaitingForNetwork) {
                        Slog.d(TAG_NETWORK, "No need to handle older seq no., Uid: " + uid
                                + ", curProcstateSeq: " + record.curProcStateSeq
                                + ", procStateSeq: " + procStateSeq);
                    }
                    return;
                }
                if (record.waitingForNetwork) {
                    if (DEBUG_NETWORK) {
                    if (DEBUG_NETWORK) {
                        Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid
                        Slog.d(TAG_NETWORK, "Notifying all blocking threads for uid: " + uid
                                + ", procStateSeq: " + procStateSeq);
                                + ", procStateSeq: " + procStateSeq
                                + ", procStateSeqWaitingForNetwork: "
                                + record.procStateSeqWaitingForNetwork);
                    }
                    }
                    record.networkStateLock.notifyAll();
                    record.networkStateLock.notifyAll();
                }
                }
@@ -16313,7 +16307,7 @@ public class ActivityManagerService extends IActivityManager.Stub
            if (mNetworkPolicyUidObserver != null) {
            if (mNetworkPolicyUidObserver != null) {
                try {
                try {
                    mNetworkPolicyUidObserver.onUidStateChanged(uid, PROCESS_STATE_TOP,
                    mNetworkPolicyUidObserver.onUidStateChanged(uid, PROCESS_STATE_TOP,
                            mProcessList.getProcStateSeqCounter(), PROCESS_CAPABILITY_ALL);
                            mProcessList.getNextProcStateSeq(), PROCESS_CAPABILITY_ALL);
                } catch (RemoteException e) {
                } catch (RemoteException e) {
                    // Should not happen; call is within the same process
                    // Should not happen; call is within the same process
                }
                }
@@ -16551,23 +16545,6 @@ public class ActivityManagerService extends IActivityManager.Stub
            }
            }
        }
        }
        synchronized (record.networkStateLock) {
        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 (record.lastNetworkUpdatedProcStateSeq >= procStateSeq) {
                if (DEBUG_NETWORK) {
                if (DEBUG_NETWORK) {
                    Slog.d(TAG_NETWORK, "Network rules have been already updated for seq no. "
                    Slog.d(TAG_NETWORK, "Network rules have been already updated for seq no. "
@@ -16583,9 +16560,9 @@ public class ActivityManagerService extends IActivityManager.Stub
                        + " Uid: " + callingUid + " procStateSeq: " + procStateSeq);
                        + " Uid: " + callingUid + " procStateSeq: " + procStateSeq);
                }
                }
                final long startTime = SystemClock.uptimeMillis();
                final long startTime = SystemClock.uptimeMillis();
                record.waitingForNetwork = true;
                record.procStateSeqWaitingForNetwork = procStateSeq;
                record.networkStateLock.wait(mWaitForNetworkTimeoutMs);
                record.networkStateLock.wait(mWaitForNetworkTimeoutMs);
                record.waitingForNetwork = false;
                record.procStateSeqWaitingForNetwork = 0;
                final long totalTime = SystemClock.uptimeMillis() - startTime;
                final long totalTime = SystemClock.uptimeMillis() - startTime;
                if (totalTime >= mWaitForNetworkTimeoutMs || DEBUG_NETWORK) {
                if (totalTime >= mWaitForNetworkTimeoutMs || DEBUG_NETWORK) {
                    Slog.w(TAG_NETWORK, "Total time waited for network rules to get updated: "
                    Slog.w(TAG_NETWORK, "Total time waited for network rules to get updated: "
+10 −7
Original line number Original line Diff line number Diff line
@@ -4877,13 +4877,17 @@ public final class ProcessList {
    }
    }


    /**
    /**
     * Checks if any uid is coming from background to foreground or vice versa and if so, increments
     * Increments the {@link UidRecord#curProcStateSeq} for all uids using global seq counter
     * the {@link UidRecord#curProcStateSeq} corresponding to that uid using global seq counter
     * {@link ProcessList#mProcStateSeqCounter} and checks if any uid is coming
     * {@link ProcessList#mProcStateSeqCounter} and notifies the app if it needs to block.
     * from background to foreground or vice versa and if so, notifies the app if it needs to block.
     */
     */
    @VisibleForTesting
    @VisibleForTesting
    @GuardedBy(anyOf = {"mService", "mProcLock"})
    @GuardedBy(anyOf = {"mService", "mProcLock"})
    void incrementProcStateSeqAndNotifyAppsLOSP(ActiveUids activeUids) {
    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) {
        if (mService.mWaitForNetworkTimeoutMs <= 0) {
            return;
            return;
        }
        }
@@ -4910,7 +4914,6 @@ public final class ProcessList {
                continue;
                continue;
            }
            }
            synchronized (uidRec.networkStateLock) {
            synchronized (uidRec.networkStateLock) {
                uidRec.curProcStateSeq = ++mProcStateSeqCounter; // TODO: use method
                if (blockState == NETWORK_STATE_BLOCK) {
                if (blockState == NETWORK_STATE_BLOCK) {
                    if (blockingUids == null) {
                    if (blockingUids == null) {
                        blockingUids = new ArrayList<>();
                        blockingUids = new ArrayList<>();
@@ -4921,7 +4924,7 @@ public final class ProcessList {
                        Slog.d(TAG_NETWORK, "uid going to background, notifying all blocking"
                        Slog.d(TAG_NETWORK, "uid going to background, notifying all blocking"
                                + " threads for uid: " + uidRec);
                                + " threads for uid: " + uidRec);
                    }
                    }
                    if (uidRec.waitingForNetwork) {
                    if (uidRec.procStateSeqWaitingForNetwork != 0) {
                        uidRec.networkStateLock.notifyAll();
                        uidRec.networkStateLock.notifyAll();
                    }
                    }
                }
                }
@@ -4955,8 +4958,8 @@ public final class ProcessList {
        }
        }
    }
    }


    long getProcStateSeqCounter() {
    long getNextProcStateSeq() {
        return mProcStateSeqCounter;
        return ++mProcStateSeqCounter;
    }
    }


    /**
    /**
+0 −1
Original line number Original line Diff line number Diff line
@@ -219,7 +219,6 @@ public class UidObserverController {
                    validateUid.setCurProcState(item.procState);
                    validateUid.setCurProcState(item.procState);
                    validateUid.setSetCapability(item.capability);
                    validateUid.setSetCapability(item.capability);
                    validateUid.setCurCapability(item.capability);
                    validateUid.setCurCapability(item.capability);
                    validateUid.lastDispatchedProcStateSeq = item.procStateSeq;
                }
                }
            }
            }
        }
        }
+2 −24
Original line number Original line Diff line number Diff line
@@ -95,16 +95,9 @@ public final class UidRecord {
    long lastNetworkUpdatedProcStateSeq;
    long lastNetworkUpdatedProcStateSeq;


    /**
    /**
     * Last seq number for which AcitivityManagerService dispatched uid state change to
     * Indicates if any thread is waiting for network rules to get updated for {@link #mUid}.
     * NetworkPolicyManagerService.
     */
     */
    @GuardedBy("networkStateUpdate")
    volatile long procStateSeqWaitingForNetwork;
    long lastDispatchedProcStateSeq;

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


    /**
    /**
     * Indicates whether this uid has internet permission or not.
     * Indicates whether this uid has internet permission or not.
@@ -309,18 +302,6 @@ public final class UidRecord {
                mUid) == PackageManager.PERMISSION_GRANTED;
                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) {
    void dumpDebug(ProtoOutputStream proto, long fieldId) {
        long token = proto.start(fieldId);
        long token = proto.start(fieldId);
        proto.write(UidRecordProto.UID, mUid);
        proto.write(UidRecordProto.UID, mUid);
@@ -341,7 +322,6 @@ public final class UidRecord {
        proto.write(UidRecordProto.ProcStateSequence.CURURENT, curProcStateSeq);
        proto.write(UidRecordProto.ProcStateSequence.CURURENT, curProcStateSeq);
        proto.write(UidRecordProto.ProcStateSequence.LAST_NETWORK_UPDATED,
        proto.write(UidRecordProto.ProcStateSequence.LAST_NETWORK_UPDATED,
                lastNetworkUpdatedProcStateSeq);
                lastNetworkUpdatedProcStateSeq);
        proto.write(UidRecordProto.ProcStateSequence.LAST_DISPATCHED, lastDispatchedProcStateSeq);
        proto.end(seqToken);
        proto.end(seqToken);


        proto.end(token);
        proto.end(token);
@@ -412,8 +392,6 @@ public final class UidRecord {
        sb.append(curProcStateSeq);
        sb.append(curProcStateSeq);
        sb.append(",");
        sb.append(",");
        sb.append(lastNetworkUpdatedProcStateSeq);
        sb.append(lastNetworkUpdatedProcStateSeq);
        sb.append(",");
        sb.append(lastDispatchedProcStateSeq);
        sb.append(")}");
        sb.append(")}");
        return sb.toString();
        return sb.toString();
    }
    }
Loading