diff options
4 files changed, 77 insertions, 12 deletions
diff --git a/core/java/android/net/vcn/flags.aconfig b/core/java/android/net/vcn/flags.aconfig index 5b306243fc36..3d4401b1c71c 100644 --- a/core/java/android/net/vcn/flags.aconfig +++ b/core/java/android/net/vcn/flags.aconfig @@ -14,4 +14,14 @@ flag { namespace: "vcn" description: "Feature flag for adjustable safe mode timeout" bug: "317406085" +} + +flag { + name: "fix_config_garbage_collection" + namespace: "vcn" + description: "Handle race condition in subscription change" + bug: "370862489" + metadata { + purpose: PURPOSE_BUGFIX + } }
\ No newline at end of file diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index 51c768b80eff..dc7749cb02ef 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -48,6 +48,7 @@ import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkRequest; +import android.net.vcn.Flags; import android.net.vcn.IVcnManagementService; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; @@ -878,6 +879,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { private void garbageCollectAndWriteVcnConfigsLocked() { final SubscriptionManager subMgr = mContext.getSystemService(SubscriptionManager.class); + final Set<ParcelUuid> subGroups = mLastSnapshot.getAllSubscriptionGroups(); boolean shouldWrite = false; @@ -885,11 +887,20 @@ public class VcnManagementService extends IVcnManagementService.Stub { while (configsIterator.hasNext()) { final ParcelUuid subGrp = configsIterator.next(); - final List<SubscriptionInfo> subscriptions = subMgr.getSubscriptionsInGroup(subGrp); - if (subscriptions == null || subscriptions.isEmpty()) { - // Trim subGrps with no more subscriptions; must have moved to another subGrp - configsIterator.remove(); - shouldWrite = true; + if (Flags.fixConfigGarbageCollection()) { + if (!subGroups.contains(subGrp)) { + // Trim subGrps with no more subscriptions; must have moved to another subGrp + logDbg("Garbage collect VcnConfig for group=" + subGrp); + configsIterator.remove(); + shouldWrite = true; + } + } else { + final List<SubscriptionInfo> subscriptions = subMgr.getSubscriptionsInGroup(subGrp); + if (subscriptions == null || subscriptions.isEmpty()) { + // Trim subGrps with no more subscriptions; must have moved to another subGrp + configsIterator.remove(); + shouldWrite = true; + } } } @@ -1094,13 +1105,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { synchronized (mLock) { final Vcn vcn = mVcns.get(subGrp); final VcnConfig vcnConfig = mConfigs.get(subGrp); - if (vcn != null) { - if (vcnConfig == null) { - // TODO: b/284381334 Investigate for the root cause of this issue - // and handle it properly - logWtf("Vcn instance exists but VcnConfig does not for " + subGrp); - } - + if (vcn != null && vcnConfig != null) { if (vcn.getStatus() == VCN_STATUS_CODE_ACTIVE) { isVcnManagedNetwork = true; } @@ -1120,6 +1125,8 @@ public class VcnManagementService extends IVcnManagementService.Stub { } } } + } else if (vcn != null && vcnConfig == null) { + logWtf("Vcn instance exists but VcnConfig does not for " + subGrp); } } diff --git a/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java b/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java index baf84cf4aa8b..3392d039109a 100644 --- a/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java +++ b/services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java @@ -436,6 +436,17 @@ public class TelephonySubscriptionTracker extends BroadcastReceiver { return mPrivilegedPackages.keySet(); } + /** Returns all subscription groups */ + @NonNull + public Set<ParcelUuid> getAllSubscriptionGroups() { + final Set<ParcelUuid> subGroups = new ArraySet<>(); + for (SubscriptionInfo subInfo : mSubIdToInfoMap.values()) { + subGroups.add(subInfo.getGroupUuid()); + } + + return subGroups; + } + /** Checks if the provided package is carrier privileged for the specified sub group. */ public boolean packageHasPermissionsForSubscriptionGroup( @NonNull ParcelUuid subGrp, @NonNull String packageName) { diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index 7e0bbc4b3e50..3828a71d7b28 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -70,6 +70,7 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkRequest; import android.net.Uri; +import android.net.vcn.Flags; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; import android.net.vcn.VcnConfig; @@ -84,6 +85,7 @@ import android.os.Process; import android.os.UserHandle; import android.os.UserManager; import android.os.test.TestLooper; +import android.platform.test.flag.junit.SetFlagsRule; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; @@ -102,6 +104,7 @@ import com.android.server.vcn.util.PersistableBundleUtils; import com.android.server.vcn.util.PersistableBundleUtils.PersistableBundleWrapper; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -119,6 +122,8 @@ import java.util.UUID; @RunWith(AndroidJUnit4.class) @SmallTest public class VcnManagementServiceTest { + @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + private static final String CONTEXT_ATTRIBUTION_TAG = "VCN"; private static final String TEST_PACKAGE_NAME = VcnManagementServiceTest.class.getPackage().getName(); @@ -288,6 +293,8 @@ public class VcnManagementServiceTest { doReturn(Collections.singleton(TRANSPORT_WIFI)) .when(mMockDeps) .getRestrictedTransports(any(), any(), any()); + + mSetFlagsRule.enableFlags(Flags.FLAG_FIX_CONFIG_GARBAGE_COLLECTION); } @@ -438,6 +445,14 @@ public class VcnManagementServiceTest { return subIds; }).when(snapshot).getAllSubIdsInGroup(any()); + doAnswer(invocation -> { + final Set<ParcelUuid> subGroups = new ArraySet<>(); + for (Entry<Integer, ParcelUuid> entry : subIdToGroupMap.entrySet()) { + subGroups.add(entry.getValue()); + } + return subGroups; + }).when(snapshot).getAllSubscriptionGroups(); + return snapshot; } @@ -1483,6 +1498,28 @@ public class VcnManagementServiceTest { } @Test + public void testGarbageCollectionKeepConfigUntilNewSnapshot() throws Exception { + setupActiveSubscription(TEST_UUID_2); + startAndGetVcnInstance(TEST_UUID_2); + + // Report loss of subscription from mSubMgr + doReturn(Collections.emptyList()).when(mSubMgr).getSubscriptionsInGroup(any()); + triggerSubscriptionTrackerCbAndGetSnapshot( + TEST_UUID_2, + Collections.singleton(TEST_UUID_2), + Collections.singletonMap(TEST_SUBSCRIPTION_ID, TEST_UUID_2)); + + assertTrue(mVcnMgmtSvc.getConfigs().containsKey(TEST_UUID_2)); + + // Report loss of subscription from snapshot + triggerSubscriptionTrackerCbAndGetSnapshot(null, Collections.emptySet()); + + mTestLooper.moveTimeForward(VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS); + mTestLooper.dispatchAll(); + assertFalse(mVcnMgmtSvc.getConfigs().containsKey(TEST_UUID_2)); + } + + @Test public void testVcnCarrierConfigChangeUpdatesPolicyListener() throws Exception { setupActiveSubscription(TEST_UUID_2); |