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

Commit bf15dd96 authored by Jack Yu's avatar Jack Yu
Browse files

Fixed the race condition for data unthrottling 

The retry happened before data throttler has a chance
to unthrottle the APN. Fixed by listening the throttle
status changed event from the throttler, and then retry.

Fix: 196756946
Test: Manual
Change-Id: Ia107f0c49fa293709eb30a27cf70a58e4f929ce0
parent 81f65cc7

src/java/com/android/internal/telephony/dataconnection/DataThrottler.java

+12 −12
Original line number Diff line number Diff line
@@ -192,9 +192,18 @@ public class DataThrottler extends Handler { 


        List<ThrottleStatus> changedStatuses = new ArrayList<>();

        while (apnTypes != 0) {



            int apnType;

            // Due to an API mistake of ApnSetting.TYPE_DEFAULT (which combines default and hipri 

            // bit), we need to do special handling here.

            if ((apnTypes & ApnSetting.TYPE_DEFAULT) == ApnSetting.TYPE_DEFAULT) {

                apnType = ApnSetting.TYPE_DEFAULT;

                apnTypes &= ~ApnSetting.TYPE_DEFAULT;

            } else {

                //Extract the least significant bit.

            int apnType = apnTypes & -apnTypes;

                apnType = apnTypes & -apnTypes;

                //Remove the least significant bit.

                apnTypes &= apnTypes - 1;

            }



            //Update the apn throttle status

            ThrottleStatus newStatus = createStatus(apnType, retryElapsedTime, newRequestType);
@@ -209,9 +218,6 @@ public class DataThrottler extends Handler { 
                //Put the new status in the temp space

                mThrottleStatus.put(apnType, newStatus);

            }



            //Remove the least significant bit.

            apnTypes &= apnTypes - 1;

        }



        if (changedStatuses.size() > 0) {
@@ -232,12 +238,6 @@ public class DataThrottler extends Handler { 
     */

    @ElapsedRealtimeLong

    public long getRetryTime(@ApnType int apnType) {

        // This is the workaround to handle the mistake that

        // ApnSetting.TYPE_DEFAULT = ApnTypes.DEFAULT | ApnTypes.HIPRI.

        if (apnType == ApnSetting.TYPE_DEFAULT) {

            apnType &= ~(ApnSetting.TYPE_HIPRI);

        }



        ThrottleStatus status = mThrottleStatus.get(apnType);

        if (status != null) {

            if (status.getThrottleType() == ThrottleStatus.THROTTLE_TYPE_NONE) {

src/java/com/android/internal/telephony/dataconnection/DcTracker.java

+20 −3
Original line number Diff line number Diff line
@@ -94,6 +94,7 @@ import android.telephony.data.ApnSetting; 
import android.telephony.data.DataCallResponse;

import android.telephony.data.DataCallResponse.HandoverFailureMode;

import android.telephony.data.DataProfile;

import android.telephony.data.ThrottleStatus;

import android.telephony.gsm.GsmCellLocation;

import android.text.TextUtils;

import android.util.EventLog;
@@ -446,6 +447,19 @@ public class DcTracker extends Handler { 
        }

    };



    private class ThrottleStatusChangedCallback implements DataThrottler.Callback {

        @Override

        public void onThrottleStatusChanged(List<ThrottleStatus> throttleStatuses) {

            for (ThrottleStatus status : throttleStatuses) {

                if (status.getThrottleType() == ThrottleStatus.THROTTLE_TYPE_NONE) {

                    setupDataOnConnectableApn(mApnContextsByType.get(status.getApnType()),

                            Phone.REASON_DATA_UNTHROTTLED,

                            RetryFailures.ALWAYS);

                }

            }

        }

    }



    private NetworkPolicyManager mNetworkPolicyManager;

    private final NetworkPolicyManager.SubscriptionCallback mSubscriptionCallback =

            new NetworkPolicyManager.SubscriptionCallback() {
@@ -717,6 +731,8 @@ public class DcTracker extends Handler { 


    private final DataThrottler mDataThrottler;



    private final ThrottleStatusChangedCallback mThrottleStatusCallback;



    /**

     * Request network completion message map. Key is the APN type, value is the list of completion

     * messages to be sent. Using a list because there might be multiple network requests for
@@ -792,6 +808,9 @@ public class DcTracker extends Handler { 


        mSettingsObserver = new SettingsObserver(mPhone.getContext(), this);

        registerSettingsObserver();



        mThrottleStatusCallback = new ThrottleStatusChangedCallback();

        mDataThrottler.registerForThrottleStatusChanges(mThrottleStatusCallback);

    }



    @VisibleForTesting
@@ -806,6 +825,7 @@ public class DcTracker extends Handler { 
        mTransportType = 0;

        mDataServiceManager = null;

        mDataThrottler = null;

        mThrottleStatusCallback = null;

    }



    public void registerServiceStateTrackerEvents() {
@@ -2569,9 +2589,6 @@ public class DcTracker extends Handler { 
                @ApnType int apnTypes = apnSetting.getApnTypeBitmask();

                mDataThrottler.setRetryTime(apnTypes, RetryManager.NO_SUGGESTED_RETRY_DELAY,

                        REQUEST_TYPE_NORMAL);

                // After data unthrottled, we should see if it's possible to bring up the data

                // again.

                setupDataOnAllConnectableApns(Phone.REASON_DATA_UNTHROTTLED, RetryFailures.ALWAYS);

            } else {

                loge("EVENT_APN_UNTHROTTLED: Invalid APN passed: " + apn);

            }

tests/telephonytests/src/com/android/internal/telephony/dataconnection/DataThrottlerTest.java

+2 −18
Original line number Diff line number Diff line
@@ -60,8 +60,6 @@ public class DataThrottlerTest extends TelephonyTest { 
    @Mock

    private DataThrottler.Callback mMockChangedCallback2;



    private static final int DEFAULT_APN_TYPE = ApnSetting.TYPE_DEFAULT & ~(ApnSetting.TYPE_HIPRI);



    @Before

    public void setUp() throws Exception {

        super.setUp(getClass().getSimpleName());
@@ -99,14 +97,7 @@ public class DataThrottlerTest extends TelephonyTest { 
                new ThrottleStatus.Builder()

                    .setSlotIndex(0)

                    .setTransportType(AccessNetworkConstants.TRANSPORT_TYPE_WWAN)

                        .setApnType(ApnSetting.TYPE_HIPRI)

                        .setThrottleExpiryTimeMillis(1234567890L)

                        .setRetryType(ThrottleStatus.RETRY_TYPE_NEW_CONNECTION)

                        .build(),

                new ThrottleStatus.Builder()

                    .setSlotIndex(0)

                    .setTransportType(AccessNetworkConstants.TRANSPORT_TYPE_WWAN)

                    .setApnType(DEFAULT_APN_TYPE)

                    .setApnType(ApnSetting.TYPE_DEFAULT)

                    .setThrottleExpiryTimeMillis(1234567890L)

                    .setRetryType(ThrottleStatus.RETRY_TYPE_NEW_CONNECTION)

                    .build())
@@ -121,13 +112,6 @@ public class DataThrottlerTest extends TelephonyTest { 


        processAllMessages();

        expectedStatuses.add(List.of(

                new ThrottleStatus.Builder()

                        .setSlotIndex(0)

                        .setTransportType(AccessNetworkConstants.TRANSPORT_TYPE_WWAN)

                        .setApnType(ApnSetting.TYPE_HIPRI)

                        .setThrottleExpiryTimeMillis(13579L)

                        .setRetryType(ThrottleStatus.RETRY_TYPE_HANDOVER)

                        .build(),

                new ThrottleStatus.Builder()

                        .setSlotIndex(0)

                        .setTransportType(AccessNetworkConstants.TRANSPORT_TYPE_WWAN)
@@ -138,7 +122,7 @@ public class DataThrottlerTest extends TelephonyTest { 
                new ThrottleStatus.Builder()

                        .setSlotIndex(0)

                        .setTransportType(AccessNetworkConstants.TRANSPORT_TYPE_WWAN)

                        .setApnType(DEFAULT_APN_TYPE)

                        .setApnType(ApnSetting.TYPE_DEFAULT)

                        .setThrottleExpiryTimeMillis(13579L)

                        .setRetryType(ThrottleStatus.RETRY_TYPE_HANDOVER)

                        .build())