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
(cherry picked from commit 1f686f645294d624f8b61e4761fd14787a496c75)

1 files changed, 118 insertions, 109 deletions

diff --git a/framework/java/android/bluetooth/BluetoothHeadsetClient.java b/framework/java/android/bluetooth/BluetoothHeadsetClient.java
index 544b3b95db..73a16ded81 100644
--- a/framework/java/android/bluetooth/BluetoothHeadsetClient.java
+++ b/framework/java/android/bluetooth/BluetoothHeadsetClient.java
@@ -28,7 +28,6 @@ import android.util.Log;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.UUID;
 
 /**
  * Public API to control Hands Free Profile (HFP role only).
@@ -77,8 +76,8 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      * Intent sent whenever audio state changes.
      *
      * <p>It includes two mandatory extras:
-     * {@link BluetoothProfile.EXTRA_STATE},
-     * {@link BluetoothProfile.EXTRA_PREVIOUS_STATE},
+     * {@link BluetoothProfile#EXTRA_STATE},
+     * {@link BluetoothProfile#EXTRA_PREVIOUS_STATE},
      * with possible values:
      * {@link #STATE_AUDIO_CONNECTING},
      * {@link #STATE_AUDIO_CONNECTED},
@@ -368,7 +367,7 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
 
     private Context mContext;
     private ServiceListener mServiceListener;
-    private IBluetoothHeadsetClient mService;
+    private volatile IBluetoothHeadsetClient mService;
     private BluetoothAdapter mAdapter;
 
     final private IBluetoothStateChangeCallback mBluetoothStateChangeCallback =
@@ -480,16 +479,16 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean connect(BluetoothDevice device) {
         if (DBG) log("connect(" + device + ")");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -504,16 +503,16 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean disconnect(BluetoothDevice device) {
         if (DBG) log("disconnect(" + device + ")");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -525,15 +524,16 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
     @Override
     public List<BluetoothDevice> getConnectedDevices() {
         if (VDBG) log("getConnectedDevices()");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient 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>();
     }
 
@@ -548,15 +548,16 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
     @Override
     public List<BluetoothDevice> getDevicesMatchingConnectionStates(int[] states) {
         if (VDBG) log("getDevicesMatchingStates()");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient 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>();
     }
 
@@ -569,16 +570,16 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
     @Override
     public int getConnectionState(BluetoothDevice device) {
         if (VDBG) log("getConnectionState(" + device + ")");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -589,20 +590,20 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean setPriority(BluetoothDevice device, int priority) {
         if (DBG) log("setPriority(" + device + ", " + priority + ")");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
-            if (priority != BluetoothProfile.PRIORITY_OFF &&
-                    priority != BluetoothProfile.PRIORITY_ON) {
-              return false;
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -611,16 +612,16 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public int getPriority(BluetoothDevice device) {
         if (VDBG) log("getPriority(" + device + ")");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -639,15 +640,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean startVoiceRecognition(BluetoothDevice device) {
         if (DBG) log("startVoiceRecognition()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -666,15 +667,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean stopVoiceRecognition(BluetoothDevice device) {
         if (DBG) log("stopVoiceRecognition()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient 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;
     }
 
@@ -686,15 +687,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public List<BluetoothHeadsetClientCall> getCurrentCalls(BluetoothDevice device) {
         if (DBG) log("getCurrentCalls()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getCurrentCalls(device);
+                return service.getCurrentCalls(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 null;
     }
 
@@ -707,15 +708,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public Bundle getCurrentAgEvents(BluetoothDevice device) {
         if (DBG) log("getCurrentCalls()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getCurrentAgEvents(device);
+                return service.getCurrentAgEvents(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 null;
     }
 
@@ -733,15 +734,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean acceptCall(BluetoothDevice device, int flag) {
         if (DBG) log("acceptCall()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.acceptCall(device, flag);
+                return service.acceptCall(device, flag);
             } 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;
     }
 
@@ -756,15 +757,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean holdCall(BluetoothDevice device) {
         if (DBG) log("holdCall()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.holdCall(device);
+                return service.holdCall(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;
     }
 
@@ -783,15 +784,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean rejectCall(BluetoothDevice device) {
         if (DBG) log("rejectCall()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.rejectCall(device);
+                return service.rejectCall(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;
     }
 
@@ -802,7 +803,7 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      *
      * @param device    remote device
      * @param call      Handle of call obtained in {@link dial()} or obtained via
-     *                  {@link ACTION_CALL_CHANGED}. {@code call} may be null in which
+     *                  {@link #ACTION_CALL_CHANGED}. {@code call} may be null in which
      *                  case we will hangup all active calls.
      * @return          <code>true</code> if command has been issued successfully;
      *                   <code>false</code> otherwise;
@@ -815,15 +816,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean terminateCall(BluetoothDevice device, BluetoothHeadsetClientCall call) {
         if (DBG) log("terminateCall()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.terminateCall(device, call);
+                return service.terminateCall(device, call);
             } 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;
     }
 
@@ -845,15 +846,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean enterPrivateMode(BluetoothDevice device, int index) {
         if (DBG) log("enterPrivateMode()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.enterPrivateMode(device, index);
+                return service.enterPrivateMode(device, index);
             } 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;
     }
 
@@ -874,15 +875,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean explicitCallTransfer(BluetoothDevice device) {
         if (DBG) log("explicitCallTransfer()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.explicitCallTransfer(device);
+                return service.explicitCallTransfer(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;
     }
 
@@ -900,15 +901,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public BluetoothHeadsetClientCall dial(BluetoothDevice device, String number) {
         if (DBG) log("dial()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.dial(device, number);
+                return service.dial(device, number);
             } 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 null;
     }
 
@@ -925,15 +926,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean sendDTMF(BluetoothDevice device, byte code) {
         if (DBG) log("sendDTMF()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.sendDTMF(device, code);
+                return service.sendDTMF(device, code);
             } 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;
     }
 
@@ -952,15 +953,15 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean getLastVoiceTagNumber(BluetoothDevice device) {
         if (DBG) log("getLastVoiceTagNumber()");
-        if (mService != null && isEnabled() &&
-                isValidDevice(device)) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled() && isValidDevice(device)) {
             try {
-                return mService.getLastVoiceTagNumber(device);
+                return service.getLastVoiceTagNumber(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;
     }
 
@@ -971,10 +972,13 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public int getAudioState(BluetoothDevice device) {
         if (VDBG) log("getAudioState");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getAudioState(device);
-            } catch (RemoteException e) {Log.e(TAG, e.toString());}
+                return service.getAudioState(device);
+            } catch (RemoteException e) {
+                Log.e(TAG, e.toString());
+            }
         } else {
             Log.w(TAG, "Proxy not attached to service");
             if (DBG) Log.d(TAG, Log.getStackTraceString(new Throwable()));
@@ -991,10 +995,13 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public void setAudioRouteAllowed(BluetoothDevice device, boolean allowed) {
         if (VDBG) log("setAudioRouteAllowed");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                mService.setAudioRouteAllowed(device, allowed);
-            } catch (RemoteException e) {Log.e(TAG, e.toString());}
+                service.setAudioRouteAllowed(device, allowed);
+            } catch (RemoteException e) {
+                Log.e(TAG, e.toString());
+            }
         } else {
             Log.w(TAG, "Proxy not attached to service");
             if (DBG) Log.d(TAG, Log.getStackTraceString(new Throwable()));
@@ -1009,10 +1016,13 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      */
     public boolean getAudioRouteAllowed(BluetoothDevice device) {
         if (VDBG) log("getAudioRouteAllowed");
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getAudioRouteAllowed(device);
-            } catch (RemoteException e) {Log.e(TAG, e.toString());}
+                return service.getAudioRouteAllowed(device);
+            } catch (RemoteException e) {
+                Log.e(TAG, e.toString());
+            }
         } else {
             Log.w(TAG, "Proxy not attached to service");
             if (DBG) Log.d(TAG, Log.getStackTraceString(new Throwable()));
@@ -1032,9 +1042,10 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      *                   intent;
      */
     public boolean connectAudio(BluetoothDevice device) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.connectAudio(device);
+                return service.connectAudio(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1057,9 +1068,10 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      *                   intent;
      */
     public boolean disconnectAudio(BluetoothDevice device) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.disconnectAudio(device);
+                return service.disconnectAudio(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1078,9 +1090,10 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
      *                  AG not connected
      */
     public Bundle getCurrentAgFeatures(BluetoothDevice device) {
-        if (mService != null && isEnabled()) {
+        final IBluetoothHeadsetClient service = mService;
+        if (service != null && isEnabled()) {
             try {
-                return mService.getCurrentAgFeatures(device);
+                return service.getCurrentAgFeatures(device);
             } catch (RemoteException e) {
                 Log.e(TAG, e.toString());
             }
@@ -1092,7 +1105,7 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
     }
 
 
-    private ServiceConnection mConnection = new ServiceConnection() {
+    private final ServiceConnection mConnection = new ServiceConnection() {
         @Override
         public void onServiceConnected(ComponentName className, IBinder service) {
             if (DBG) Log.d(TAG, "Proxy object connected");
@@ -1114,15 +1127,11 @@ public final class BluetoothHeadsetClient implements BluetoothProfile {
     };
 
     private boolean isEnabled() {
-       if (mAdapter.getState() == BluetoothAdapter.STATE_ON) return true;
-       return false;
+        return mAdapter.getState() == BluetoothAdapter.STATE_ON;
     }
 
-    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) {