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

Commit 7b4603df authored by Yan Yan's avatar Yan Yan Committed by Gerrit Code Review
Browse files

Merge "Fix VcnConfig garbage collection" into main

parents 227e7290 67a66f47
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -15,3 +15,13 @@ flag {
    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
+19 −12
Original line number Diff line number Diff line
@@ -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,6 +887,14 @@ public class VcnManagementService extends IVcnManagementService.Stub {
        while (configsIterator.hasNext()) {
            final ParcelUuid subGrp = configsIterator.next();

            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
@@ -892,6 +902,7 @@ public class VcnManagementService extends IVcnManagementService.Stub {
                    shouldWrite = true;
                }
            }
        }

        if (shouldWrite) {
            writeConfigsToDiskLocked();
@@ -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 −0
Original line number Diff line number Diff line
@@ -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 −0
Original line number Diff line number Diff line
@@ -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);