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, 26 deletions

diff --git a/framework/java/android/bluetooth/BluetoothHealth.java b/framework/java/android/bluetooth/BluetoothHealth.java
index dc5f38135a..57a019755f 100644
--- a/framework/java/android/bluetooth/BluetoothHealth.java
+++ b/framework/java/android/bluetooth/BluetoothHealth.java
@@ -176,9 +176,10 @@ public final class BluetoothHealth implements BluetoothProfile {
         BluetoothHealthAppConfiguration config =
                 new BluetoothHealthAppConfiguration(name, dataType, role, channelType);
 
-        if (mService != null) {
+        final IBluetoothHealth service = mService;
+        if (service != null) {
             try {
-                result = mService.registerAppConfiguration(config, wrapper);
+                result = service.registerAppConfiguration(config, wrapper);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -200,9 +201,10 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     public boolean unregisterAppConfiguration(BluetoothHealthAppConfiguration config) {
         boolean result = false;
-        if (mService != null && isEnabled() && config != null) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled() && config != null) {
             try {
-                result = mService.unregisterAppConfiguration(config);
+                result = service.unregisterAppConfiguration(config);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -228,9 +230,10 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     public boolean connectChannelToSource(BluetoothDevice device,
             BluetoothHealthAppConfiguration config) {
-        if (mService != null && isEnabled() && isValidDevice(device) && config != null) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled() && isValidDevice(device) && config != null) {
             try {
-                return mService.connectChannelToSource(device, config);
+                return service.connectChannelToSource(device, config);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -256,9 +259,10 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     public boolean connectChannelToSink(BluetoothDevice device,
             BluetoothHealthAppConfiguration config, int channelType) {
-        if (mService != null && isEnabled() && isValidDevice(device) && config != null) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled() && isValidDevice(device) && config != null) {
             try {
-                return mService.connectChannelToSink(device, config, channelType);
+                return service.connectChannelToSink(device, config, channelType);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -284,9 +288,10 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     public boolean disconnectChannel(BluetoothDevice device,
             BluetoothHealthAppConfiguration config, int channelId) {
-        if (mService != null && isEnabled() && isValidDevice(device) && config != null) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled() && isValidDevice(device) && config != null) {
             try {
-                return mService.disconnectChannel(device, config, channelId);
+                return service.disconnectChannel(device, config, channelId);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -312,9 +317,10 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     public ParcelFileDescriptor getMainChannelFd(BluetoothDevice device,
             BluetoothHealthAppConfiguration config) {
-        if (mService != null && isEnabled() && isValidDevice(device) && config != null) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled() && isValidDevice(device) && config != null) {
             try {
-                return mService.getMainChannelFd(device, config);
+                return service.getMainChannelFd(device, config);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -341,9 +347,10 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     @Override
     public int getConnectionState(BluetoothDevice device) {
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getHealthDeviceConnectionState(device);
+                return service.getHealthDeviceConnectionState(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -370,15 +377,16 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     @Override
     public List<BluetoothDevice> getConnectedDevices() {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getConnectedHealthDevices();
+                return service.getConnectedHealthDevices();
             } 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>();
     }
 
@@ -401,15 +409,16 @@ public final class BluetoothHealth implements BluetoothProfile {
      */
     @Override
     public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHealth service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getHealthDevicesMatchingConnectionStates(states);
+                return service.getHealthDevicesMatchingConnectionStates(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>();
     }
 
@@ -455,7 +464,7 @@ public final class BluetoothHealth implements BluetoothProfile {
 
     private Context mContext;
     private ServiceListener mServiceListener;
-    private IBluetoothHealth mService;
+    private volatile IBluetoothHealth mService;
     BluetoothAdapter mAdapter;
 
     /**
@@ -540,11 +549,8 @@ public final class BluetoothHealth 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());
     }
 
     private boolean checkAppParam(String name, int role, int channelType,