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

Commit 17cf02f3 authored by Jean-Michel Trivi's avatar Jean-Michel Trivi
Browse files

AudioService: simplify/document locking 

  In AudioDeviceBroker, mDeviceStateLock was the main lock and
was previously used almost as a global synchronization, present
on the handling of most messages, and numerous other methods
without actually protecting the only states it maintains. Note
that BtHelper and AudioDeviceInventory have their own sync
model, so they don't need to be protected by a lock from
AudioDeviceBroker.
  This CL renames mDeviceStateLock to mDeviceBrokerLock
specifically protecting:
- "force use" setting for communication
- Bluetooth A2DP enabled (coming from AudioManager API)
- all management of the message queue, both for enqueueing
  and removal (which was exposed to race condition since commit
  dc552e9d for A2DP (dis)connection.
It also removes the lock used to synchronize the audio mode
owner with the SCO operations: the only information AudioDeviceBroker
needed from AudioService regarding audio mode was the PID of the
mode owner. The new code uses an atomic integer to store/provide
this information, instead of relying on a shared lock amongst
multiple classes.
  Add test for behavior where media gets unmuted if it was muted
when the user connects an A2DP headset.

Bug: 140200704
Test: atest AudioDeviceBrokerTest ; atest AudioManagerTest
Change-Id: I555145c9673fc634ec0c2af9da718e084f3ac067
parent 681b432e

services/core/java/com/android/server/audio/AudioDeviceBroker.java

+247 −256

File changed.

Preview size limit exceeded, changes collapsed.

services/core/java/com/android/server/audio/AudioService.java

+35 −35
Original line number Diff line number Diff line
@@ -469,12 +469,11 @@ public class AudioService extends IAudioService.Stub 


    // List of binder death handlers for setMode() client processes.

    // The last process to have called setMode() is at the top of the list.

    // package-private so it can be accessed in AudioDeviceBroker.getSetModeDeathHandlers

    //TODO candidate to be moved to separate class that handles synchronization

    @GuardedBy("mDeviceBroker.mSetModeLock")

    /*package*/ final ArrayList<SetModeDeathHandler> mSetModeDeathHandlers =

    private final ArrayList<SetModeDeathHandler> mSetModeDeathHandlers =

            new ArrayList<SetModeDeathHandler>();



    private volatile int mCurrentModeOwnerPid = 0;



    // true if boot sequence has been completed

    private boolean mSystemReady;

    // true if Intent.ACTION_USER_SWITCHED has ever been received
@@ -3150,15 +3149,10 @@ public class AudioService extends IAudioService.Stub 
     * @return 0 if nobody owns the mode

     */

    /*package*/ int getModeOwnerPid() {

        int modeOwnerPid = 0;

        try {

            modeOwnerPid = mSetModeDeathHandlers.get(0).getPid();

        } catch (Exception e) {

            // nothing to do, modeOwnerPid is not modified

        }

        return modeOwnerPid;

        return  mCurrentModeOwnerPid;

    }





    private class SetModeDeathHandler implements IBinder.DeathRecipient {

        private IBinder mCb; // To be notified of client's death

        private int mPid;
@@ -3172,7 +3166,7 @@ public class AudioService extends IAudioService.Stub 
        public void binderDied() {

            int oldModeOwnerPid = 0;

            int newModeOwnerPid = 0;

            synchronized (mDeviceBroker.mSetModeLock) {

            synchronized (mSetModeDeathHandlers) {

                Log.w(TAG, "setMode() client died");

                if (!mSetModeDeathHandlers.isEmpty()) {

                    oldModeOwnerPid = mSetModeDeathHandlers.get(0).getPid();
@@ -3183,13 +3177,17 @@ public class AudioService extends IAudioService.Stub 
                } else {

                    newModeOwnerPid = setModeInt(AudioSystem.MODE_NORMAL, mCb, mPid, TAG);

                }

            }

            // when entering RINGTONE, IN_CALL or IN_COMMUNICATION mode, clear all

            // SCO connections not started by the application changing the mode when pid changes

            if ((newModeOwnerPid != oldModeOwnerPid) && (newModeOwnerPid != 0)) {



                if (newModeOwnerPid != oldModeOwnerPid) {

                    mCurrentModeOwnerPid = newModeOwnerPid;

                    // when entering RINGTONE, IN_CALL or IN_COMMUNICATION mode, clear all SCO

                    // connections not started by the application changing the mode when pid changes

                    if (newModeOwnerPid != 0) {

                        mDeviceBroker.postDisconnectBluetoothSco(newModeOwnerPid);

                    }

                }

            }

        }



        public int getPid() {

            return mPid;
@@ -3210,13 +3208,15 @@ public class AudioService extends IAudioService.Stub 


    /** @see AudioManager#setMode(int) */

    public void setMode(int mode, IBinder cb, String callingPackage) {

        if (DEBUG_MODE) { Log.v(TAG, "setMode(mode=" + mode + ", callingPackage=" + callingPackage + ")"); }

        if (DEBUG_MODE) {

            Log.v(TAG, "setMode(mode=" + mode + ", callingPackage=" + callingPackage + ")");

        }

        if (!checkAudioSettingsPermission("setMode()")) {

            return;

        }



        if ( (mode == AudioSystem.MODE_IN_CALL) &&

                (mContext.checkCallingOrSelfPermission(

        if ((mode == AudioSystem.MODE_IN_CALL)

                && (mContext.checkCallingOrSelfPermission(

                        android.Manifest.permission.MODIFY_PHONE_STATE)

                        != PackageManager.PERMISSION_GRANTED)) {

            Log.w(TAG, "MODIFY_PHONE_STATE Permission Denial: setMode(MODE_IN_CALL) from pid="
@@ -3230,7 +3230,7 @@ public class AudioService extends IAudioService.Stub 


        int oldModeOwnerPid = 0;

        int newModeOwnerPid = 0;

        synchronized (mDeviceBroker.mSetModeLock) {

        synchronized (mSetModeDeathHandlers) {

            if (!mSetModeDeathHandlers.isEmpty()) {

                oldModeOwnerPid = mSetModeDeathHandlers.get(0).getPid();

            }
@@ -3238,17 +3238,21 @@ public class AudioService extends IAudioService.Stub 
                mode = mMode;

            }

            newModeOwnerPid = setModeInt(mode, cb, Binder.getCallingPid(), callingPackage);

        }



            if (newModeOwnerPid != oldModeOwnerPid) {

                mCurrentModeOwnerPid = newModeOwnerPid;

                // when entering RINGTONE, IN_CALL or IN_COMMUNICATION mode, clear all

                // SCO connections not started by the application changing the mode when pid changes

        if ((newModeOwnerPid != oldModeOwnerPid) && (newModeOwnerPid != 0)) {

                if (newModeOwnerPid != 0) {

                    mDeviceBroker.postDisconnectBluetoothSco(newModeOwnerPid);

                }

            }

        }

    }



    // setModeInt() returns a valid PID if the audio mode was successfully set to

    // any mode other than NORMAL.

    @GuardedBy("mDeviceBroker.mSetModeLock")

    @GuardedBy("mSetModeDeathHandlers")

    private int setModeInt(int mode, IBinder cb, int pid, String caller) {

        if (DEBUG_MODE) { Log.v(TAG, "setModeInt(mode=" + mode + ", pid=" + pid + ", caller="

                + caller + ")"); }
@@ -3587,10 +3591,8 @@ public class AudioService extends IAudioService.Stub 
                !mSystemReady) {

            return;

        }

        synchronized (mDeviceBroker.mSetModeLock) {

        mDeviceBroker.startBluetoothScoForClient_Sync(cb, scoAudioMode, eventSource);

    }

    }



    /** @see AudioManager#stopBluetoothSco() */

    public void stopBluetoothSco(IBinder cb){
@@ -3601,10 +3603,8 @@ public class AudioService extends IAudioService.Stub 
        final String eventSource =  new StringBuilder("stopBluetoothSco()")

                .append(") from u/pid:").append(Binder.getCallingUid()).append("/")

                .append(Binder.getCallingPid()).toString();

        synchronized (mDeviceBroker.mSetModeLock) {

        mDeviceBroker.stopBluetoothScoForClient_Sync(cb, eventSource);

    }

    }





    /*package*/ ContentResolver getContentResolver() {
@@ -4352,7 +4352,7 @@ public class AudioService extends IAudioService.Stub 


    // NOTE: Locking order for synchronized objects related to volume or ringer mode management:

    //  1 mScoclient OR mSafeMediaVolumeState

    //  2   mSetModeLock

    //  2   mSetModeDeathHandlers

    //  3     mSettingsLock

    //  4       VolumeStreamState.class

    private class VolumeStreamState {

services/core/java/com/android/server/audio/BtHelper.java

+0 −30
Original line number Diff line number Diff line
@@ -171,8 +171,6 @@ public class BtHelper { 
    //----------------------------------------------------------------------

    // Interface for AudioDeviceBroker



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void onSystemReady() {

        mScoConnectionState = android.media.AudioManager.SCO_AUDIO_STATE_ERROR;

        resetBluetoothSco();
@@ -245,8 +243,6 @@ public class BtHelper { 
        return mapBluetoothCodecToAudioFormat(btCodecConfig.getCodecType());

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void receiveBtEvent(Intent intent) {

        final String action = intent.getAction();

        if (action.equals(BluetoothHeadset.ACTION_ACTIVE_DEVICE_CHANGED)) {
@@ -333,8 +329,6 @@ public class BtHelper { 
     *

     * @param exceptPid pid whose SCO connections through {@link AudioManager} should be kept

     */

    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void disconnectBluetoothSco(int exceptPid) {

        checkScoAudioState();

        if (mScoAudioState == SCO_STATE_ACTIVE_EXTERNAL) {
@@ -343,8 +337,6 @@ public class BtHelper { 
        clearAllScoClients(exceptPid, true);

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void startBluetoothScoForClient(IBinder cb, int scoAudioMode,

                @NonNull String eventSource) {

        ScoClient client = getScoClient(cb, true);
@@ -364,8 +356,6 @@ public class BtHelper { 
        Binder.restoreCallingIdentity(ident);

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void stopBluetoothScoForClient(IBinder cb,

            @NonNull String eventSource) {

        ScoClient client = getScoClient(cb, false);
@@ -423,8 +413,6 @@ public class BtHelper { 
        mDeviceBroker.postDisconnectHearingAid();

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void resetBluetoothSco() {

        clearAllScoClients(0, false);

        mScoAudioState = SCO_STATE_INACTIVE;
@@ -433,8 +421,6 @@ public class BtHelper { 
        mDeviceBroker.setBluetoothScoOn(false, "resetBluetoothSco");

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void disconnectHeadset() {

        setBtScoActiveDevice(null);

        mBluetoothHeadset = null;
@@ -480,8 +466,6 @@ public class BtHelper { 
                /*eventSource*/ "mBluetoothProfileServiceListener");

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void onHeadsetProfileConnected(BluetoothHeadset headset) {

        // Discard timeout message

        mDeviceBroker.handleCancelFailureToConnectToBtHeadsetService();
@@ -568,8 +552,6 @@ public class BtHelper { 
        return result;

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    @GuardedBy("BtHelper.this")

    private void setBtScoActiveDevice(BluetoothDevice btDevice) {

        Log.i(TAG, "setBtScoActiveDevice: " + mBluetoothHeadsetDevice + " -> " + btDevice);
@@ -652,8 +634,6 @@ public class BtHelper { 
            };



    //----------------------------------------------------------------------

    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void scoClientDied(Object obj) {

        final ScoClient client = (ScoClient) obj;

        Log.w(TAG, "SCO client died");
@@ -684,8 +664,6 @@ public class BtHelper { 
            mDeviceBroker.postScoClientDied(this);

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void incCount(int scoAudioMode) {

            if (!requestScoState(BluetoothHeadset.STATE_AUDIO_CONNECTED, scoAudioMode)) {
@@ -705,8 +683,6 @@ public class BtHelper { 
            mStartcount++;

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void decCount() {

            if (mStartcount == 0) {
@@ -726,8 +702,6 @@ public class BtHelper { 
            }

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void clearCount(boolean stopSco) {

            if (mStartcount != 0) {
@@ -764,8 +738,6 @@ public class BtHelper { 
            return count;

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        private boolean requestScoState(int state, int scoAudioMode) {

            checkScoAudioState();
@@ -959,8 +931,6 @@ public class BtHelper { 
        return null;

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    @GuardedBy("BtHelper.this")

    private void clearAllScoClients(int exceptPid, boolean stopSco) {

        ScoClient savedClient = null;

services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java

+16 −0
Original line number Diff line number Diff line
@@ -126,6 +126,22 @@ public class AudioDeviceBrokerTest { 
        doTestConnectionDisconnectionReconnection(AudioService.BECOMING_NOISY_DELAY_MS / 2);

    }



    /**

     * Verify connecting an A2DP sink will call into AudioService to unmute media

     */

    @Test

    public void testA2dpConnectionUnmutesMedia() throws Exception {

        Log.i(TAG, "testA2dpConnectionUnmutesMedia");

        Assert.assertNotNull("invalid null BT device", mFakeBtDevice);



        mAudioDeviceBroker.postBluetoothA2dpDeviceConnectionStateSuppressNoisyIntent(mFakeBtDevice,

                BluetoothProfile.STATE_CONNECTED, BluetoothProfile.A2DP, true, 1);

        Thread.sleep(MAX_MESSAGE_HANDLING_DELAY_MS);

        verify(mMockAudioService, times(1)).postAccessoryPlugMediaUnmute(

                ArgumentMatchers.eq(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP));



    }



    private void doTestConnectionDisconnectionReconnection(int delayAfterDisconnection)

            throws Exception {

        when(mMockAudioService.getDeviceForStream(AudioManager.STREAM_MUSIC))