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

Commit 1b6208b2 authored by Tyler Gunn's avatar Tyler Gunn
Browse files

Fix concurrency issues in ImsPhoneCall. 

The class had a couple places where getFirstConnection() was referenced,
but it was possible for concurrent access to take place on the connection
between when getFirstConnection() was first called and when the return
value from it was referenced.  This was easy to fix by just storing the
instance of ImsPhoneConnection returned.

Fixes: 156168243
Test: Manual regression test for IMS phone calls.
Test: Add unit tests for get methods in question; can't really test the
concurrency in the test but can test basic operation.

Change-Id: I341fabf6e7cde869a7a8075f18ed15b593206471
parent ed4731a7

src/java/com/android/internal/telephony/imsphone/ImsPhoneCall.java

+11 −7
Original line number Diff line number Diff line
@@ -269,10 +269,14 @@ public class ImsPhoneCall extends Call { 
        return (ImsPhoneConnection) connections.get(0);

    }



    /*package*/ void

    setMute(boolean mute) {

        ImsCall imsCall = getFirstConnection() == null ?

                null : getFirstConnection().getImsCall();

    /**

     * Sets the mute state of the call.

     * @param mute {@code true} if the call could be muted; {@code false} otherwise.

     */

    @VisibleForTesting

    public void setMute(boolean mute) {

        ImsPhoneConnection connection = getFirstConnection();

        ImsCall imsCall = connection == null ? null : connection.getImsCall();

        if (imsCall != null) {

            try {

                imsCall.setMute(mute);
@@ -317,9 +321,9 @@ public class ImsPhoneCall extends Call { 
     */

    @VisibleForTesting

    @UnsupportedAppUsage

    public ImsCall

    getImsCall() {

        return (getFirstConnection() == null) ? null : getFirstConnection().getImsCall();

    public ImsCall getImsCall() {

        ImsPhoneConnection connection = getFirstConnection();

        return (connection == null) ? null : connection.getImsCall();

    }



    /*package*/ static boolean isLocalTone(ImsCall imsCall) {

tests/telephonytests/src/com/android/internal/telephony/imsphone/ImsPhoneCallTest.java

+26 −0
Original line number Diff line number Diff line
@@ -30,6 +30,8 @@ import android.test.suitebuilder.annotation.SmallTest; 


import androidx.test.filters.FlakyTest;



import com.android.ims.ImsCall;

import com.android.ims.ImsException;

import com.android.internal.telephony.Call;

import com.android.internal.telephony.TelephonyTest;
@@ -183,4 +185,28 @@ public class ImsPhoneCallTest extends TelephonyTest { 
        mImsCallUT.isMultiparty();

        verify(mImsCall, times(1)).isMultiparty();

    }



    @Test

    @SmallTest

    public void testGetImsCall() {

        doReturn(mImsCall).when(mConnection1).getImsCall();

        mImsCallUT.attach(mConnection1, Call.State.ACTIVE);



        ImsCall imsCall = mImsCallUT.getImsCall();

        assertEquals(mImsCall, imsCall);

    }



    @Test

    @SmallTest

    public void testSetMute() {

        doReturn(mImsCall).when(mConnection1).getImsCall();

        mImsCallUT.attach(mConnection1, Call.State.ACTIVE);



        mImsCallUT.setMute(true);

        try {

            verify(mImsCall).setMute(eq(true));

        } catch (ImsException e) {

            fail("Exception unexpected");

        }

    }

}