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

Commit 79486c77 authored by Jack Yu's avatar Jack Yu
Browse files

Fixed crash when retry data setup 

When network suggests retry, the original
associated network requests were not properly
populated in the retry entry, which caused crash
when retry happens.

Bug: 213587285
Test: Manual & atest DataNetworkControllerTest DataRetryManagerTest
Change-Id: I3f0f3033c0b32df96a9620040e04a1c034f8c42b
parent 90783bca

src/java/com/android/internal/telephony/data/DataNetworkController.java

+5 −12
Original line number Diff line number Diff line
@@ -1913,17 +1913,6 @@ public class DataNetworkController extends Handler { 
    private void onDataNetworkSetupRetry(@NonNull DataSetupRetryEntry dataSetupRetryEntry) {

        TelephonyNetworkRequest telephonyNetworkRequest =

                dataSetupRetryEntry.networkRequestList.get(0);

        // Since this is a retry, the network request might be already removed. So we need to double

        // check and remove request if necessary.

        dataSetupRetryEntry.networkRequestList.removeIf(

                networkRequest -> !mAllNetworkRequestList.contains(networkRequest));



        if (dataSetupRetryEntry.networkRequestList.isEmpty()) {

            log("onDataNetworkSetupRetry: all network requests in the retry entry has been "

                    + "released. Retry cancelled.");

            dataSetupRetryEntry.setState(DataRetryEntry.RETRY_STATE_CANCELLED);

            return;

        }

        int networkCapability = telephonyNetworkRequest.getApnTypeNetworkCapability();

        int preferredTransport = mAccessNetworksManager.getPreferredTransportByNetworkCapability(

                networkCapability);
@@ -1944,7 +1933,11 @@ public class DataNetworkController extends Handler { 
            if (dataProfile == null) {

                dataProfile = evaluation.getCandidateDataProfile();

            }

            if (dataProfile != null) {

                setupDataNetwork(dataProfile, dataSetupRetryEntry);

            } else {

                loge("onDataNetworkSetupRetry: Not able to find a suitable data profile to retry.");

            }

        }

    }

src/java/com/android/internal/telephony/data/DataRetryManager.java

+67 −31
Original line number Diff line number Diff line
@@ -128,10 +128,15 @@ public class DataRetryManager extends Handler { 
     */

    public static class DataThrottlingEntry {

        /**

         * The data profile that is being throttled for setup retry. Should be {@code null} when

         * retryType is {@link ThrottleStatus#RETRY_TYPE_HANDOVER}.

         * The data profile that is being throttled for setup/handover retry.

         */

        public final @Nullable DataProfile dataProfile;

        public final @NonNull DataProfile dataProfile;



        /**

         * The associated network request list when throttling happened. Should be {@code null} when

         * retry type is {@link ThrottleStatus#RETRY_TYPE_HANDOVER}.

         */

        public final @Nullable NetworkRequestList networkRequestList;



        /**

         * @param dataNetwork The data network that is being throttled for handover retry. Should be
@@ -154,8 +159,9 @@ public class DataRetryManager extends Handler { 
        /**

         * Constructor.

         *

         * @param dataProfile The data profile that is being throttled for setup retry. Should be

         * {@code null} when retryType is {@link ThrottleStatus#RETRY_TYPE_HANDOVER}.

         * @param dataProfile The data profile that is being throttled for setup/handover retry.

         * @param networkRequestList The associated network request list when throttling happened.

         * Should be {@code null} when retry type is {@link ThrottleStatus#RETRY_TYPE_HANDOVER}.

         * @param dataNetwork The data network that is being throttled for handover retry.

         * Should be {@code null} when retryType is

         * {@link ThrottleStatus#RETRY_TYPE_NEW_CONNECTION}.
@@ -163,10 +169,12 @@ public class DataRetryManager extends Handler { 
         * @param retryType The retry type when throttling expires.

         * @param expirationTimeMillis The expiration elapsed time of data throttling.

         */

        public DataThrottlingEntry(@Nullable DataProfile dataProfile,

        public DataThrottlingEntry(@NonNull DataProfile dataProfile,

                @Nullable NetworkRequestList networkRequestList,

                @Nullable DataNetwork dataNetwork, @TransportType int transport,

                @RetryType int retryType, @ElapsedRealtimeLong long expirationTimeMillis) {

            this.dataProfile = dataProfile;

            this.networkRequestList = networkRequestList;

            this.dataNetwork = dataNetwork;

            this.transport = transport;

            this.retryType = retryType;
@@ -175,8 +183,8 @@ public class DataRetryManager extends Handler { 


        @Override

        public @NonNull String toString() {

            return "[DataThrottlingEntry: dataProfile=" + dataProfile + ", dataNetwork="

                    + dataNetwork + ", transport="

            return "[DataThrottlingEntry: dataProfile=" + dataProfile + ", request list="

                    + networkRequestList + ", dataNetwork=" + dataNetwork + ", transport="

                    + AccessNetworkConstants.transportTypeToString(transport) + ", expiration time="

                    + DataUtils.elapsedTimeToString(expirationTimeMillis) + "]";

        }
@@ -574,6 +582,10 @@ public class DataRetryManager extends Handler { 
     * Represent a setup data retry entry.

     */

    public static class DataSetupRetryEntry extends DataRetryEntry {

        /**

         * Retry type is unknown. This should be only used for initialized value.

         */

        public static final int RETRY_TYPE_UNKNOWN = 0;

        /**

         * To retry setup data with the same data profile.

         */
@@ -587,6 +599,7 @@ public class DataRetryManager extends Handler { 


        @IntDef(prefix = {"RETRY_TYPE_"},

                value = {

                        RETRY_TYPE_UNKNOWN,

                        RETRY_TYPE_DATA_PROFILE,

                        RETRY_TYPE_NETWORK_REQUESTS,

                })
@@ -596,7 +609,7 @@ public class DataRetryManager extends Handler { 
        public final @SetupRetryType int setupRetryType;



        /** The network requests to satisfy when retry happens. */

        public final @Nullable NetworkRequestList networkRequestList;

        public final @NonNull NetworkRequestList networkRequestList;



        /** The data profile that will be used for retry. */

        public final @Nullable DataProfile dataProfile;
@@ -658,16 +671,16 @@ public class DataRetryManager extends Handler { 
         */

        public static class Builder<T extends Builder<T>> extends DataRetryEntry.Builder<T> {

            /** Data setup retry type. Could be retry by same data profile or same capabilities. */

            private @SetupRetryType int mSetupRetryType;

            private @SetupRetryType int mSetupRetryType = RETRY_TYPE_UNKNOWN;



            /** The network requests to satisfy when retry happens. */

            private @Nullable NetworkRequestList mNetworkRequestList;

            private @NonNull NetworkRequestList mNetworkRequestList;



            /** The data profile that will be used for retry. */

            private @NonNull DataProfile mDataProfile;

            private @Nullable DataProfile mDataProfile;



            /** The transport to retry data setup. */

            private @TransportType int mTransport;

            private @TransportType int mTransport = AccessNetworkConstants.TRANSPORT_TYPE_INVALID;



            /**

             * Set the data retry type.
@@ -721,6 +734,18 @@ public class DataRetryManager extends Handler { 
             * @return The instance of {@link DataSetupRetryEntry}.

             */

            public @NonNull DataSetupRetryEntry build() {

                if (mNetworkRequestList == null) {

                    throw new IllegalArgumentException("network request list is not specified.");

                }

                if (mTransport != AccessNetworkConstants.TRANSPORT_TYPE_WWAN

                        && mTransport != AccessNetworkConstants.TRANSPORT_TYPE_WLAN) {

                    throw new IllegalArgumentException("Invalid transport type " + mTransport);

                }

                if (mSetupRetryType != RETRY_TYPE_DATA_PROFILE

                        && mSetupRetryType != RETRY_TYPE_NETWORK_REQUESTS) {

                    throw new IllegalArgumentException("Invalid setup retry type "

                            + mSetupRetryType);

                }

                return new DataSetupRetryEntry(mSetupRetryType, mNetworkRequestList, mDataProfile,

                        mTransport, (DataSetupRetryRule) mAppliedDataRetryRule, mRetryDelayMillis);

            }
@@ -923,6 +948,8 @@ public class DataRetryManager extends Handler { 
        reset();

        mDataSetupRetryRuleList = mDataConfigManager.getDataSetupRetryRules();

        mDataHandoverRetryRuleList = mDataConfigManager.getDataHandoverRetryRules();

        log("onDataConfigUpdated: mDataSetupRetryRuleList=" + mDataSetupRetryRuleList

                + ", mDataHandoverRetryRuleList=" + mDataHandoverRetryRuleList);

    }



    /**
@@ -968,21 +995,24 @@ public class DataRetryManager extends Handler { 
            // when unthrottling happens, we still want to retry and we'll need

            // a type there so we know what to retry. Using RETRY_TYPE_NONE

            // ThrottleStatus is just for API backwards compatibility reason.

            updateThrottleStatus(dataProfile, null, ThrottleStatus.RETRY_TYPE_NEW_CONNECTION,

                    transport, Long.MAX_VALUE);

            updateThrottleStatus(dataProfile, requestList, null,

                    ThrottleStatus.RETRY_TYPE_NEW_CONNECTION, transport, Long.MAX_VALUE);

        } else if (retryDelayMillis != DataCallResponse.RETRY_DURATION_UNDEFINED) {

            // Network specifically asks retry the previous data profile again.

            DataSetupRetryEntry dataSetupRetryEntry = new DataSetupRetryEntry.Builder<>()

                    .setRetryDelay(retryDelayMillis)

                    .setSetupRetryType(DataSetupRetryEntry.RETRY_TYPE_DATA_PROFILE)

                    .setNetworkRequestList(requestList)

                    .setDataProfile(dataProfile)

                    .setTransport(transport)

                    .build();

            updateThrottleStatus(dataProfile, null, ThrottleStatus.RETRY_TYPE_NEW_CONNECTION,

                    transport, dataSetupRetryEntry.retryElapsedTime);

            updateThrottleStatus(dataProfile, requestList, null,

                    ThrottleStatus.RETRY_TYPE_NEW_CONNECTION, transport,

                    dataSetupRetryEntry.retryElapsedTime);

            schedule(dataSetupRetryEntry);

        } else {

            // Network did not suggest any retry. Use the configured rules to perform retry.

            logv("mDataSetupRetryRuleList=" + mDataSetupRetryRuleList);



            boolean retryScheduled = false;

            List<NetworkRequestList> groupedNetworkRequestLists =
@@ -996,7 +1026,7 @@ public class DataRetryManager extends Handler { 
                        if (isSimilarNetworkRequestRetryScheduled(networkRequestList.get(0))) {

                            log(networkRequestList.get(0) + " already had similar retry "

                                    + "scheduled.");

                            break;

                            return;

                        }



                        int failedCount = getRetryFailedCount(capability, retryRule);
@@ -1009,7 +1039,7 @@ public class DataRetryManager extends Handler { 
                                    + DataUtils.networkCapabilityToString(capability)

                                    + ". Condition-based retry will still happen when condition "

                                    + "changes.");

                            break;

                            return;

                        }



                        retryDelayMillis = retryRule.getRetryIntervalsMillis().get(
@@ -1063,7 +1093,8 @@ public class DataRetryManager extends Handler { 
            // when unthrottling happens, we still want to retry and we'll need

            // a type there so we know what to retry. Using RETRY_TYPE_NONE

            // ThrottleStatus is just for API backwards compatibility reason.

            updateThrottleStatus(dataNetwork.getDataProfile(), dataNetwork,

            updateThrottleStatus(dataNetwork.getDataProfile(),

                    dataNetwork.getAttachedNetworkRequestList(), dataNetwork,

                    ThrottleStatus.RETRY_TYPE_HANDOVER, targetTransport, Long.MAX_VALUE);

        } else if (retryDelayMillis != DataCallResponse.RETRY_DURATION_UNDEFINED) {

            // Network specifically asks retry the previous data profile again.
@@ -1072,7 +1103,8 @@ public class DataRetryManager extends Handler { 
                    .setDataNetwork(dataNetwork)

                    .build();



            updateThrottleStatus(dataNetwork.getDataProfile(), dataNetwork,

            updateThrottleStatus(dataNetwork.getDataProfile(),

                    dataNetwork.getAttachedNetworkRequestList(), dataNetwork,

                    ThrottleStatus.RETRY_TYPE_HANDOVER, targetTransport,

                    dataHandoverRetryEntry.retryElapsedTime);

            schedule(dataHandoverRetryEntry);
@@ -1108,7 +1140,7 @@ public class DataRetryManager extends Handler { 


    /** Cancel all retries and throttling entries. */

    private void onReset() {

        logl("onReset");

        logl("Remove all retry and throttling entries.");

        removeMessages(EVENT_DATA_SETUP_RETRY);

        removeMessages(EVENT_DATA_HANDOVER_RETRY);

        mDataRetryEntries.stream()
@@ -1199,35 +1231,38 @@ public class DataRetryManager extends Handler { 
    /**

     * Add the latest throttling request and report it to the clients.

     *

     * @param dataProfile The data profile that is being throttled for setup retry. Should be

     * {@code null} when retryType is {@link ThrottleStatus#RETRY_TYPE_HANDOVER}.

     * @param dataProfile The data profile that is being throttled for setup/handover retry.

     * @param networkRequestList The associated network request list when throttling happened.

     * Can be {@code null} when retry type is {@link ThrottleStatus#RETRY_TYPE_HANDOVER}.

     * @param dataNetwork The data network that is being throttled for handover retry.

     * Should be {@code null} when retryType is

     * Must be {@code null} when retryType is

     * {@link ThrottleStatus#RETRY_TYPE_NEW_CONNECTION}.

     * @param retryType The retry type when throttling expires.

     * @param transport The transport that the data profile has been throttled on.

     * @param expirationTime The expiration time of data throttling. This is the time retrieved from

     * {@link SystemClock#elapsedRealtime()}.

     */

    private void updateThrottleStatus(@Nullable DataProfile dataProfile,

    private void updateThrottleStatus(@NonNull DataProfile dataProfile,

            @Nullable NetworkRequestList networkRequestList,

            @Nullable DataNetwork dataNetwork, @RetryType int retryType,

            @TransportType int transport, @ElapsedRealtimeLong long expirationTime) {

        DataThrottlingEntry entry = new DataThrottlingEntry(dataProfile, dataNetwork, transport,

                retryType, expirationTime);

        DataThrottlingEntry entry = new DataThrottlingEntry(dataProfile, networkRequestList,

                dataNetwork, transport, retryType, expirationTime);

        if (mDataThrottlingEntries.size() >= MAXIMUM_HISTORICAL_ENTRIES) {

            mDataThrottlingEntries.remove(0);

        }



        // Remove previous entry that contains the same data profile.

        mDataThrottlingEntries.removeIf(

                throttlingEntry -> throttlingEntry.dataProfile.equals(dataProfile));

                throttlingEntry -> dataProfile.equals(throttlingEntry.dataProfile));





        logl("Add throttling entry " + entry);

        mDataThrottlingEntries.add(entry);



        // For backwards compatibility, we use RETRY_TYPE_NONE if network suggests never retry.

        final int dataRetryType = expirationTime == Long.MAX_VALUE

                ? ThrottleStatus.RETRY_TYPE_NONE

                : retryType;

                ? ThrottleStatus.RETRY_TYPE_NONE : retryType;



        // Report to the clients.

        final List<ThrottleStatus> throttleStatusList = new ArrayList<>();
@@ -1321,6 +1356,7 @@ public class DataRetryManager extends Handler { 
                        .setDataProfile(entry.dataProfile)

                        .setTransport(entry.transport)

                        .setSetupRetryType(DataSetupRetryEntry.RETRY_TYPE_DATA_PROFILE)

                        .setNetworkRequestList(entry.networkRequestList)

                        .setRetryDelay(0)

                        .build());

            } else if (entry.retryType == ThrottleStatus.RETRY_TYPE_HANDOVER) {

tests/telephonytests/src/com/android/internal/telephony/TelephonyTest.java

+40 −3
Original line number Diff line number Diff line
@@ -1093,13 +1093,50 @@ public abstract class TelephonyTest { 
        }

    }



    /**

     * @return The longest delay from all the message queues.

     */

    private long getLongestDelay() {

        long delay = 0;

        for (TestableLooper looper : mTestableLoopers) {

            MessageQueue queue = looper.getLooper().getQueue();

            try {

                Message msg = (Message) MESSAGE_QUEUE_FIELD.get(queue);

                while (msg != null) {

                    delay = Math.max(msg.getWhen(), delay);

                    msg = (Message) MESSAGE_NEXT_FIELD.get(msg);

                }

            } catch (IllegalAccessException e) {

                throw new RuntimeException("Access failed in TelephonyTest", e);

            }

        }

        return delay;

    }



    /**

     * @return {@code true} if there are any messages in the queue.

     */

    private boolean messagesExist() {

        for (TestableLooper looper : mTestableLoopers) {

            MessageQueue queue = looper.getLooper().getQueue();

            try {

                Message msg = (Message) MESSAGE_QUEUE_FIELD.get(queue);

                if (msg != null) return true;

            } catch (IllegalAccessException e) {

                throw new RuntimeException("Access failed in TelephonyTest", e);

            }

        }

        return false;

    }



    /**

     * Handle all messages including the delayed messages.

     */

    public void processAllFutureMessages() {

        while (messagesExist()) {

            moveTimeForward(getLongestDelay());

            processAllMessages();

        moveTimeForward(TimeUnit.DAYS.toMillis(1));

        processAllMessages();

        }

    }



    /**

tests/telephonytests/src/com/android/internal/telephony/data/DataNetworkControllerTest.java

+154 −18

File changed.

Preview size limit exceeded, changes collapsed.

tests/telephonytests/src/com/android/internal/telephony/data/DataRetryManagerTest.java

+1 −1
Original line number Diff line number Diff line
@@ -278,7 +278,7 @@ public class DataRetryManagerTest extends TelephonyTest { 
        assertThat(entry.setupRetryType).isEqualTo(DataSetupRetryEntry.RETRY_TYPE_DATA_PROFILE);

        assertThat(entry.dataProfile).isEqualTo(mDataProfile1);

        assertThat(entry.retryDelayMillis).isEqualTo(1000);

        assertThat(entry.networkRequestList).isNull();

        assertThat(entry.networkRequestList).isEqualTo(networkRequestList);

        assertThat(entry.appliedDataRetryRule).isNull();

    }