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, 102 insertions, 79 deletions

diff --git a/framework/java/android/bluetooth/BluetoothHeadset.java b/framework/java/android/bluetooth/BluetoothHeadset.java
index be1ce63cad..85550c7720 100644
--- a/framework/java/android/bluetooth/BluetoothHeadset.java
+++ b/framework/java/android/bluetooth/BluetoothHeadset.java
@@ -306,7 +306,7 @@ public final class BluetoothHeadset implements BluetoothProfile {
 
     private Context mContext;
     private ServiceListener mServiceListener;
-    private IBluetoothHeadset mService;
+    private volatile IBluetoothHeadset mService;
     private BluetoothAdapter mAdapter;
 
     private final IBluetoothStateChangeCallback mBluetoothStateChangeCallback =
@@ -418,15 +418,16 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean connect(BluetoothDevice device) {
         if (DBG) log("connect(" + device + ")");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset 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) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return false;
     }
 
@@ -457,15 +458,16 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean disconnect(BluetoothDevice device) {
         if (DBG) log("disconnect(" + device + ")");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.disconnect(device);
+                return service.disconnect(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
                 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;
     }
 
@@ -475,15 +477,16 @@ public final class BluetoothHeadset implements BluetoothProfile {
     @Override
     public List<BluetoothDevice> getConnectedDevices() {
         if (VDBG) log("getConnectedDevices()");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset 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) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return new ArrayList<BluetoothDevice>();
     }
 
@@ -493,15 +496,16 @@ public final class BluetoothHeadset implements BluetoothProfile {
     @Override
     public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {
         if (VDBG) log("getDevicesMatchingStates()");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset 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) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return new ArrayList<BluetoothDevice>();
     }
 
@@ -511,15 +515,16 @@ public final class BluetoothHeadset implements BluetoothProfile {
     @Override
     public int getConnectionState(BluetoothDevice device) {
         if (VDBG) log("getConnectionState(" + device + ")");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset 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) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return BluetoothProfile.STATE_DISCONNECTED;
     }
 
@@ -540,19 +545,20 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean setPriority(BluetoothDevice device, int priority) {
         if (DBG) log("setPriority(" + device + ", " + priority + ")");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset 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) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return false;
     }
 
@@ -571,15 +577,16 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public int getPriority(BluetoothDevice device) {
         if (VDBG) log("getPriority(" + device + ")");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset 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) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return PRIORITY_OFF;
     }
 
@@ -605,14 +612,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean startVoiceRecognition(BluetoothDevice device) {
         if (DBG) log("startVoiceRecognition()");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.startVoiceRecognition(device);
+                return service.startVoiceRecognition(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
             }
         }
-        if (mService == null) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return false;
     }
 
@@ -627,14 +635,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean stopVoiceRecognition(BluetoothDevice device) {
         if (DBG) log("stopVoiceRecognition()");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.stopVoiceRecognition(device);
+                return service.stopVoiceRecognition(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
             }
         }
-        if (mService == null) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return false;
     }
 
@@ -648,14 +657,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean isAudioConnected(BluetoothDevice device) {
         if (VDBG) log("isAudioConnected()");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.isAudioConnected(device);
+                return service.isAudioConnected(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
             }
         }
-        if (mService == null) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return false;
     }
 
@@ -674,14 +684,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public int getBatteryUsageHint(BluetoothDevice device) {
         if (VDBG) log("getBatteryUsageHint()");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getBatteryUsageHint(device);
+                return service.getBatteryUsageHint(device);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
             }
         }
-        if (mService == null) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return -1;
     }
 
@@ -704,9 +715,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean acceptIncomingConnect(BluetoothDevice device) {
         if (DBG) log("acceptIncomingConnect");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.acceptIncomingConnect(device);
+                return service.acceptIncomingConnect(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -724,9 +736,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean rejectIncomingConnect(BluetoothDevice device) {
         if (DBG) log("rejectIncomingConnect");
-        if (mService != null) {
+        final IBluetoothHeadset service = mService;
+        if (service != null) {
             try {
-                return mService.rejectIncomingConnect(device);
+                return service.rejectIncomingConnect(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -745,9 +758,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public int getAudioState(BluetoothDevice device) {
         if (VDBG) log("getAudioState");
-        if (mService != null && !isDisabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && !isDisabled()) {
             try {
-                return mService.getAudioState(device);
+                return service.getAudioState(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -770,9 +784,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public void setAudioRouteAllowed(boolean allowed) {
         if (VDBG) log("setAudioRouteAllowed");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                mService.setAudioRouteAllowed(allowed);
+                service.setAudioRouteAllowed(allowed);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -790,9 +805,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean getAudioRouteAllowed() {
         if (VDBG) log("getAudioRouteAllowed");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getAudioRouteAllowed();
+                return service.getAudioRouteAllowed();
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -812,9 +828,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public void setForceScoAudio(boolean forced) {
         if (VDBG) log("setForceScoAudio " + String.valueOf(forced));
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                mService.setForceScoAudio(forced);
+                service.setForceScoAudio(forced);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -834,14 +851,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean isAudioOn() {
         if (VDBG) log("isAudioOn()");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.isAudioOn();
+                return service.isAudioOn();
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
             }
         }
-        if (mService == null) Log.w(TAG, "Proxy not attached to service");
+        if (service == null) Log.w(TAG, "Proxy not attached to service");
         return false;
 
     }
@@ -855,9 +873,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      * @hide
      */
     public boolean connectAudio() {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.connectAudio();
+                return service.connectAudio();
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -877,9 +896,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      * @hide
      */
     public boolean disconnectAudio() {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.disconnectAudio();
+                return service.disconnectAudio();
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -903,9 +923,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean startScoUsingVirtualVoiceCall(BluetoothDevice device) {
         if (DBG) log("startScoUsingVirtualVoiceCall()");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.startScoUsingVirtualVoiceCall(device);
+                return service.startScoUsingVirtualVoiceCall(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -926,9 +947,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public boolean stopScoUsingVirtualVoiceCall(BluetoothDevice device) {
         if (DBG) log("stopScoUsingVirtualVoiceCall()");
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.stopScoUsingVirtualVoiceCall(device);
+                return service.stopScoUsingVirtualVoiceCall(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -949,9 +971,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public void phoneStateChanged(int numActive, int numHeld, int callState, String number,
             int type) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                mService.phoneStateChanged(numActive, numHeld, callState, number, type);
+                service.phoneStateChanged(numActive, numHeld, callState, number, type);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -968,9 +991,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      */
     public void clccResponse(int index, int direction, int status, int mode, boolean mpty,
             String number, int type) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                mService.clccResponse(index, direction, status, mode, mpty, number, type);
+                service.clccResponse(index, direction, status, mode, mpty, number, type);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1006,14 +1030,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
         if (command == null) {
             throw new IllegalArgumentException("command is null");
         }
-        if (mService != null && isEnabled() && isValidDevice(device)) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.sendVendorSpecificResultCode(device, command, arg);
+                return service.sendVendorSpecificResultCode(device, command, arg);
             } catch (RemoteException e) {
                 Log.e(TAG, Log.getStackTraceString(new Throwable()));
             }
         }
-        if (mService == null) {
+        if (service == null) {
             Log.w(TAG, "Proxy not attached to service");
         }
         return false;
@@ -1027,9 +1052,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      * @hide
      */
     public boolean enableWBS() {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.enableWBS();
+                return service.enableWBS();
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1048,9 +1074,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      * @hide
      */
     public boolean disableWBS() {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.disableWBS();
+                return service.disableWBS();
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1083,9 +1110,10 @@ public final class BluetoothHeadset implements BluetoothProfile {
      * @hide
      */
     public void bindResponse(int indId, boolean indStatus) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadset service = mService;
+        if (service != null && isEnabled()) {
             try {
-                mService.bindResponse(indId, indStatus);
+                service.bindResponse(indId, indStatus);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1115,20 +1143,15 @@ public final class BluetoothHeadset implements BluetoothProfile {
     };
 
     private boolean isEnabled() {
-        if (mAdapter.getState() == BluetoothAdapter.STATE_ON) return true;
-        return false;
+        return mAdapter.getState() == BluetoothAdapter.STATE_ON;
     }
 
     private boolean isDisabled() {
-        if (mAdapter.getState() == BluetoothAdapter.STATE_OFF) return true;
-        return false;
+        return mAdapter.getState() == BluetoothAdapter.STATE_OFF;
     }
 
-    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) {