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

Commit 3d4b8e78 authored by Grace Jia's avatar Grace Jia
Browse files

Fix potential race condition in Call.

1) Add getConnections method to Call and return a copied mConnection field.
2) Add removeConnection, addConnection, getConnectionsFrom methods to
Call to avoid direct access of mConnection.
3) Add synchronization of all operation on mConnection.

Test: atest FrameworksTelephonyTests.
Bug: 131876970

Change-Id: If8f8df48ad52226cea401a82dfc22f6edb1f607e
parent 6a51c683
Loading
Loading
Loading
Loading
+62 −7
Original line number Original line Diff line number Diff line
@@ -85,7 +85,9 @@ public abstract class Call {
    public State mState = State.IDLE;
    public State mState = State.IDLE;


    @UnsupportedAppUsage
    @UnsupportedAppUsage
    public ArrayList<Connection> mConnections = new ArrayList<Connection>();
    public ArrayList<Connection> mConnections = new ArrayList<>();

    private Object mLock = new Object();


    /* Instance Methods */
    /* Instance Methods */


@@ -94,7 +96,30 @@ public abstract class Call {
     */
     */


    @UnsupportedAppUsage
    @UnsupportedAppUsage
    public abstract List<Connection> getConnections();
    public ArrayList<Connection> getConnections() {
        synchronized (mLock) {
            return (ArrayList<Connection>) mConnections.clone();
        }
    }

    /**
     * Get mConnections field from another Call instance.
     * @param other
     */
    public void copyConnectionFrom(Call other) {
        mConnections = other.getConnections();
    }

    /**
     * Get connections count of this instance.
     * @return the count to return
     */
    public int getConnectionsCount() {
        synchronized (mLock) {
            return mConnections.size();
        }
    }

    @UnsupportedAppUsage
    @UnsupportedAppUsage
    public abstract Phone getPhone();
    public abstract Phone getPhone();
    @UnsupportedAppUsage
    @UnsupportedAppUsage
@@ -129,6 +154,37 @@ public abstract class Call {
        return connections.size() > 0;
        return connections.size() > 0;
    }
    }


    /**
     * removeConnection
     *
     * @param conn the connection to be removed
     */
    public void removeConnection(Connection conn) {
        synchronized (mLock) {
            mConnections.remove(conn);
        }
    }

    /**
     * addConnection
     *
     * @param conn the connection to be added
     */
    public void addConnection(Connection conn) {
        synchronized (mLock) {
            mConnections.add(conn);
        }
    }

    /**
     * clearConnection
     */
    public void clearConnections() {
        synchronized (mLock) {
            mConnections.clear();
        }
    }

    /**
    /**
     * getState
     * getState
     * @return state of class call
     * @return state of class call
@@ -289,14 +345,13 @@ public abstract class Call {
     * Called when it's time to clean up disconnected Connection objects
     * Called when it's time to clean up disconnected Connection objects
     */
     */
    public void clearDisconnected() {
    public void clearDisconnected() {
        for (int i = mConnections.size() - 1 ; i >= 0 ; i--) {
        for (Connection conn : getConnections()) {
            Connection c = mConnections.get(i);
            if (conn.getState() == State.DISCONNECTED) {
            if (c.getState() == State.DISCONNECTED) {
                removeConnection(conn);
                mConnections.remove(i);
            }
            }
        }
        }


        if (mConnections.size() == 0) {
        if (getConnectionsCount() == 0) {
            setState(State.IDLE);
            setState(State.IDLE);
        }
        }
    }
    }
+10 −20
Original line number Original line Diff line number Diff line
@@ -18,8 +18,6 @@ package com.android.internal.telephony;


import android.compat.annotation.UnsupportedAppUsage;
import android.compat.annotation.UnsupportedAppUsage;


import java.util.List;

/**
/**
 * {@hide}
 * {@hide}
 */
 */
@@ -36,12 +34,6 @@ public class GsmCdmaCall extends Call {


    /************************** Overridden from Call *************************/
    /************************** Overridden from Call *************************/


    @Override
    public List<Connection> getConnections() {
        // FIXME should return Collections.unmodifiableList();
        return mConnections;
    }

    @Override
    @Override
    public Phone getPhone() {
    public Phone getPhone() {
        return mOwner.getPhone();
        return mOwner.getPhone();
@@ -49,7 +41,7 @@ public class GsmCdmaCall extends Call {


    @Override
    @Override
    public boolean isMultiparty() {
    public boolean isMultiparty() {
        return mConnections.size() > 1;
        return getConnectionsCount() > 1;
    }
    }


    /** Please note: if this is the foreground call and a
    /** Please note: if this is the foreground call and a
@@ -79,14 +71,14 @@ public class GsmCdmaCall extends Call {
    //***** Called from GsmCdmaConnection
    //***** Called from GsmCdmaConnection


    public void attach(Connection conn, DriverCall dc) {
    public void attach(Connection conn, DriverCall dc) {
        mConnections.add(conn);
        addConnection(conn);


        mState = stateFromDCState (dc.state);
        mState = stateFromDCState (dc.state);
    }
    }


    @UnsupportedAppUsage
    @UnsupportedAppUsage
    public void attachFake(Connection conn, State state) {
    public void attachFake(Connection conn, State state) {
        mConnections.add(conn);
        addConnection(conn);


        mState = state;
        mState = state;
    }
    }
@@ -100,8 +92,8 @@ public class GsmCdmaCall extends Call {


            boolean hasOnlyDisconnectedConnections = true;
            boolean hasOnlyDisconnectedConnections = true;


            for (int i = 0, s = mConnections.size(); i < s; i ++) {
            for (Connection c : getConnections()) {
                if (mConnections.get(i).getState() != State.DISCONNECTED) {
                if (c.getState() != State.DISCONNECTED) {
                    hasOnlyDisconnectedConnections = false;
                    hasOnlyDisconnectedConnections = false;
                    break;
                    break;
                }
                }
@@ -117,9 +109,9 @@ public class GsmCdmaCall extends Call {
    }
    }


    public void detach(GsmCdmaConnection conn) {
    public void detach(GsmCdmaConnection conn) {
        mConnections.remove(conn);
        removeConnection(conn);


        if (mConnections.size() == 0) {
        if (getConnectionsCount() == 0) {
            mState = State.IDLE;
            mState = State.IDLE;
        }
        }
    }
    }
@@ -143,7 +135,7 @@ public class GsmCdmaCall extends Call {
     * connections to be added via "conference"
     * connections to be added via "conference"
     */
     */
    /*package*/ boolean isFull() {
    /*package*/ boolean isFull() {
        return mConnections.size() == mOwner.getMaxConnectionsPerCall();
        return getConnectionsCount() == mOwner.getMaxConnectionsPerCall();
    }
    }


    //***** Called from GsmCdmaCallTracker
    //***** Called from GsmCdmaCallTracker
@@ -155,10 +147,8 @@ public class GsmCdmaCall extends Call {
     * but no response has yet been received so update() has not yet been called
     * but no response has yet been received so update() has not yet been called
     */
     */
    void onHangupLocal() {
    void onHangupLocal() {
        for (int i = 0, s = mConnections.size(); i < s; i++) {
        for (Connection conn : getConnections()) {
            GsmCdmaConnection cn = (GsmCdmaConnection)mConnections.get(i);
            ((GsmCdmaConnection) conn).onHangupLocal();

            cn.onHangupLocal();
        }
        }
        mState = State.DISCONNECTING;
        mState = State.DISCONNECTING;
    }
    }
+33 −38
Original line number Original line Diff line number Diff line
@@ -260,16 +260,13 @@ public class GsmCdmaCallTracker extends CallTracker {


    @UnsupportedAppUsage
    @UnsupportedAppUsage
    private void fakeHoldForegroundBeforeDial() {
    private void fakeHoldForegroundBeforeDial() {
        List<Connection> connCopy;

        // We need to make a copy here, since fakeHoldBeforeDial()
        // We need to make a copy here, since fakeHoldBeforeDial()
        // modifies the lists, and we don't want to reverse the order
        // modifies the lists, and we don't want to reverse the order
        connCopy = (List<Connection>) mForegroundCall.mConnections.clone();
        ArrayList<Connection> connCopy = mForegroundCall.getConnections();

        for (int i = 0, s = connCopy.size() ; i < s ; i++) {
            GsmCdmaConnection conn = (GsmCdmaConnection)connCopy.get(i);


            conn.fakeHoldBeforeDial();
        for (Connection conn : connCopy) {
            GsmCdmaConnection gsmCdmaConn = (GsmCdmaConnection) conn;
            gsmCdmaConn.fakeHoldBeforeDial();
        }
        }
    }
    }


@@ -602,7 +599,7 @@ public class GsmCdmaCallTracker extends CallTracker {
                mCi.switchWaitingOrHoldingAndActive(
                mCi.switchWaitingOrHoldingAndActive(
                        obtainCompleteMessage(EVENT_SWITCH_RESULT));
                        obtainCompleteMessage(EVENT_SWITCH_RESULT));
            } else {
            } else {
                if (mForegroundCall.getConnections().size() > 1) {
                if (mForegroundCall.getConnectionsCount() > 1) {
                    flashAndSetGenericTrue();
                    flashAndSetGenericTrue();
                } else {
                } else {
                    // Send a flash command to CDMA network for putting the other party on hold.
                    // Send a flash command to CDMA network for putting the other party on hold.
@@ -931,18 +928,19 @@ public class GsmCdmaCallTracker extends CallTracker {
                    // we need to clean up the foregroundCall and ringingCall.
                    // we need to clean up the foregroundCall and ringingCall.
                    // Loop through foreground call connections as
                    // Loop through foreground call connections as
                    // it contains the known logical connections.
                    // it contains the known logical connections.
                    int count = mForegroundCall.mConnections.size();
                    ArrayList<Connection> connections = mForegroundCall.getConnections();
                    int count = connections.size();
                    for (int n = 0; n < count; n++) {
                    for (int n = 0; n < count; n++) {
                        if (Phone.DEBUG_PHONE) log("adding fgCall cn " + n + " to droppedDuringPoll");
                        if (Phone.DEBUG_PHONE) log("adding fgCall cn " + n + " to droppedDuringPoll");
                        GsmCdmaConnection cn = (GsmCdmaConnection)mForegroundCall.mConnections.get(n);
                        GsmCdmaConnection cn = (GsmCdmaConnection) connections.get(n);
                        mDroppedDuringPoll.add(cn);
                        mDroppedDuringPoll.add(cn);
                    }
                    }
                    count = mRingingCall.mConnections.size();
                    count = mRingingCall.getConnectionsCount();
                    // Loop through ringing call connections as
                    // Loop through ringing call connections as
                    // it may contain the known logical connections.
                    // it may contain the known logical connections.
                    for (int n = 0; n < count; n++) {
                    for (int n = 0; n < count; n++) {
                        if (Phone.DEBUG_PHONE) log("adding rgCall cn " + n + " to droppedDuringPoll");
                        if (Phone.DEBUG_PHONE) log("adding rgCall cn " + n + " to droppedDuringPoll");
                        GsmCdmaConnection cn = (GsmCdmaConnection)mRingingCall.mConnections.get(n);
                        GsmCdmaConnection cn = (GsmCdmaConnection) connections.get(n);
                        mDroppedDuringPoll.add(cn);
                        mDroppedDuringPoll.add(cn);
                    }
                    }


@@ -1274,7 +1272,7 @@ public class GsmCdmaCallTracker extends CallTracker {
    //***** Called from GsmCdmaCall
    //***** Called from GsmCdmaCall


    public void hangup(GsmCdmaCall call) throws CallStateException {
    public void hangup(GsmCdmaCall call) throws CallStateException {
        if (call.getConnections().size() == 0) {
        if (call.getConnectionsCount() == 0) {
            throw new CallStateException("no connections in call");
            throw new CallStateException("no connections in call");
        }
        }


@@ -1316,18 +1314,20 @@ public class GsmCdmaCallTracker extends CallTracker {
    }
    }


    private void logHangupEvent(GsmCdmaCall call) {
    private void logHangupEvent(GsmCdmaCall call) {
        int count = call.mConnections.size();
        for (Connection conn : call.getConnections()) {
        for (int i = 0; i < count; i++) {
            GsmCdmaConnection c = (GsmCdmaConnection) conn;
            GsmCdmaConnection cn = (GsmCdmaConnection) call.mConnections.get(i);
            int call_index;
            int call_index;
            try {
            try {
                call_index = cn.getGsmCdmaIndex();
                call_index = c.getGsmCdmaIndex();
            } catch (CallStateException ex) {
            } catch (CallStateException e) {
                call_index = -1;
                call_index = -1;
            }
            }
            mMetrics.writeRilHangup(mPhone.getPhoneId(), cn, call_index, getNetworkCountryIso());
            mMetrics.writeRilHangup(mPhone.getPhoneId(), c, call_index, getNetworkCountryIso());
        }
        if (VDBG) {
            Rlog.v(LOG_TAG, "logHangupEvent logged " + call.getConnectionsCount()
                    + " Connections ");
        }
        }
        if (VDBG) Rlog.v(LOG_TAG, "logHangupEvent logged " + count + " Connections ");
    }
    }


    public void hangupWaitingOrBackground() {
    public void hangupWaitingOrBackground() {
@@ -1343,29 +1343,26 @@ public class GsmCdmaCallTracker extends CallTracker {


    public void hangupConnectionByIndex(GsmCdmaCall call, int index)
    public void hangupConnectionByIndex(GsmCdmaCall call, int index)
            throws CallStateException {
            throws CallStateException {
        int count = call.mConnections.size();
        for (Connection conn : call.getConnections()) {
        for (int i = 0; i < count; i++) {
            GsmCdmaConnection c = (GsmCdmaConnection) conn;
            GsmCdmaConnection cn = (GsmCdmaConnection)call.mConnections.get(i);
            if (!c.mDisconnected && c.getGsmCdmaIndex() == index) {
            if (!cn.mDisconnected && cn.getGsmCdmaIndex() == index) {
                mMetrics.writeRilHangup(mPhone.getPhoneId(), c, c.getGsmCdmaIndex(),
                mMetrics.writeRilHangup(mPhone.getPhoneId(), cn, cn.getGsmCdmaIndex(),
                        getNetworkCountryIso());
                        getNetworkCountryIso());
                mCi.hangupConnection(index, obtainCompleteMessage());
                mCi.hangupConnection(index, obtainCompleteMessage());
                return;
                return;
            }
            }
        }
        }

        throw new CallStateException("no GsmCdma index found");
        throw new CallStateException("no GsmCdma index found");
    }
    }


    public void hangupAllConnections(GsmCdmaCall call) {
    public void hangupAllConnections(GsmCdmaCall call) {
        try {
        try {
            int count = call.mConnections.size();
            for (Connection conn : call.getConnections()) {
            for (int i = 0; i < count; i++) {
                GsmCdmaConnection c = (GsmCdmaConnection) conn;
                GsmCdmaConnection cn = (GsmCdmaConnection)call.mConnections.get(i);
                if (!c.mDisconnected) {
                if (!cn.mDisconnected) {
                    mMetrics.writeRilHangup(mPhone.getPhoneId(), c, c.getGsmCdmaIndex(),
                    mMetrics.writeRilHangup(mPhone.getPhoneId(), cn, cn.getGsmCdmaIndex(),
                            getNetworkCountryIso());
                            getNetworkCountryIso());
                    mCi.hangupConnection(cn.getGsmCdmaIndex(), obtainCompleteMessage());
                    mCi.hangupConnection(c.getGsmCdmaIndex(), obtainCompleteMessage());
                }
                }
            }
            }
        } catch (CallStateException ex) {
        } catch (CallStateException ex) {
@@ -1375,14 +1372,12 @@ public class GsmCdmaCallTracker extends CallTracker {


    public GsmCdmaConnection getConnectionByIndex(GsmCdmaCall call, int index)
    public GsmCdmaConnection getConnectionByIndex(GsmCdmaCall call, int index)
            throws CallStateException {
            throws CallStateException {
        int count = call.mConnections.size();
        for (Connection conn : call.getConnections()) {
        for (int i = 0; i < count; i++) {
            GsmCdmaConnection c = (GsmCdmaConnection) conn;
            GsmCdmaConnection cn = (GsmCdmaConnection)call.mConnections.get(i);
            if (!c.mDisconnected && c.getGsmCdmaIndex() == index) {
            if (!cn.mDisconnected && cn.getGsmCdmaIndex() == index) {
                return c;
                return cn;
            }
            }
        }
        }

        return null;
        return null;
    }
    }


+1 −9
Original line number Original line Diff line number Diff line
@@ -21,11 +21,8 @@ import android.telephony.ims.ImsExternalCallState;


import com.android.internal.telephony.Call;
import com.android.internal.telephony.Call;
import com.android.internal.telephony.CallStateException;
import com.android.internal.telephony.CallStateException;
import com.android.internal.telephony.Connection;
import com.android.internal.telephony.Phone;
import com.android.internal.telephony.Phone;


import java.util.List;

/**
/**
 * Companion class for {@link ImsExternalConnection}; represents an external call which was
 * Companion class for {@link ImsExternalConnection}; represents an external call which was
 * received via {@link ImsExternalCallState} info.
 * received via {@link ImsExternalCallState} info.
@@ -37,12 +34,7 @@ public class ImsExternalCall extends Call {
    @UnsupportedAppUsage
    @UnsupportedAppUsage
    public ImsExternalCall(Phone phone, ImsExternalConnection connection) {
    public ImsExternalCall(Phone phone, ImsExternalConnection connection) {
        mPhone = phone;
        mPhone = phone;
        mConnections.add(connection);
        addConnection(connection);
    }

    @Override
    public List<Connection> getConnections() {
        return mConnections;
    }
    }


    @Override
    @Override
+3 −3
Original line number Original line Diff line number Diff line
@@ -1402,11 +1402,11 @@ public class ImsPhone extends ImsPhoneBase {
    public ArrayList<Connection> getHandoverConnection() {
    public ArrayList<Connection> getHandoverConnection() {
        ArrayList<Connection> connList = new ArrayList<Connection>();
        ArrayList<Connection> connList = new ArrayList<Connection>();
        // Add all foreground call connections
        // Add all foreground call connections
        connList.addAll(getForegroundCall().mConnections);
        connList.addAll(getForegroundCall().getConnections());
        // Add all background call connections
        // Add all background call connections
        connList.addAll(getBackgroundCall().mConnections);
        connList.addAll(getBackgroundCall().getConnections());
        // Add all background call connections
        // Add all background call connections
        connList.addAll(getRingingCall().mConnections);
        connList.addAll(getRingingCall().getConnections());
        if (connList.size() > 0) {
        if (connList.size() > 0) {
            return connList;
            return connList;
        } else {
        } else {
Loading