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

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

Fixed race condition in data connection 

Fixed that multiple threads are accessing
APN contexts associated with the data connection.

Test: Telephony sanity tests and unit tests
Bug: 120088861
Merged-In: I0cb85c6df7f221d841dbd42c9481f8563558e421
Change-Id: I0cb85c6df7f221d841dbd42c9481f8563558e421
(cherry picked from commit 74c62b14)
parent 6e24cd44

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

+10 −5
Original line number Diff line number Diff line
@@ -78,9 +78,12 @@ import java.io.PrintWriter; 
import java.io.StringWriter;

import java.net.InetAddress;

import java.net.UnknownHostException;

import java.util.ArrayList;

import java.util.Collection;

import java.util.HashMap;

import java.util.List;

import java.util.Locale;

import java.util.Map;

import java.util.concurrent.ConcurrentHashMap;

import java.util.concurrent.atomic.AtomicInteger;



/**
@@ -189,7 +192,7 @@ public class DataConnection extends StateMachine { 


    int mTag;

    public int mCid;

    public HashMap<ApnContext, ConnectionParams> mApnContexts = null;

    private final Map<ApnContext, ConnectionParams> mApnContexts = new ConcurrentHashMap<>();

    PendingIntent mReconnectIntent = null;
@@ -489,8 +492,6 @@ public class DataConnection extends StateMachine { 
            addState(mDisconnectingState, mDefaultState);

            addState(mDisconnectingErrorCreatingConnection, mDefaultState);

        setInitialState(mInactiveState);



        mApnContexts = new HashMap<>();

    }



    /**
@@ -1282,7 +1283,7 @@ public class DataConnection extends StateMachine { 
                mAc.disconnected();

                mAc = null;

            }

            mApnContexts = null;

            mApnContexts.clear();

            mReconnectIntent = null;

            mDct = null;

            mApnSetting = null;
@@ -2451,6 +2452,10 @@ public class DataConnection extends StateMachine { 
        return (long) response.getSuggestedRetryTime();

    }



    public List<ApnContext> getApnContexts() {

        return new ArrayList<>(mApnContexts.keySet());

    }



    /**

     * @return the string for msg.what as our info.

     */

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

+13 −11
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ import java.io.FileDescriptor; 
import java.io.PrintWriter;

import java.util.ArrayList;

import java.util.HashMap;

import java.util.List;



/**

 * Data Connection Controller which is a package visible class and controls
@@ -314,7 +315,8 @@ public class DcController extends StateMachine { 
                    continue;

                }



                if (dc.mApnContexts.size() == 0) {

                List<ApnContext> apnContexts = dc.getApnContexts();

                if (apnContexts.size() == 0) {

                    if (DBG) loge("onDataStateChanged: no connected apns, ignore");

                } else {

                    // Determine if the connection/apnContext should be cleaned up
@@ -325,7 +327,7 @@ public class DcController extends StateMachine { 
                    }

                    if (newState.getActive() == DATA_CONNECTION_ACTIVE_PH_LINK_INACTIVE) {

                        if (mDct.isCleanupRequired.get()) {

                            apnsToCleanup.addAll(dc.mApnContexts.keySet());

                            apnsToCleanup.addAll(apnContexts);

                            mDct.isCleanupRequired.set(false);

                        } else {

                            DcFailCause failCause = DcFailCause.fromInt(newState.getStatus());
@@ -341,7 +343,7 @@ public class DcController extends StateMachine { 
                                    log("onDataStateChanged: inactive, add to cleanup list. "

                                            + "failCause=" + failCause);

                                }

                                apnsToCleanup.addAll(dc.mApnContexts.keySet());

                                apnsToCleanup.addAll(apnContexts);

                            } else {

                                if (DBG) {

                                    log("onDataStateChanged: inactive, add to retry list. "
@@ -382,16 +384,16 @@ public class DcController extends StateMachine { 
                                    }

                                    if (needToClean) {

                                        if (DBG) {

                                            log("onDataStateChanged: addr change," +

                                                    " cleanup apns=" + dc.mApnContexts +

                                                    " oldLp=" + result.oldLp +

                                                    " newLp=" + result.newLp);

                                            log("onDataStateChanged: addr change,"

                                                    + " cleanup apns=" + apnContexts

                                                    + " oldLp=" + result.oldLp

                                                    + " newLp=" + result.newLp);

                                        }

                                        apnsToCleanup.addAll(dc.mApnContexts.keySet());

                                        apnsToCleanup.addAll(apnContexts);

                                    } else {

                                        if (DBG) log("onDataStateChanged: simple change");



                                        for (ApnContext apnContext : dc.mApnContexts.keySet()) {

                                        for (ApnContext apnContext : apnContexts) {

                                             mPhone.notifyDataConnection(

                                                 PhoneConstants.REASON_LINK_PROPERTIES_CHANGED,

                                                 apnContext.getApnType());
@@ -403,10 +405,10 @@ public class DcController extends StateMachine { 
                                    }

                                }

                            } else {

                                apnsToCleanup.addAll(dc.mApnContexts.keySet());

                                apnsToCleanup.addAll(apnContexts);

                                if (DBG) {

                                    log("onDataStateChanged: interface change, cleanup apns="

                                            + dc.mApnContexts);

                                            + apnContexts);

                                }

                            }

                        }

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

+5 −3
Original line number Diff line number Diff line
@@ -104,6 +104,7 @@ import java.io.PrintWriter; 
import java.util.ArrayList;

import java.util.Comparator;

import java.util.HashMap;

import java.util.List;

import java.util.Map.Entry;

import java.util.PriorityQueue;

import java.util.Set;
@@ -4434,7 +4435,7 @@ public class DcTracker extends Handler { 
                }

                // check if this dc is still connecting

                if (cid == -1) {

                    for (ApnContext apnContext : dc.mApnContexts.keySet()) {

                    for (ApnContext apnContext : dc.getApnContexts()) {

                        if (apnContext.getState() == DctConstants.State.CONNECTING) {

                            if (VDBG) log("  found potential " + dc);

                            dcList.add(dc);
@@ -4449,11 +4450,12 @@ public class DcTracker extends Handler { 
            return;

        }

        for (DataConnection dc : dcList) {

            if (dc.mApnContexts.size() == 0) {

            List<ApnContext> apnContextList = dc.getApnContexts();

            if (apnContextList.size() == 0) {

                break;

            }

            // send one out for each apn type in play

            for (ApnContext apnContext : dc.mApnContexts.keySet()) {

            for (ApnContext apnContext : apnContextList) {

                String apnType = apnContext.getApnType();



                final Intent intent = new Intent(TelephonyIntents.ACTION_CARRIER_SIGNAL_PCO_VALUE);

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

+5 −6
Original line number Diff line number Diff line
@@ -42,7 +42,6 @@ import android.test.suitebuilder.annotation.SmallTest; 


import com.android.internal.telephony.DctConstants;

import com.android.internal.telephony.TelephonyTest;

import com.android.internal.telephony.dataconnection.DataConnection.ConnectionParams;

import com.android.internal.telephony.dataconnection.DataConnection.UpdateLinkPropertyResult;

import com.android.internal.util.IState;

import com.android.internal.util.StateMachine;
@@ -55,7 +54,7 @@ import org.mockito.Mock; 
import java.lang.reflect.Method;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.HashMap;

import java.util.List;



public class DcControllerTest extends TelephonyTest {
@@ -63,11 +62,11 @@ public class DcControllerTest extends TelephonyTest { 
    private static final int EVENT_DATA_STATE_CHANGED = 0x00040007;



    @Mock

    DataConnection mDc;

    private DataConnection mDc;

    @Mock

    HashMap<ApnContext, ConnectionParams> mApnContexts;

    private List<ApnContext> mApnContexts;

    @Mock

    DataServiceManager mDataServiceManager;

    private DataServiceManager mDataServiceManager;



    UpdateLinkPropertyResult mResult;
@@ -107,8 +106,8 @@ public class DcControllerTest extends TelephonyTest { 
        super.setUp(getClass().getSimpleName());



        doReturn("fake.action_detached").when(mPhone).getActionDetached();

        mDc.mApnContexts = mApnContexts;

        doReturn(1).when(mApnContexts).size();

        doReturn(mApnContexts).when(mDc).getApnContexts();



        LinkProperties lp = new LinkProperties();

        mResult = new UpdateLinkPropertyResult(lp);