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

1 files changed, 32 insertions, 31 deletions

diff --git a/framework/java/android/bluetooth/BluetoothPbapClient.java b/framework/java/android/bluetooth/BluetoothPbapClient.java
index b9b372c848..00a15f3f70 100644
--- a/framework/java/android/bluetooth/BluetoothPbapClient.java
+++ b/framework/java/android/bluetooth/BluetoothPbapClient.java
@@ -42,7 +42,7 @@ public final class BluetoothPbapClient implements BluetoothProfile {
     public static final String ACTION_CONNECTION_STATE_CHANGED =
             "android.bluetooth.pbap.profile.action.CONNECTION_STATE_CHANGED";
 
-    private IBluetoothPbapClient mService;
+    private volatile IBluetoothPbapClient mService;
     private final Context mContext;
     private ServiceListener mServiceListener;
     private BluetoothAdapter mAdapter;
@@ -173,15 +173,16 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         if (DBG) {
             log("connect(" + device + ") for PBAP Client.");
         }
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothPbapClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.connect(device);
+                return service.connect(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 return false;
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return false;
@@ -197,16 +198,17 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         if (DBG) {
             log("disconnect(" + device + ")" + new Exception());
         }
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothPbapClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                mService.disconnect(device);
+                service.disconnect(device);
                 return true;
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 return false;
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return false;
@@ -223,15 +225,16 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         if (DBG) {
             log("getConnectedDevices()");
         }
-        if (mService != null && isEnabled()) {
+        final IBluetoothPbapClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getConnectedDevices();
+                return service.getConnectedDevices();
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 return new ArrayList<BluetoothDevice>();
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return new ArrayList<BluetoothDevice>();
@@ -247,15 +250,16 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         if (DBG) {
             log("getDevicesMatchingStates()");
         }
-        if (mService != null && isEnabled()) {
+        final IBluetoothPbapClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getDevicesMatchingConnectionStates(states);
+                return service.getDevicesMatchingConnectionStates(states);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 return new ArrayList<BluetoothDevice>();
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return new ArrayList<BluetoothDevice>();
@@ -271,15 +275,16 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         if (DBG) {
             log("getConnectionState(" + device + ")");
         }
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothPbapClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getConnectionState(device);
+                return service.getConnectionState(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 return BluetoothProfile.STATE_DISCONNECTED;
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return BluetoothProfile.STATE_DISCONNECTED;
@@ -321,14 +326,8 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         return false;
     }
 
-    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());
     }
 
     /**
@@ -339,26 +338,27 @@ public final class BluetoothPbapClient implements BluetoothProfile {
      * {@link #PRIORITY_OFF},
      *
      * @param device Paired bluetooth device
-     * @param priority
+     * @param priority Priority of this profile
      * @return true if priority is set, false on error
      */
     public boolean setPriority(BluetoothDevice device, int priority) {
         if (DBG) {
             log("setPriority(" + device + ", " + priority + ")");
         }
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothPbapClient 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, Log.getStackTraceString(new Throwable()));
                 return false;
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return false;
@@ -378,15 +378,16 @@ public final class BluetoothPbapClient implements BluetoothProfile {
         if (VDBG) {
             log("getPriority(" + device + ")");
         }
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothPbapClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getPriority(device);
+                return service.getPriority(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 return PRIORITY_OFF;
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return PRIORITY_OFF;