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

Commit feb9b897 authored by Seulki Shin's avatar Seulki Shin Committed by Eric Laurent
Browse files

audioservice: fix not able to disconnect VoIP call 



startBluetoothSco() is not safe enough from subseqent method call which
was found on the Wechat. For instance, VoIP call will not be terminated
properly in the following call flows:
startBluetoothSco() (-> incCount) ->
startBluetoothSco() (-> incCount) ->
stopBluetoothSco() (-> decCount) -> at this point, it cannot reached to
disconnectBluetoothScoAudioHelper() due to mStartCount is not 0 from
requestScoState()

To fix this, mStartCount was removed because it's usage is unclear.
Also incCount, decCount was removed because it makes difficult to
understand what the purpose of those functions from their name.

Bug: 150312620
Test: manual test cell and VoIP calls with BT SCO HS.

Change-Id: Iabc44795b8a87c10b00d84c67de9938bef385841
Signed-off-by: default avatarslki.shin <slki.shin@samsung.com>
parent 8a45b969

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

+1 −1
Original line number Diff line number Diff line
@@ -324,7 +324,7 @@ import java.io.PrintWriter; 
    }



    /*package*/ void setBluetoothScoOn(boolean on, String eventSource) {

        //Log.i(TAG, "setBluetoothScoOnInt: " + on + " " + eventSource);

        //Log.i(TAG, "setBluetoothScoOn: " + on + " " + eventSource);

        synchronized (mDeviceStateLock) {

            if (on) {

                // do not accept SCO ON if SCO audio is not connected

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

+50 −95
Original line number Diff line number Diff line
@@ -58,6 +58,7 @@ public class BtHelper { 
    }



    // List of clients having issued a SCO start request

    @GuardedBy("BtHelper.this")

    private final @NonNull ArrayList<ScoClient> mScoClients = new ArrayList<ScoClient>();



    // BluetoothHeadset API to control SCO connection
@@ -356,9 +357,8 @@ public class BtHelper { 
        // client is created.

        final long ident = Binder.clearCallingIdentity();

        try {

            eventSource += " client count before=" + client.getCount();

            AudioService.sDeviceLogger.log(new AudioEventLogger.StringEvent(eventSource));

            client.incCount(scoAudioMode);

            client.requestScoState(BluetoothHeadset.STATE_AUDIO_CONNECTED, scoAudioMode);

        } catch (NullPointerException e) {

            Log.e(TAG, "Null ScoClient", e);

        }
@@ -375,9 +375,15 @@ public class BtHelper { 
        // and this must be done on behalf of system server to make sure permissions are granted.

        final long ident = Binder.clearCallingIdentity();

        if (client != null) {

            eventSource += " client count before=" + client.getCount();

            AudioService.sDeviceLogger.log(new AudioEventLogger.StringEvent(eventSource));

            client.decCount();

            client.requestScoState(BluetoothHeadset.STATE_AUDIO_DISCONNECTED,

                    SCO_MODE_VIRTUAL_CALL);

            // If a disconnection is pending, the client will be removed whne clearAllScoClients()

            // is called form receiveBtEvent()

            if (mScoAudioState != SCO_STATE_DEACTIVATE_REQ

                    && mScoAudioState != SCO_STATE_DEACTIVATING) {

                client.remove(false /*stop */, true /*unregister*/);

            }

        }

        Binder.restoreCallingIdentity(ident);

    }
@@ -657,96 +663,40 @@ public class BtHelper { 
    @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

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

        final ScoClient client = (ScoClient) obj;

        client.remove(true /*stop*/, false /*unregister*/);

        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;

        private int mStartcount; // number of SCO connections started by this client



        ScoClient(IBinder cb) {

            mCb = cb;

            mCreatorPid = Binder.getCallingPid();

            mStartcount = 0;

        }



        @Override

        public void binderDied() {

            // 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) {

            if (!requestScoState(BluetoothHeadset.STATE_AUDIO_CONNECTED, scoAudioMode)) {

                Log.e(TAG, "Request sco connected with scoAudioMode("

                        + scoAudioMode + ") failed");

                return;

            }

            if (mStartcount == 0) {

        public void registerDeathRecipient() {

            try {

                mCb.linkToDeath(this, 0);

            } catch (RemoteException e) {

                    // client has already died!

                    Log.w(TAG, "ScoClient  incCount() could not link to "

                            + mCb + " binder death");

                }

            }

            mStartcount++;

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void decCount() {

            if (mStartcount == 0) {

                Log.w(TAG, "ScoClient.decCount() already 0");

            } else {

                mStartcount--;

                if (mStartcount == 0) {

                    try {

                        mCb.unlinkToDeath(this, 0);

                    } catch (NoSuchElementException e) {

                        Log.w(TAG, "decCount() going to 0 but not registered to binder");

                    }

                }

                if (!requestScoState(BluetoothHeadset.STATE_AUDIO_DISCONNECTED, 0)) {

                    Log.w(TAG, "Request sco disconnected with scoAudioMode(0) failed");

                }

                Log.w(TAG, "ScoClient could not link to " + mCb + " binder death");

            }

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        // @GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        void clearCount(boolean stopSco) {

            if (mStartcount != 0) {

        public void unregisterDeathRecipient() {

            try {

                mCb.unlinkToDeath(this, 0);

            } catch (NoSuchElementException e) {

                    Log.w(TAG, "clearCount() mStartcount: "

                            + mStartcount + " != 0 but not registered to binder");

                }

            }

            mStartcount = 0;

            if (stopSco) {

                requestScoState(BluetoothHeadset.STATE_AUDIO_DISCONNECTED, 0);

                Log.w(TAG, "ScoClient could not not unregistered to binder");

            }

        }



        int getCount() {

            return mStartcount;

        @Override

        public void binderDied() {

            // 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);

        }



        IBinder getBinder() {
@@ -757,23 +707,14 @@ public class BtHelper { 
            return mCreatorPid;

        }



        private int totalCount() {

            int count = 0;

            for (ScoClient mScoClient : mScoClients) {

                count += mScoClient.getCount();

            }

            return count;

        }



        // @GuardedBy("AudioDeviceBroker.mSetModeLock")

        //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

        @GuardedBy("BtHelper.this")

        private boolean requestScoState(int state, int scoAudioMode) {

            checkScoAudioState();

            int clientCount = totalCount();

            if (clientCount != 0) {

            if (mScoClients.size() != 1) {

                Log.i(TAG, "requestScoState: state=" + state + ", scoAudioMode=" + scoAudioMode

                        + ", clientCount=" + clientCount);

                        + ", num SCO clients=" + mScoClients.size());

                return true;

            }

            if (state == BluetoothHeadset.STATE_AUDIO_CONNECTED) {
@@ -842,12 +783,14 @@ public class BtHelper { 
                        mScoAudioState = SCO_STATE_ACTIVE_INTERNAL;

                        broadcastScoConnectionState(AudioManager.SCO_AUDIO_STATE_CONNECTED);

                        break;

                    case SCO_STATE_ACTIVE_INTERNAL:

                        Log.w(TAG, "requestScoState: already in ACTIVE mode, simply return");

                        break;

                    default:

                        Log.w(TAG, "requestScoState: failed to connect in state "

                                + mScoAudioState + ", scoAudioMode=" + scoAudioMode);

                        broadcastScoConnectionState(AudioManager.SCO_AUDIO_STATE_DISCONNECTED);

                        return false;



                }

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

                switch (mScoAudioState) {
@@ -893,6 +836,18 @@ public class BtHelper { 
            }

            return true;

        }



        @GuardedBy("BtHelper.this")

        void remove(boolean stop, boolean unregister) {

            if (unregister) {

                unregisterDeathRecipient();

            }

            if (stop) {

                requestScoState(BluetoothHeadset.STATE_AUDIO_DISCONNECTED,

                        SCO_MODE_VIRTUAL_CALL);

            }

            mScoClients.remove(this);

        }

    }



    //-----------------------------------------------------
@@ -946,6 +901,7 @@ public class BtHelper { 
    }





    @GuardedBy("BtHelper.this")

    private ScoClient getScoClient(IBinder cb, boolean create) {

        for (ScoClient existingClient : mScoClients) {

            if (existingClient.getBinder() == cb) {
@@ -954,6 +910,7 @@ public class BtHelper { 
        }

        if (create) {

            ScoClient newClient = new ScoClient(cb);

            newClient.registerDeathRecipient();

            mScoClients.add(newClient);

            return newClient;

        }
@@ -964,18 +921,16 @@ public class BtHelper { 
    //@GuardedBy("AudioDeviceBroker.mDeviceStateLock")

    @GuardedBy("BtHelper.this")

    private void clearAllScoClients(int exceptPid, boolean stopSco) {

        ScoClient savedClient = null;

        final ArrayList<ScoClient> clients = new ArrayList<ScoClient>();

        for (ScoClient cl : mScoClients) {

            if (cl.getPid() != exceptPid) {

                cl.clearCount(stopSco);

            } else {

                savedClient = cl;

                clients.add(cl);

            }

        }

        mScoClients.clear();

        if (savedClient != null) {

            mScoClients.add(savedClient);

        for (ScoClient cl : clients) {

            cl.remove(stopSco, true /*unregister*/);

        }



    }



    private boolean getBluetoothHeadset() {