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, 86 insertions, 57 deletions

diff --git a/framework/java/android/bluetooth/BluetoothDevice.java b/framework/java/android/bluetooth/BluetoothDevice.java
index 3ab2c4a8f4..d982bb7ffb 100644
--- a/framework/java/android/bluetooth/BluetoothDevice.java
+++ b/framework/java/android/bluetooth/BluetoothDevice.java
@@ -712,7 +712,7 @@ public final class BluetoothDevice implements Parcelable {
      * getService() called.
      * TODO: Unify implementation of sService amongst BluetoothFoo API's
      */
-    private static IBluetooth sService;
+    private static volatile IBluetooth sService;
 
     private final String mAddress;
 
@@ -839,12 +839,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public String getName() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot get Remote Device name");
             return null;
         }
         try {
-            return sService.getRemoteName(this);
+            return service.getRemoteName(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -859,12 +860,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public int getType() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot get Remote Device type");
             return DEVICE_TYPE_UNKNOWN;
         }
         try {
-            return sService.getRemoteType(this);
+            return service.getRemoteType(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -879,12 +881,13 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public String getAlias() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot get Remote Device Alias");
             return null;
         }
         try {
-            return sService.getRemoteAlias(this);
+            return service.getRemoteAlias(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -902,12 +905,13 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean setAlias(String alias) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot set Remote Device name");
             return false;
         }
         try {
-            return sService.setRemoteAlias(this, alias);
+            return service.setRemoteAlias(this, alias);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -942,12 +946,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public int getBatteryLevel() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "Bluetooth disabled. Cannot get remote device battery level");
             return BATTERY_LEVEL_UNKNOWN;
         }
         try {
-            return sService.getBatteryLevel(this);
+            return service.getBatteryLevel(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -966,7 +971,8 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH_ADMIN)
     public boolean createBond() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot create bond to Remote Device");
             return false;
         }
@@ -974,7 +980,7 @@ public final class BluetoothDevice implements Parcelable {
             Log.i(TAG, "createBond() for device " + getAddress()
                     + " called by pid: " + Process.myPid()
                     + " tid: " + Process.myTid());
-            return sService.createBond(this, TRANSPORT_AUTO);
+            return service.createBond(this, TRANSPORT_AUTO);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -998,7 +1004,8 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean createBond(int transport) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot create bond to Remote Device");
             return false;
         }
@@ -1009,7 +1016,7 @@ public final class BluetoothDevice implements Parcelable {
             Log.i(TAG, "createBond() for device " + getAddress()
                     + " called by pid: " + Process.myPid()
                     + " tid: " + Process.myTid());
-            return sService.createBond(this, transport);
+            return service.createBond(this, transport);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1035,8 +1042,13 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean createBondOutOfBand(int transport, OobData oobData) {
+        final IBluetooth service = sService;
+        if (service == null) {
+            Log.w(TAG, "BT not enabled, createBondOutOfBand failed");
+            return false;
+        }
         try {
-            return sService.createBondOutOfBand(this, transport, oobData);
+            return service.createBondOutOfBand(this, transport, oobData);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1045,8 +1057,13 @@ public final class BluetoothDevice implements Parcelable {
 
     /** @hide */
     public boolean isBondingInitiatedLocally() {
+        final IBluetooth service = sService;
+        if (service == null) {
+            Log.w(TAG, "BT not enabled, isBondingInitiatedLocally failed");
+            return false;
+        }
         try {
-            return sService.isBondingInitiatedLocally(this);
+            return service.isBondingInitiatedLocally(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1082,7 +1099,8 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean cancelBondProcess() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot cancel Remote Device bond");
             return false;
         }
@@ -1090,7 +1108,7 @@ public final class BluetoothDevice implements Parcelable {
             Log.i(TAG, "cancelBondProcess() for device " + getAddress()
                     + " called by pid: " + Process.myPid()
                     + " tid: " + Process.myTid());
-            return sService.cancelBondProcess(this);
+            return service.cancelBondProcess(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1108,7 +1126,8 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean removeBond() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot remove Remote Device bond");
             return false;
         }
@@ -1116,7 +1135,7 @@ public final class BluetoothDevice implements Parcelable {
             Log.i(TAG, "removeBond() for device " + getAddress()
                     + " called by pid: " + Process.myPid()
                     + " tid: " + Process.myTid());
-            return sService.removeBond(this);
+            return service.removeBond(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1134,19 +1153,15 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public int getBondState() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot get bond state");
             return BOND_NONE;
         }
         try {
-            return sService.getBondState(this);
+            return service.getBondState(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
-        } catch (NullPointerException npe) {
-            // Handle case where bluetooth service proxy
-            // is already null.
-            Log.e(TAG, "NullPointerException for getBondState() of device ("
-                    + getAddress() + ")", npe);
         }
         return BOND_NONE;
     }
@@ -1160,12 +1175,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @SystemApi
     public boolean isConnected() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             // BT is not enabled, we cannot be connected.
             return false;
         }
         try {
-            return sService.getConnectionState(this) != CONNECTION_STATE_DISCONNECTED;
+            return service.getConnectionState(this) != CONNECTION_STATE_DISCONNECTED;
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
             return false;
@@ -1182,12 +1198,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @SystemApi
     public boolean isEncrypted() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             // BT is not enabled, we cannot be connected.
             return false;
         }
         try {
-            return sService.getConnectionState(this) > CONNECTION_STATE_CONNECTED;
+            return service.getConnectionState(this) > CONNECTION_STATE_CONNECTED;
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
             return false;
@@ -1201,12 +1218,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public BluetoothClass getBluetoothClass() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot get Bluetooth Class");
             return null;
         }
         try {
-            int classInt = sService.getRemoteClass(this);
+            int classInt = service.getRemoteClass(this);
             if (classInt == BluetoothClass.ERROR) return null;
             return new BluetoothClass(classInt);
         } catch (RemoteException e) {
@@ -1227,12 +1245,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public ParcelUuid[] getUuids() {
-        if (sService == null || !isBluetoothEnabled()) {
+        final IBluetooth service = sService;
+        if (service == null || !isBluetoothEnabled()) {
             Log.e(TAG, "BT not enabled. Cannot get remote device Uuids");
             return null;
         }
         try {
-            return sService.getRemoteUuids(this);
+            return service.getRemoteUuids(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1254,7 +1273,7 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     public boolean fetchUuidsWithSdp() {
-        IBluetooth service = sService;
+        final IBluetooth service = sService;
         if (service == null || !isBluetoothEnabled()) {
             Log.e(TAG, "BT not enabled. Cannot fetchUuidsWithSdp");
             return false;
@@ -1289,12 +1308,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     /** @hide */
     public boolean sdpSearch(ParcelUuid uuid) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot query remote device sdp records");
             return false;
         }
         try {
-            return sService.sdpSearch(this, uuid);
+            return service.sdpSearch(this, uuid);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1308,12 +1328,13 @@ public final class BluetoothDevice implements Parcelable {
      * @return true pin has been set false for error
      */
     public boolean setPin(byte[] pin) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot set Remote Device pin");
             return false;
         }
         try {
-            return sService.setPin(this, true, pin.length, pin);
+            return service.setPin(this, true, pin.length, pin);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1337,12 +1358,13 @@ public final class BluetoothDevice implements Parcelable {
      */
     @RequiresPermission(Manifest.permission.BLUETOOTH_PRIVILEGED)
     public boolean setPairingConfirmation(boolean confirm) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot set pairing confirmation");
             return false;
         }
         try {
-            return sService.setPairingConfirmation(this, confirm);
+            return service.setPairingConfirmation(this, confirm);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1361,12 +1383,13 @@ public final class BluetoothDevice implements Parcelable {
 
     /** @hide */
     public boolean cancelPairingUserInput() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             Log.e(TAG, "BT not enabled. Cannot create pairing user input");
             return false;
         }
         try {
-            return sService.cancelBondProcess(this);
+            return service.cancelBondProcess(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1400,11 +1423,12 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public int getPhonebookAccessPermission() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             return ACCESS_UNKNOWN;
         }
         try {
-            return sService.getPhonebookAccessPermission(this);
+            return service.getPhonebookAccessPermission(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1421,11 +1445,12 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean setPhonebookAccessPermission(int value) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             return false;
         }
         try {
-            return sService.setPhonebookAccessPermission(this, value);
+            return service.setPhonebookAccessPermission(this, value);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1440,11 +1465,12 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public int getMessageAccessPermission() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             return ACCESS_UNKNOWN;
         }
         try {
-            return sService.getMessageAccessPermission(this);
+            return service.getMessageAccessPermission(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1461,11 +1487,12 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean setMessageAccessPermission(int value) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             return false;
         }
         try {
-            return sService.setMessageAccessPermission(this, value);
+            return service.setMessageAccessPermission(this, value);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1480,11 +1507,12 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public int getSimAccessPermission() {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             return ACCESS_UNKNOWN;
         }
         try {
-            return sService.getSimAccessPermission(this);
+            return service.getSimAccessPermission(this);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }
@@ -1501,11 +1529,12 @@ public final class BluetoothDevice implements Parcelable {
      * @hide
      */
     public boolean setSimAccessPermission(int value) {
-        if (sService == null) {
+        final IBluetooth service = sService;
+        if (service == null) {
             return false;
         }
         try {
-            return sService.setSimAccessPermission(this, value);
+            return service.setSimAccessPermission(this, value);
         } catch (RemoteException e) {
             Log.e(TAG, "", e);
         }