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

Commit 218da167 authored by Jean-Michel Trivi's avatar Jean-Michel Trivi
Browse files

AudioService: fix audio mode lock 

Fix lock "sharing" for audio mode, with locking order as
1/ AudioDeviceBroker.mSetModeLock
2/ AudioDeviceBroker.mDeviceStateLock
3/ BtHelper class

The following code paths have been automatically generated
  as accessing BtHelper.requestScoState(int, int), which
  requires the lock to mSetModeLock. They have been checked
  for consistency of locks, and have been fixed and/or annotated:

ScoClient in BtHelper.decCount()  (com.android.server.audio)
    BtHelper.stopBluetoothScoForClient(IBinder, String)  (com.android.server.audio)
        AudioDeviceBroker.stopBluetoothScoForClient_Sync(IBinder, String)  (com.android.server.audio)
            AudioService.stopBluetoothSco(IBinder)  (com.android.server.audio)
ScoClient in BtHelper.incCount(int)  (com.android.server.audio)
    BtHelper.startBluetoothScoForClient(IBinder, int, String)  (com.android.server.audio)
        AudioDeviceBroker.startBluetoothScoForClient_Sync(IBinder, int, String)  (com.android.server.audio)
            AudioService.startBluetoothScoInt(IBinder, int, String)  (com.android.server.audio)
ScoClient in BtHelper.clearCount(boolean)  (com.android.server.audio)
    BtHelper.clearAllScoClients(int, boolean)  (com.android.server.audio)
        BtHelper.disconnectBluetoothSco(int)  (com.android.server.audio)
            BrokerHandler in AudioDeviceBroker.handleMessage(Message)  (com.android.server.audio)
        BtHelper.resetBluetoothSco()  (com.android.server.audio)
            BrokerHandler in AudioDeviceBroker.handleMessage(Message)  (com.android.server.audio)
            BtHelper.setBtScoActiveDevice(BluetoothDevice)  (com.android.server.audio)
                BtHelper.disconnectHeadset()  (com.android.server.audio)
                    BrokerHandler in AudioDeviceBroker.handleMessage(Message)  (com.android.server.audio)
                BtHelper.receiveBtEvent(Intent)  (com.android.server.audio)
                    AudioDeviceBroker.receiveBtEvent(Intent)  (com.android.server.audio)
                BtHelper.onHeadsetProfileConnected(BluetoothHeadset)  (com.android.server.audio)
                    BrokerHandler in AudioDeviceBroker.handleMessage(Message)  (com.android.server.audio)
            BtHelper.onSystemReady()  (com.android.server.audio)
                AudioDeviceBroker.onSystemReady()  (com.android.server.audio)
        BtHelper.receiveBtEvent(Intent)  (com.android.server.audio)
    BtHelper.scoClientDied(Object)  (com.android.server.audio)
        BrokerHandler in AudioDeviceBroker.handleMessage(Message)  (com.android.server.audio)

Bug: 123769055
Test: see bug
Change-Id: I5fbb5e8c56d69b8ccfc6b2f44b00169c6b75b632
parent 7e46f59a

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

+45 −14
Original line number Diff line number Diff line
@@ -114,10 +114,12 @@ import java.util.ArrayList; 
    // All post* methods are asynchronous



    /*package*/ void onSystemReady() {

        synchronized (mSetModeLock) {

            synchronized (mDeviceStateLock) {

                mBtHelper.onSystemReady();

            }

        }

    }



    /*package*/ void onAudioServerDied() {

        // Restore forced usage for communications and record
@@ -151,10 +153,12 @@ import java.util.ArrayList; 
     * @param intent

     */

    /*package*/ void receiveBtEvent(@NonNull Intent intent) {

        synchronized (mSetModeLock) {

            synchronized (mDeviceStateLock) {

                mBtHelper.receiveBtEvent(intent);

            }

        }

    }



    /*package*/ void setBluetoothA2dpOn_Async(boolean on, String source) {

        synchronized (mDeviceStateLock) {
@@ -350,14 +354,20 @@ import java.util.ArrayList; 
        sendLMsgNoDelay(MSG_L_A2DP_DEVICE_CONFIG_CHANGE, SENDMSG_QUEUE, device);

    }



    @GuardedBy("mSetModeLock")

    /*package*/ void startBluetoothScoForClient_Sync(IBinder cb, int scoAudioMode,

                @NonNull String eventSource) {

        synchronized (mDeviceStateLock) {

            mBtHelper.startBluetoothScoForClient(cb, scoAudioMode, eventSource);

        }

    }



    @GuardedBy("mSetModeLock")

    /*package*/ void stopBluetoothScoForClient_Sync(IBinder cb, @NonNull String eventSource) {

        synchronized (mDeviceStateLock) {

            mBtHelper.stopBluetoothScoForClient(cb, eventSource);

        }

    }



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

    // Communication with (to) AudioService
@@ -479,6 +489,10 @@ import java.util.ArrayList; 
                hearingAidProfile);

    }



    /*package*/ void postScoClientDied(Object obj) {

        sendLMsgNoDelay(MSG_L_SCOCLIENT_DIED, SENDMSG_QUEUE, obj);

    }



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

    // Method forwarding between the helper classes (BtHelper, AudioDeviceInventory)

    // only call from a "handle"* method or "on"* method
@@ -708,9 +722,11 @@ import java.util.ArrayList; 
                    }

                    break;

                case MSG_BT_HEADSET_CNCT_FAILED:

                    synchronized (mSetModeLock) {

                        synchronized (mDeviceStateLock) {

                            mBtHelper.resetBluetoothSco();

                        }

                    }

                    break;

                case MSG_IL_BTA2DP_DOCK_TIMEOUT:

                    // msg.obj  == address of BTA2DP device
@@ -742,9 +758,18 @@ import java.util.ArrayList; 
                    }

                    break;

                case MSG_I_DISCONNECT_BT_SCO:

                    synchronized (mSetModeLock) {

                        synchronized (mDeviceStateLock) {

                            mBtHelper.disconnectBluetoothSco(msg.arg1);

                        }

                    }

                    break;

                case MSG_L_SCOCLIENT_DIED:

                    synchronized (mSetModeLock) {

                        synchronized (mDeviceStateLock) {

                            mBtHelper.scoClientDied(msg.arg1);

                        }

                    }

                    break;

                case MSG_TOGGLE_HDMI:

                    synchronized (mDeviceStateLock) {
@@ -774,9 +799,11 @@ import java.util.ArrayList; 
                    }

                    break;

                case MSG_DISCONNECT_BT_HEADSET:

                    synchronized (mSetModeLock) {

                        synchronized (mDeviceStateLock) {

                            mBtHelper.disconnectHeadset();

                        }

                    }

                    break;

                case MSG_L_BT_SERVICE_CONNECTED_PROFILE_A2DP:

                    synchronized (mDeviceStateLock) {
@@ -794,9 +821,11 @@ import java.util.ArrayList; 
                    }

                    break;

                case MSG_L_BT_SERVICE_CONNECTED_PROFILE_HEADSET:

                    synchronized (mSetModeLock) {

                        synchronized (mDeviceStateLock) {

                            mBtHelper.onHeadsetProfileConnected((BluetoothHeadset) msg.obj);

                        }

                    }

                    break;

                case MSG_L_A2DP_DEVICE_CONNECTION_CHANGE_EXT: {

                    final BtDeviceConnectionInfo info = (BtDeviceConnectionInfo) msg.obj;
@@ -892,6 +921,8 @@ import java.util.ArrayList; 
    private static final int MSG_L_HEARING_AID_DEVICE_CONNECTION_CHANGE_EXT = 28;

    // process external command to (dis)connect or change active A2DP device

    private static final int MSG_L_A2DP_ACTIVE_DEVICE_CHANGE_EXT = 29;

    // a ScoClient died in BtHelper

    private static final int MSG_L_SCOCLIENT_DIED = 30;





    private static boolean isMessageHandledUnderWakelock(int msgId) {

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

+6 −2
Original line number Diff line number Diff line
@@ -3484,8 +3484,10 @@ 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){
@@ -3496,8 +3498,10 @@ 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() {

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

+108 −71
Original line number Diff line number Diff line
@@ -36,6 +36,8 @@ import android.os.UserHandle; 
import android.provider.Settings;

import android.util.Log;



import com.android.internal.annotations.GuardedBy;



import java.util.ArrayList;

import java.util.List;

import java.util.NoSuchElementException;
@@ -163,6 +165,8 @@ 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();
@@ -231,6 +235,8 @@ 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)) {
@@ -317,6 +323,8 @@ 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) {
@@ -325,6 +333,8 @@ 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);
@@ -344,6 +354,8 @@ 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);
@@ -401,6 +413,8 @@ public class BtHelper { 
        mDeviceBroker.postDisconnectHearingAid();

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void resetBluetoothSco() {

        clearAllScoClients(0, false);

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

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    /*package*/ synchronized void disconnectHeadset() {

        setBtScoActiveDevice(null);

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

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

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

        // Discard timeout message

        mDeviceBroker.handleCancelFailureToConnectToBtHeadsetService();
@@ -540,6 +558,9 @@ 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);

        final BluetoothDevice previousActiveDevice = mBluetoothHeadsetDevice;
@@ -621,6 +642,20 @@ 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");

        int index = mScoClients.indexOf(client);

        if (index < 0) {

            Log.w(TAG, "unregistered SCO client died");

        } else {

            client.clearCount(true);

            mScoClients.remove(client);

        }

    }



    private class ScoClient implements IBinder.DeathRecipient {

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

        private int mCreatorPid;
@@ -634,21 +669,14 @@ public class BtHelper { 


        @Override

        public void binderDied() {

            // this is the only place the implementation of ScoClient needs to be synchronized

            // on the instance, as all other methods are directly or indirectly called from

            // package-private methods, which are synchronized

            synchronized (BtHelper.this) {

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

                int index = mScoClients.indexOf(this);

                if (index < 0) {

                    Log.w(TAG, "unregistered SCO client died");

                } else {

                    clearCount(true);

                    mScoClients.remove(this);

                }

            }

            // process this from DeviceBroker's message queue to take the right locks since

            // this event can impact SCO mode and requires querying audio mode stack

            mDeviceBroker.postScoClientDied(this);

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void incCount(int scoAudioMode) {

            requestScoState(BluetoothHeadset.STATE_AUDIO_CONNECTED, scoAudioMode);

            if (mStartcount == 0) {
@@ -663,6 +691,9 @@ public class BtHelper { 
            mStartcount++;

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void decCount() {

            if (mStartcount == 0) {

                Log.w(TAG, "ScoClient.decCount() already 0");
@@ -679,6 +710,9 @@ public class BtHelper { 
            }

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void clearCount(boolean stopSco) {

            if (mStartcount != 0) {

                try {
@@ -714,6 +748,9 @@ public class BtHelper { 
            return count;

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        private void requestScoState(int state, int scoAudioMode) {

            checkScoAudioState();

            int clientCount = totalCount();
@@ -728,8 +765,6 @@ public class BtHelper { 
                broadcastScoConnectionState(AudioManager.SCO_AUDIO_STATE_CONNECTING);

                // Accept SCO audio activation only in NORMAL audio mode or if the mode is

                // currently controlled by the same client process.

                // TODO do not sync that way, see b/123769055

                synchronized (mDeviceBroker.mSetModeLock) {

                int modeOwnerPid =  mDeviceBroker.getSetModeDeathHandlers().isEmpty()

                        ? 0 : mDeviceBroker.getSetModeDeathHandlers().get(0).getPid();

                if (modeOwnerPid != 0 && (modeOwnerPid != mCreatorPid)) {
@@ -796,7 +831,6 @@ public class BtHelper { 
                        break;



                }

                }

            } else if (state == BluetoothHeadset.STATE_AUDIO_DISCONNECTED) {

                switch (mScoAudioState) {

                    case SCO_STATE_ACTIVE_INTERNAL:
@@ -906,6 +940,9 @@ public class BtHelper { 
        return null;

    }



    // @GuardedBy("AudioDeviceBroker.mSetModeLock")

    //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    @GuardedBy("BtHelper.this")

    private void clearAllScoClients(int exceptPid, boolean stopSco) {

        ScoClient savedClient = null;

        for (ScoClient cl : mScoClients) {