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

Commit 16eeac35 authored by Jack He's avatar Jack He Committed by Andre Eisenbach
Browse files

Bluetooth: Thread-safe binder invocation 

* Binder object may become null between null check and actual invocation
  if using a instance private variable assignable by service connection
  callbacks
* The solution to this problem without locking is to assign existing
  binder variable to a local final variable before the null check
* Any further invocation to a disconnected binder object will result in
  RemoteException that is caught by the try-catch block
* Read and write to volatile variable is always atomic and hence thread-safe
* Removed unnecessary synchronization in BluetoothAdapter constructor
* Private mConnection objects should be final
* Simplfied several return statements where booleans can be returned
  directly
* Removed unnecessary catches for NPE since there won't be any

Bug: 64724692
Test: make, pair and use devices, no functional change
Change-Id: Ifc9d6337c0d451a01484b61243230725d5314f8e
parent 7d2219d1

core/java/android/bluetooth/BluetoothA2dpSink.java

+40 −41
Original line number Diff line number Diff line
@@ -125,7 +125,7 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 


    private Context mContext;

    private ServiceListener mServiceListener;

    private IBluetoothA2dpSink mService;

    private volatile IBluetoothA2dpSink mService;

    private BluetoothAdapter mAdapter;



    private final IBluetoothStateChangeCallback mBluetoothStateChangeCallback =
@@ -240,15 +240,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

    public boolean connect(BluetoothDevice device) {

        if (DBG) log("connect(" + device + ")");

        if (mService != null && isEnabled() && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.connect(device);

                return service.connect(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return false;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return false;

    }
@@ -279,15 +280,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

    public boolean disconnect(BluetoothDevice device) {

        if (DBG) log("disconnect(" + device + ")");

        if (mService != null && isEnabled() && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.disconnect(device);

                return service.disconnect(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return false;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return false;

    }
@@ -297,15 +299,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
    @Override

    public List<BluetoothDevice> getConnectedDevices() {

        if (VDBG) log("getConnectedDevices()");

        if (mService != null && isEnabled()) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled()) {

            try {

                return mService.getConnectedDevices();

                return service.getConnectedDevices();

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return new ArrayList<BluetoothDevice>();

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return new ArrayList<BluetoothDevice>();

    }
@@ -315,15 +318,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
    @Override

    public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {

        if (VDBG) log("getDevicesMatchingStates()");

        if (mService != null && isEnabled()) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled()) {

            try {

                return mService.getDevicesMatchingConnectionStates(states);

                return service.getDevicesMatchingConnectionStates(states);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return new ArrayList<BluetoothDevice>();

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return new ArrayList<BluetoothDevice>();

    }
@@ -333,16 +337,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
    @Override

    public int getConnectionState(BluetoothDevice device) {

        if (VDBG) log("getState(" + device + ")");

        if (mService != null && isEnabled()

                && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.getConnectionState(device);

                return service.getConnectionState(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return BluetoothProfile.STATE_DISCONNECTED;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return BluetoothProfile.STATE_DISCONNECTED;

    }
@@ -359,16 +363,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

    public BluetoothAudioConfig getAudioConfig(BluetoothDevice device) {

        if (VDBG) log("getAudioConfig(" + device + ")");

        if (mService != null && isEnabled()

                && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.getAudioConfig(device);

                return service.getAudioConfig(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return null;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return null;

    }
@@ -389,20 +393,20 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

    public boolean setPriority(BluetoothDevice device, int priority) {

        if (DBG) log("setPriority(" + device + ", " + priority + ")");

        if (mService != null && isEnabled()

                && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            if (priority != BluetoothProfile.PRIORITY_OFF

                    && priority != BluetoothProfile.PRIORITY_ON) {

                return false;

            }

            try {

                return mService.setPriority(device, priority);

                return service.setPriority(device, priority);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return false;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return false;

    }
@@ -421,16 +425,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

    public int getPriority(BluetoothDevice device) {

        if (VDBG) log("getPriority(" + device + ")");

        if (mService != null && isEnabled()

                && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.getPriority(device);

                return service.getPriority(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return BluetoothProfile.PRIORITY_OFF;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return BluetoothProfile.PRIORITY_OFF;

    }
@@ -442,16 +446,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     * @param device BluetoothDevice device

     */

    public boolean isA2dpPlaying(BluetoothDevice device) {

        if (mService != null && isEnabled()

                && isValidDevice(device)) {

        final IBluetoothA2dpSink service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.isA2dpPlaying(device);

                return service.isA2dpPlaying(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return false;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return false;

    }
@@ -485,7 +489,6 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
        public void onServiceConnected(ComponentName className, IBinder service) {

            if (DBG) Log.d(TAG, "Proxy object connected");

            mService = IBluetoothA2dpSink.Stub.asInterface(Binder.allowBlocking(service));



            if (mServiceListener != null) {

                mServiceListener.onServiceConnected(BluetoothProfile.A2DP_SINK,

                        BluetoothA2dpSink.this);
@@ -502,15 +505,11 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
    };



    private boolean isEnabled() {

        if (mAdapter.getState() == BluetoothAdapter.STATE_ON) return true;

        return false;

        return mAdapter.getState() == BluetoothAdapter.STATE_ON;

    }



    private boolean isValidDevice(BluetoothDevice device) {

        if (device == null) return false;



        if (BluetoothAdapter.checkBluetoothAddress(device.getAddress())) return true;

        return false;

    private static boolean isValidDevice(BluetoothDevice device) {

        return device != null && BluetoothAdapter.checkBluetoothAddress(device.getAddress());

    }



    private static void log(String msg) {

core/java/android/bluetooth/BluetoothAvrcpController.java

+27 −27
Original line number Diff line number Diff line
@@ -81,7 +81,7 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 


    private Context mContext;

    private ServiceListener mServiceListener;

    private IBluetoothAvrcpController mService;

    private volatile IBluetoothAvrcpController mService;

    private BluetoothAdapter mAdapter;



    private final IBluetoothStateChangeCallback mBluetoothStateChangeCallback =
@@ -179,15 +179,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
    @Override

    public List<BluetoothDevice> getConnectedDevices() {

        if (VDBG) log("getConnectedDevices()");

        if (mService != null && isEnabled()) {

        final IBluetoothAvrcpController service = mService;

        if (service != null && isEnabled()) {

            try {

                return mService.getConnectedDevices();

                return service.getConnectedDevices();

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return new ArrayList<BluetoothDevice>();

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return new ArrayList<BluetoothDevice>();

    }
@@ -197,15 +198,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
    @Override

    public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {

        if (VDBG) log("getDevicesMatchingStates()");

        if (mService != null && isEnabled()) {

        final IBluetoothAvrcpController service = mService;

        if (service != null && isEnabled()) {

            try {

                return mService.getDevicesMatchingConnectionStates(states);

                return service.getDevicesMatchingConnectionStates(states);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return new ArrayList<BluetoothDevice>();

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return new ArrayList<BluetoothDevice>();

    }
@@ -215,16 +217,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
    @Override

    public int getConnectionState(BluetoothDevice device) {

        if (VDBG) log("getState(" + device + ")");

        if (mService != null && isEnabled()

                && isValidDevice(device)) {

        final IBluetoothAvrcpController service = mService;

        if (service != null && isEnabled() && isValidDevice(device)) {

            try {

                return mService.getConnectionState(device);

                return service.getConnectionState(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Stack:" + Log.getStackTraceString(new Throwable()));

                return BluetoothProfile.STATE_DISCONNECTED;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return BluetoothProfile.STATE_DISCONNECTED;

    }
@@ -236,9 +238,10 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
    public BluetoothAvrcpPlayerSettings getPlayerSettings(BluetoothDevice device) {

        if (DBG) Log.d(TAG, "getPlayerSettings");

        BluetoothAvrcpPlayerSettings settings = null;

        if (mService != null && isEnabled()) {

        final IBluetoothAvrcpController service = mService;

        if (service != null && isEnabled()) {

            try {

                settings = mService.getPlayerSettings(device);

                settings = service.getPlayerSettings(device);

            } catch (RemoteException e) {

                Log.e(TAG, "Error talking to BT service in getMetadata() " + e);

                return null;
@@ -253,15 +256,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
     */

    public boolean setPlayerApplicationSetting(BluetoothAvrcpPlayerSettings plAppSetting) {

        if (DBG) Log.d(TAG, "setPlayerApplicationSetting");

        if (mService != null && isEnabled()) {

        final IBluetoothAvrcpController service = mService;

        if (service != null && isEnabled()) {

            try {

                return mService.setPlayerApplicationSetting(plAppSetting);

                return service.setPlayerApplicationSetting(plAppSetting);

            } catch (RemoteException e) {

                Log.e(TAG, "Error talking to BT service in setPlayerApplicationSetting() " + e);

                return false;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

        return false;

    }
@@ -272,23 +276,23 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
    public void sendGroupNavigationCmd(BluetoothDevice device, int keyCode, int keyState) {

        Log.d(TAG, "sendGroupNavigationCmd dev = " + device + " key " + keyCode + " State = "

                + keyState);

        if (mService != null && isEnabled()) {

        final IBluetoothAvrcpController service = mService;

        if (service != null && isEnabled()) {

            try {

                mService.sendGroupNavigationCmd(device, keyCode, keyState);

                service.sendGroupNavigationCmd(device, keyCode, keyState);

                return;

            } catch (RemoteException e) {

                Log.e(TAG, "Error talking to BT service in sendGroupNavigationCmd()", e);

                return;

            }

        }

        if (mService == null) Log.w(TAG, "Proxy not attached to service");

        if (service == null) Log.w(TAG, "Proxy not attached to service");

    }



    private final ServiceConnection mConnection = new ServiceConnection() {

        public void onServiceConnected(ComponentName className, IBinder service) {

            if (DBG) Log.d(TAG, "Proxy object connected");

            mService = IBluetoothAvrcpController.Stub.asInterface(Binder.allowBlocking(service));



            if (mServiceListener != null) {

                mServiceListener.onServiceConnected(BluetoothProfile.AVRCP_CONTROLLER,

                        BluetoothAvrcpController.this);
@@ -305,15 +309,11 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
    };



    private boolean isEnabled() {

        if (mAdapter.getState() == BluetoothAdapter.STATE_ON) return true;

        return false;

        return mAdapter.getState() == BluetoothAdapter.STATE_ON;

    }



    private boolean isValidDevice(BluetoothDevice device) {

        if (device == null) return false;



        if (BluetoothAdapter.checkBluetoothAddress(device.getAddress())) return true;

        return false;

    private static boolean isValidDevice(BluetoothDevice device) {

        return device != null && BluetoothAdapter.checkBluetoothAddress(device.getAddress());

    }



    private static void log(String msg) {

core/java/android/bluetooth/BluetoothDevice.java

+86 −57

File changed.

Preview size limit exceeded, changes collapsed.

core/java/android/bluetooth/BluetoothHeadset.java

+102 −79

File changed.

Preview size limit exceeded, changes collapsed.

core/java/android/bluetooth/BluetoothHeadsetClient.java

+105 −83

File changed.

Preview size limit exceeded, changes collapsed.