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 1f686f645294d624f8b61e4761fd14787a496c75)

1 files changed, 29 insertions, 30 deletions

diff --git a/framework/java/android/bluetooth/BluetoothAvrcpController.java b/framework/java/android/bluetooth/BluetoothAvrcpController.java
index 0261b1b35e..a04f110d6f 100644
--- a/framework/java/android/bluetooth/BluetoothAvrcpController.java
+++ b/framework/java/android/bluetooth/BluetoothAvrcpController.java
@@ -20,8 +20,6 @@ import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.content.ServiceConnection;
-import android.media.MediaMetadata;
-import android.media.session.PlaybackState;
 import android.os.Binder;
 import android.os.IBinder;
 import android.os.RemoteException;
@@ -83,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;
 
     final private IBluetoothStateChangeCallback mBluetoothStateChangeCallback =
@@ -180,15 +178,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile {
      */
     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 +196,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile {
      */
     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>();
     }
 
@@ -214,16 +214,16 @@ public final class BluetoothAvrcpController implements BluetoothProfile {
      */
     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;
     }
 
@@ -235,9 +235,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;
@@ -252,15 +253,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;
     }
 
@@ -269,24 +271,25 @@ public final class BluetoothAvrcpController implements BluetoothProfile {
      * possible keycode values: next_grp, previous_grp defined above
      */
     public void sendGroupNavigationCmd(BluetoothDevice device, int keyCode, int keyState) {
-        Log.d(TAG, "sendGroupNavigationCmd dev = " + device + " key " + keyCode + " State = " + keyState);
-        if (mService != null && isEnabled()) {
+        Log.d(TAG, "sendGroupNavigationCmd dev = " + device + " key " + keyCode + " State = "
+                + keyState);
+        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);
@@ -302,15 +305,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) {