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

Commit 440e81f7 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
(cherry picked from commit 1f686f64)
parent 7ec636e0

framework/java/android/bluetooth/BluetoothA2dpSink.java

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




    private Context mContext;

    private Context mContext;

    private ServiceListener mServiceListener;

    private ServiceListener mServiceListener;

    private IBluetoothA2dpSink mService;

    private volatile IBluetoothA2dpSink mService;

    private BluetoothAdapter mAdapter;

    private BluetoothAdapter mAdapter;





    final private IBluetoothStateChangeCallback mBluetoothStateChangeCallback =

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

     */

    public boolean connect(BluetoothDevice device) {

    public boolean connect(BluetoothDevice device) {

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

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

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

        final IBluetoothA2dpSink service = mService;

            isValidDevice(device)) {

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

            try {

            try {

                return mService.connect(device);

                return service.connect(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return false;

                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;

        return false;

    }

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

     */

    public boolean disconnect(BluetoothDevice device) {

    public boolean disconnect(BluetoothDevice device) {

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

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

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

        final IBluetoothA2dpSink service = mService;

            isValidDevice(device)) {

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

            try {

            try {

                return mService.disconnect(device);

                return service.disconnect(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return false;

                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;

        return false;

    }

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

     */

    public List<BluetoothDevice> getConnectedDevices() {

    public List<BluetoothDevice> getConnectedDevices() {

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

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

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

        final IBluetoothA2dpSink service = mService;

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

            try {

            try {

                return mService.getConnectedDevices();

                return service.getConnectedDevices();

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return new ArrayList<BluetoothDevice>();

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

        return new ArrayList<BluetoothDevice>();

    }

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

     */

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

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

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

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

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

        final IBluetoothA2dpSink service = mService;

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

            try {

            try {

                return mService.getDevicesMatchingConnectionStates(states);

                return service.getDevicesMatchingConnectionStates(states);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return new ArrayList<BluetoothDevice>();

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

        return new ArrayList<BluetoothDevice>();

    }

    }
@@ -332,16 +334,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

     */

    public int getConnectionState(BluetoothDevice device) {

    public int getConnectionState(BluetoothDevice device) {

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

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

        if (mService != null && isEnabled()

        final IBluetoothA2dpSink service = mService;

            && isValidDevice(device)) {

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

            try {

            try {

                return mService.getConnectionState(device);

                return service.getConnectionState(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return BluetoothProfile.STATE_DISCONNECTED;

                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;

        return BluetoothProfile.STATE_DISCONNECTED;

    }

    }
@@ -358,16 +360,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

     */

          public BluetoothAudioConfig getAudioConfig(BluetoothDevice device) {

          public BluetoothAudioConfig getAudioConfig(BluetoothDevice device) {

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

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

        if (mService != null && isEnabled()

        final IBluetoothA2dpSink service = mService;

            && isValidDevice(device)) {

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

            try {

            try {

                return mService.getAudioConfig(device);

                return service.getAudioConfig(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return null;

                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;

        return null;

    }

    }
@@ -388,20 +390,20 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

     */

    public boolean setPriority(BluetoothDevice device, int priority) {

    public boolean setPriority(BluetoothDevice device, int priority) {

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

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

        if (mService != null && isEnabled()

        final IBluetoothA2dpSink service = mService;

            && isValidDevice(device)) {

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

            if (priority != BluetoothProfile.PRIORITY_OFF &&

            if (priority != BluetoothProfile.PRIORITY_OFF

                priority != BluetoothProfile.PRIORITY_ON){

                    && priority != BluetoothProfile.PRIORITY_ON) {

                return false;

                return false;

            }

            }

            try {

            try {

                return mService.setPriority(device, priority);

                return service.setPriority(device, priority);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                   return false;

                   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;

        return false;

    }

    }
@@ -420,16 +422,16 @@ public final class BluetoothA2dpSink implements BluetoothProfile { 
     */

     */

    public int getPriority(BluetoothDevice device) {

    public int getPriority(BluetoothDevice device) {

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

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

        if (mService != null && isEnabled()

        final IBluetoothA2dpSink service = mService;

            && isValidDevice(device)) {

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

            try {

            try {

                return mService.getPriority(device);

                return service.getPriority(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return BluetoothProfile.PRIORITY_OFF;

                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;

        return BluetoothProfile.PRIORITY_OFF;

    }

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

     * @param device BluetoothDevice device

     */

     */

    public boolean isA2dpPlaying(BluetoothDevice device) {

    public boolean isA2dpPlaying(BluetoothDevice device) {

        if (mService != null && isEnabled()

        final IBluetoothA2dpSink service = mService;

            && isValidDevice(device)) {

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

            try {

            try {

                return mService.isA2dpPlaying(device);

                return service.isA2dpPlaying(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return false;

                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;

        return false;

    }

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

        public void onServiceConnected(ComponentName className, IBinder service) {

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

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

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

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



            if (mServiceListener != null) {

            if (mServiceListener != null) {

                mServiceListener.onServiceConnected(BluetoothProfile.A2DP_SINK,

                mServiceListener.onServiceConnected(BluetoothProfile.A2DP_SINK,

                        BluetoothA2dpSink.this);

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

    };





    private boolean isEnabled() {

    private boolean isEnabled() {

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

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

       return false;

    }

    }





    private boolean isValidDevice(BluetoothDevice device) {

    private static boolean isValidDevice(BluetoothDevice device) {

       if (device == null) return false;

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



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

       return false;

    }

    }





    private static void log(String msg) {

    private static void log(String msg) {

framework/java/android/bluetooth/BluetoothAvrcpController.java

+29 −30
Original line number Original line Diff line number Diff line
@@ -20,8 +20,6 @@ import android.content.ComponentName; 
import android.content.Context;

import android.content.Context;

import android.content.Intent;

import android.content.Intent;

import android.content.ServiceConnection;

import android.content.ServiceConnection;

import android.media.MediaMetadata;

import android.media.session.PlaybackState;

import android.os.Binder;

import android.os.Binder;

import android.os.IBinder;

import android.os.IBinder;

import android.os.RemoteException;

import android.os.RemoteException;
@@ -83,7 +81,7 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 




    private Context mContext;

    private Context mContext;

    private ServiceListener mServiceListener;

    private ServiceListener mServiceListener;

    private IBluetoothAvrcpController mService;

    private volatile IBluetoothAvrcpController mService;

    private BluetoothAdapter mAdapter;

    private BluetoothAdapter mAdapter;





    final private IBluetoothStateChangeCallback mBluetoothStateChangeCallback =

    final private IBluetoothStateChangeCallback mBluetoothStateChangeCallback =
@@ -180,15 +178,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
     */

     */

    public List<BluetoothDevice> getConnectedDevices() {

    public List<BluetoothDevice> getConnectedDevices() {

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

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

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

        final IBluetoothAvrcpController service = mService;

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

            try {

            try {

                return mService.getConnectedDevices();

                return service.getConnectedDevices();

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return new ArrayList<BluetoothDevice>();

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

        return new ArrayList<BluetoothDevice>();

    }

    }
@@ -197,15 +196,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
     */

     */

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

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

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

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

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

        final IBluetoothAvrcpController service = mService;

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

            try {

            try {

                return mService.getDevicesMatchingConnectionStates(states);

                return service.getDevicesMatchingConnectionStates(states);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return new ArrayList<BluetoothDevice>();

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

        return new ArrayList<BluetoothDevice>();

    }

    }
@@ -214,16 +214,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
     */

     */

    public int getConnectionState(BluetoothDevice device) {

    public int getConnectionState(BluetoothDevice device) {

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

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

        if (mService != null && isEnabled()

        final IBluetoothAvrcpController service = mService;

            && isValidDevice(device)) {

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

            try {

            try {

                return mService.getConnectionState(device);

                return service.getConnectionState(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return BluetoothProfile.STATE_DISCONNECTED;

                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;

        return BluetoothProfile.STATE_DISCONNECTED;

    }

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

    public BluetoothAvrcpPlayerSettings getPlayerSettings(BluetoothDevice device) {

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

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

        BluetoothAvrcpPlayerSettings settings = null;

        BluetoothAvrcpPlayerSettings settings = null;

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

        final IBluetoothAvrcpController service = mService;

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

            try {

            try {

                settings = mService.getPlayerSettings(device);

                settings = service.getPlayerSettings(device);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return null;

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

     */

    public boolean setPlayerApplicationSetting(BluetoothAvrcpPlayerSettings plAppSetting) {

    public boolean setPlayerApplicationSetting(BluetoothAvrcpPlayerSettings plAppSetting) {

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

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

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

        final IBluetoothAvrcpController service = mService;

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

            try {

            try {

                return mService.setPlayerApplicationSetting(plAppSetting);

                return service.setPlayerApplicationSetting(plAppSetting);

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return false;

                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;

        return false;

    }

    }
@@ -269,24 +271,25 @@ public final class BluetoothAvrcpController implements BluetoothProfile { 
     * possible keycode values: next_grp, previous_grp defined above

     * possible keycode values: next_grp, previous_grp defined above

     */

     */

    public void sendGroupNavigationCmd(BluetoothDevice device, int keyCode, int keyState) {

    public void sendGroupNavigationCmd(BluetoothDevice device, int keyCode, int keyState) {

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

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

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

                + keyState);

        final IBluetoothAvrcpController service = mService;

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

            try {

            try {

                mService.sendGroupNavigationCmd(device, keyCode, keyState);

                service.sendGroupNavigationCmd(device, keyCode, keyState);

                return;

                return;

            } catch (RemoteException e) {

            } catch (RemoteException e) {

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

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

                return;

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

    private final ServiceConnection mConnection = new ServiceConnection() {

        public void onServiceConnected(ComponentName className, IBinder service) {

        public void onServiceConnected(ComponentName className, IBinder service) {

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

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

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

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



            if (mServiceListener != null) {

            if (mServiceListener != null) {

                mServiceListener.onServiceConnected(BluetoothProfile.AVRCP_CONTROLLER,

                mServiceListener.onServiceConnected(BluetoothProfile.AVRCP_CONTROLLER,

                        BluetoothAvrcpController.this);

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

    };





    private boolean isEnabled() {

    private boolean isEnabled() {

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

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

       return false;

    }

    }





    private boolean isValidDevice(BluetoothDevice device) {

    private static boolean isValidDevice(BluetoothDevice device) {

       if (device == null) return false;

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



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

       return false;

    }

    }





    private static void log(String msg) {

    private static void log(String msg) {

framework/java/android/bluetooth/BluetoothDevice.java

+198 −136

File changed.

Preview size limit exceeded, changes collapsed.

framework/java/android/bluetooth/BluetoothHeadset.java

+125 −102

File changed.

Preview size limit exceeded, changes collapsed.

framework/java/android/bluetooth/BluetoothHeadsetClient.java

+118 −109

File changed.

Preview size limit exceeded, changes collapsed.