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

Commit 67a66f47 authored by Yan Yan's avatar Yan Yan
Browse files

Fix VcnConfig garbage collection 

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
parent edd8447b

core/java/android/net/vcn/flags.aconfig

+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

services/core/java/com/android/server/VcnManagementService.java

+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);

                }

            }

services/core/java/com/android/server/vcn/TelephonySubscriptionTracker.java

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

tests/vcn/java/com/android/server/VcnManagementServiceTest.java

+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);