Merge "Remove UidStateCallbackInfo when uid is gone" into main
diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index c11356b..2fceb5b 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -728,7 +728,7 @@
* Map of uid -> UidStateCallbackInfo objects holding the data received from
* {@link IUidObserver#onUidStateChanged(int, int, long, int)} callbacks. In order to avoid
* creating a new object for every callback received, we hold onto the object created for each
- * uid and reuse it.
+ * uid and reuse it until the uid stays alive.
*
* Note that the lock used for accessing this object should not be used for anything else and we
* should not be acquiring new locks or doing any heavy work while this lock is held since this
@@ -801,6 +801,13 @@
Clock.systemUTC());
}
+ @VisibleForTesting
+ UidState getUidStateForTest(int uid) {
+ synchronized (mUidRulesFirstLock) {
+ return mUidState.get(uid);
+ }
+ }
+
static class Dependencies {
final Context mContext;
final NetworkStatsManager mNetworkStatsManager;
@@ -1256,6 +1263,10 @@
}
@Override public void onUidGone(int uid, boolean disabled) {
+ synchronized (mUidStateCallbackInfos) {
+ mUidStateCallbackInfos.remove(uid);
+ }
+ // TODO: b/327058756 - Remove any pending UID_MSG_STATE_CHANGED on the handler.
mUidEventHandler.obtainMessage(UID_MSG_GONE, uid, 0).sendToTarget();
}
};
@@ -5899,7 +5910,7 @@
void handleUidGone(int uid) {
Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "onUidGone");
try {
- boolean updated;
+ final boolean updated;
synchronized (mUidRulesFirstLock) {
updated = removeUidStateUL(uid);
}
diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
index a529382..b1e62f9 100644
--- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
@@ -149,6 +149,7 @@
import android.net.Network;
import android.net.NetworkCapabilities;
import android.net.NetworkPolicy;
+import android.net.NetworkPolicyManager;
import android.net.NetworkStateSnapshot;
import android.net.NetworkTemplate;
import android.net.TelephonyNetworkSpecifier;
@@ -1620,6 +1621,12 @@
verify(mActivityManagerInternal).notifyNetworkPolicyRulesUpdated(UID_A, procStateSeq);
}
+ private void callAndWaitOnUidGone(int uid) throws Exception {
+ // The disabled argument is used only for ephemeral apps and does not matter here.
+ mUidObserver.onUidGone(uid, false /* disabled */);
+ waitForUidEventHandlerIdle();
+ }
+
private void callAndWaitOnUidStateChanged(int uid, int procState, long procStateSeq)
throws Exception {
callAndWaitOnUidStateChanged(uid, procState, procStateSeq,
@@ -2250,6 +2257,15 @@
assertTrue(mService.isUidNetworkingBlocked(UID_A, false));
}
+ private boolean isUidState(int uid, int procState, int procStateSeq, int capability) {
+ final NetworkPolicyManager.UidState uidState = mService.getUidStateForTest(uid);
+ if (uidState == null) {
+ return false;
+ }
+ return uidState.uid == uid && uidState.procStateSeq == procStateSeq
+ && uidState.procState == procState && uidState.capability == capability;
+ }
+
@Ignore("Temporarily disabled until the feature is enabled")
@Test
@RequiresFlagsEnabled(Flags.FLAG_NETWORK_BLOCKED_FOR_TOP_SLEEPING_AND_ABOVE)
@@ -2371,6 +2387,31 @@
}
@Test
+ public void testUidStateChangeAfterUidGone() throws Exception {
+ final int testProcStateSeq = 51;
+ final int testProcState = PROCESS_STATE_IMPORTANT_FOREGROUND;
+
+ try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
+ // First callback for uid.
+ callOnUidStatechanged(UID_B, testProcState, testProcStateSeq, PROCESS_CAPABILITY_NONE);
+ assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
+ }
+ waitForUidEventHandlerIdle();
+ assertTrue(isUidState(UID_B, testProcState, testProcStateSeq, PROCESS_CAPABILITY_NONE));
+
+ callAndWaitOnUidGone(UID_B);
+ try (SyncBarrier b = new SyncBarrier(mService.mUidEventHandler)) {
+ // Even though the procState is the same, the uid had exited - so this should be
+ // processed as a fresh callback.
+ callOnUidStatechanged(UID_B, testProcState, testProcStateSeq + 1,
+ PROCESS_CAPABILITY_NONE);
+ assertTrue(mService.mUidEventHandler.hasMessages(UID_MSG_STATE_CHANGED));
+ }
+ waitForUidEventHandlerIdle();
+ assertTrue(isUidState(UID_B, testProcState, testProcStateSeq + 1, PROCESS_CAPABILITY_NONE));
+ }
+
+ @Test
public void testLowPowerStandbyAllowlist() throws Exception {
// Chain background is also enabled but these procstates are important enough to be exempt.
callAndWaitOnUidStateChanged(UID_A, PROCESS_STATE_TOP, 0);