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, 35 insertions, 30 deletions

diff --git a/framework/java/android/bluetooth/BluetoothInputHost.java b/framework/java/android/bluetooth/BluetoothInputHost.java
index 15303dc794..37f04278d4 100644
--- a/framework/java/android/bluetooth/BluetoothInputHost.java
+++ b/framework/java/android/bluetooth/BluetoothInputHost.java
@@ -113,7 +113,7 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
     private ServiceListener mServiceListener;
 
-    private IBluetoothInputHost mService;
+    private volatile IBluetoothInputHost mService;
 
     private BluetoothAdapter mAdapter;
 
@@ -202,24 +202,18 @@ public final class BluetoothInputHost implements BluetoothProfile {
                 }
             };
 
-    private ServiceConnection mConnection = new ServiceConnection() {
-
+    private final ServiceConnection mConnection = new ServiceConnection() {
         public void onServiceConnected(ComponentName className, IBinder service) {
             Log.d(TAG, "onServiceConnected()");
-
             mService = IBluetoothInputHost.Stub.asInterface(service);
-
             if (mServiceListener != null) {
                 mServiceListener.onServiceConnected(BluetoothProfile.INPUT_HOST,
                         BluetoothInputHost.this);
             }
         }
-
         public void onServiceDisconnected(ComponentName className) {
             Log.d(TAG, "onServiceDisconnected()");
-
             mService = null;
-
             if (mServiceListener != null) {
                 mServiceListener.onServiceDisconnected(BluetoothProfile.INPUT_HOST);
             }
@@ -291,9 +285,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
     public List<BluetoothDevice> getConnectedDevices() {
         Log.v(TAG, "getConnectedDevices()");
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                return mService.getConnectedDevices();
+                return service.getConnectedDevices();
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -311,9 +306,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
     public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {
         Log.v(TAG, "getDevicesMatchingConnectionStates(): states=" + Arrays.toString(states));
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                return mService.getDevicesMatchingConnectionStates(states);
+                return service.getDevicesMatchingConnectionStates(states);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -331,9 +327,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
     public int getConnectionState(BluetoothDevice device) {
         Log.v(TAG, "getConnectionState(): device=" + device);
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                return mService.getConnectionState(device);
+                return service.getConnectionState(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -370,13 +367,14 @@ public final class BluetoothInputHost implements BluetoothProfile {
             return false;
         }
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
                 BluetoothHidDeviceAppConfiguration config =
                         new BluetoothHidDeviceAppConfiguration();
                 BluetoothHidDeviceCallbackWrapper cbw =
                         new BluetoothHidDeviceCallbackWrapper(callback);
-                result = mService.registerApp(config, sdp, inQos, outQos, cbw);
+                result = service.registerApp(config, sdp, inQos, outQos, cbw);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -403,9 +401,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.unregisterApp(config);
+                result = service.unregisterApp(config);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -427,9 +426,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
     public boolean sendReport(BluetoothDevice device, int id, byte[] data) {
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.sendReport(device, id, data);
+                result = service.sendReport(device, id, data);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -454,9 +454,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.replyReport(device, type, id, data);
+                result = service.replyReport(device, type, id, data);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -479,9 +480,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.reportError(device, error);
+                result = service.reportError(device, error);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -502,9 +504,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.unplug(device);
+                result = service.unplug(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -526,9 +529,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.connect(device);
+                result = service.connect(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -549,9 +553,10 @@ public final class BluetoothInputHost implements BluetoothProfile {
 
         boolean result = false;
 
-        if (mService != null) {
+        final IBluetoothInputHost service = mService;
+        if (service != null) {
             try {
-                result = mService.disconnect(device);
+                result = service.disconnect(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }