joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix VcnConfig garbage collection

Browse source 
The garbageCollectAndWriteVcnConfigsLocked is for removing
configs whose subgroups do not have any subscriptions. This
patch fixes the issue of it in handling race condition when
subscription changes.

Previously the garbage collection checks a subGroup's
subscriptions based on SubscriptionManager. When
SubscriptionManager and TelephonySubscriptionSnapshot are
out-of-synced, the config of an active VCN instance will be
removed. It can cause NPE in getUnderlyingNetworkPolicy. This
patch fixes it by checking subscriptions against the snapshot.

This patch also fixes the safety check in
getUnderlyingNetworkPolicy so that if VCN instance exists but
its config does not, it will not attempt querying data from
the VcnConfig

Bug: 370862489
Test: FrameworksVcnTests(new tests); CtsVcnTestCases
Flag: android.net.vcn.fix_config_garbage_collection
Change-Id: Id54adabcc1910102a2ab20c432b693a8f52c5c95
This commit is contained in:
Yan Yan 2024-10-28 13:43:34 -07:00
commit a2284ca391
4 changed files with 77 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

10
Vcn/flags/flags.aconfig
View file

@ -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
}
}

31
Vcn/service-b/src/com/android/server/VcnManagementService.java
View file

@ -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);
}
}

11
Vcn/service-b/src/com/android/server/vcn/TelephonySubscriptionTracker.java
View file

@ -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) {

37
Vcn/tests/unit/java/com/android/server/VcnManagementServiceTest.java
View file

@ -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;
}
@ -1482,6 +1497,28 @@ public class VcnManagementServiceTest {
verify(mMockPolicyListener).onPolicyChanged();
}
@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);