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

Commit bbe5d150 authored by Tyler Gunn's avatar Tyler Gunn
Browse files

Fix concurrency issues with ringtone playback. 

CallAudioManager has a number of calls into Ringer.java; this code can
come from CallAudioModeStateMachine which is in turn called into via
Bluetooth callbacks.  Added synchronization using the Telecom lock on these
calls to ensure ringer operations don't have concurrency issues.

Also, in AsyncRingtonePlayer add some null checks to ensure that it is
not possible to run into the NPE reported in the bug, should concurrency
issues persist.

Test: Ran unit tests to confirm no regressions in behavior.
Test: Manual test with bluetooth headset; verify I'm seeing the session
logging for ringtone playing now.
Bug: 135997076

Change-Id: I9a119b15d2853a44d5850d040bd31340d1e34127
parent bf78556a

src/com/android/server/telecom/AsyncRingtonePlayer.java

+72 −56
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import android.os.Handler; 
import android.os.HandlerThread;

import android.os.Message;

import android.telecom.Log;

import android.telecom.Logging.Session;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.os.SomeArgs;
@@ -107,6 +108,7 @@ public class AsyncRingtonePlayer { 
        args.arg2 = incomingCall;

        args.arg3 = volumeShaperConfig;

        args.arg4 = isVibrationEnabled;

        args.arg5 = Log.createSubsession();

        postMessage(EVENT_PLAY, true /* shouldCreateHandler */, args);

        return mHapticsFuture;

    }
@@ -173,11 +175,17 @@ public class AsyncRingtonePlayer { 
        Call incomingCall = (Call) args.arg2;

        VolumeShaper.Configuration volumeShaperConfig = (VolumeShaper.Configuration) args.arg3;

        boolean isVibrationEnabled = (boolean) args.arg4;

        Session session = (Session) args.arg5;

        args.recycle();



        Log.continueSession(session, "ARP.hP");

        try {

            // don't bother with any of this if there is an EVENT_STOP waiting.

            if (mHandler.hasMessages(EVENT_STOP)) {

                if (mHapticsFuture != null) {

                    mHapticsFuture.complete(false /* ringtoneHasHaptics */);

                    mHapticsFuture = null;

                }

                return;

            }
@@ -185,8 +193,11 @@ public class AsyncRingtonePlayer { 
            // anything.

            if (Uri.EMPTY.equals(incomingCall.getRingtone())) {

                mRingtone = null;

                if (mHapticsFuture != null) {

                    mHapticsFuture.complete(false /* ringtoneHasHaptics */);

                    mHapticsFuture = null;

                }



                return;

            }
@@ -201,8 +212,10 @@ public class AsyncRingtonePlayer { 
                            ringtoneUri.toSafeString();

                    Log.addEvent(null, LogUtils.Events.ERROR_LOG, "Failed to get ringtone from " +

                            "factory. Skipping ringing. Uri was: " + ringtoneUriString);

                    if (mHapticsFuture != null) {

                        mHapticsFuture.complete(false /* ringtoneHasHaptics */);

                        mHapticsFuture = null;

                    }

                    return;

                }
@@ -241,6 +254,9 @@ public class AsyncRingtonePlayer { 
                mRingtone.play();

                Log.i(this, "Play ringtone, looping.");

            }

        } finally {

            Log.cancelSubsession(session);

        }

    }



    private void handleRepeat() {

src/com/android/server/telecom/CallAudioManager.java

+27 −15
Original line number Diff line number Diff line
@@ -431,6 +431,7 @@ public class CallAudioManager extends CallsManagerListenerBase { 
    }



    void silenceRingers() {

        synchronized (mCallsManager.getLock()) {

            for (Call call : mRingingCalls) {

                call.silence();

            }
@@ -438,29 +439,38 @@ public class CallAudioManager extends CallsManagerListenerBase { 
            mRinger.stopRinging();

            mRinger.stopCallWaiting();

        }

    }



    @VisibleForTesting

    public boolean startRinging() {

        synchronized (mCallsManager.getLock()) {

            return mRinger.startRinging(mForegroundCall,

                    mCallAudioRouteStateMachine.isHfpDeviceAvailable());

        }

    }



    @VisibleForTesting

    public void startCallWaiting(String reason) {

        synchronized (mCallsManager.getLock()) {

            if (mRingingCalls.size() == 1) {

                mRinger.startCallWaiting(mRingingCalls.iterator().next(), reason);

            }

        }

    }



    @VisibleForTesting

    public void stopRinging() {

        synchronized (mCallsManager.getLock()) {

            mRinger.stopRinging();

        }

    }



    @VisibleForTesting

    public void stopCallWaiting() {

        synchronized (mCallsManager.getLock()) {

            mRinger.stopCallWaiting();

        }

    }



    @VisibleForTesting

    public void setCallAudioRouteFocusState(int focusState) {
@@ -801,12 +811,14 @@ public class CallAudioManager extends CallsManagerListenerBase { 
    private void maybeStopRingingAndCallWaitingForAnsweredOrRejectedCall(Call call) {

        // Check to see if the call being answered/rejected is the only ringing call, since this

        // will be called before the connection service acknowledges the state change.

        synchronized (mCallsManager.getLock()) {

            if (mRingingCalls.size() == 0 ||

                    (mRingingCalls.size() == 1 && call == mRingingCalls.iterator().next())) {

                mRinger.stopRinging();

                mRinger.stopCallWaiting();

            }

        }

    }



    private boolean shouldPlayDisconnectTone(int oldState, int newState) {

        if (newState != CallState.DISCONNECTED) {

tests/src/com/android/server/telecom/tests/CallAudioManagerTest.java

+3 −0
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ import com.android.server.telecom.DtmfLocalTonePlayer; 
import com.android.server.telecom.InCallTonePlayer;

import com.android.server.telecom.RingbackPlayer;

import com.android.server.telecom.Ringer;

import com.android.server.telecom.TelecomSystem;

import com.android.server.telecom.bluetooth.BluetoothStateReceiver;



import org.junit.Before;
@@ -66,6 +67,7 @@ public class CallAudioManagerTest extends TelecomTestCase { 
    @Mock private RingbackPlayer mRingbackPlayer;

    @Mock private DtmfLocalTonePlayer mDtmfLocalTonePlayer;

    @Mock private BluetoothStateReceiver mBluetoothStateReceiver;

    @Mock private TelecomSystem.SyncRoot mLock;



    private CallAudioManager mCallAudioManager;
@@ -81,6 +83,7 @@ public class CallAudioManagerTest extends TelecomTestCase { 
            }).when(mockInCallTonePlayer).startTone();

            return mockInCallTonePlayer;

        }).when(mPlayerFactory).createPlayer(anyInt());

        when(mCallsManager.getLock()).thenReturn(mLock);

        mCallAudioManager = new CallAudioManager(

                mCallAudioRouteStateMachine,

                mCallsManager,

src/com/android/server/telecom/Ringer.java

+1 −1

File changed.

Contains only whitespace changes.