From 81a3e1c578b5618bf02968a9b4402e70592b7b9b Mon Sep 17 00:00:00 2001
From: Lee Shombert <shombert@google.com>
Date: Fri, 3 Apr 2020 14:00:40 -0700
Subject: [PATCH] Fix exception handling in getState() binder cache

Bug: 153103051

A binder cache query function cannot compute a result based on any data
that is not known to the binder server.  If the code depends on local
data that can change, then invalidation will not work properly.

The getState() method returns OFF if the bluetooth service is
unavailable.  This computation now occurs in the getState() method,
outside of the binder cache query() method.

The query method converts RemoteExceptions to RuntimeExceptions.  Then,
the conversion is reversed in getState().  This double conversion is
needed because the cache query() method has no throw spec.

Test: Run 'atest BluetoothInstrumentationTests' with a special debug
image that enables binder cache VERIFY.  The test found no cache
inconsistencies.

Change-Id: I80db86f66d8b51fa94207824c8b15972a9066ef5
---
 .../android/bluetooth/BluetoothAdapter.java   | 32 +++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/core/java/android/bluetooth/BluetoothAdapter.java b/core/java/android/bluetooth/BluetoothAdapter.java
index f216db6fc7175..fc48e7f18f5fc 100644
--- a/core/java/android/bluetooth/BluetoothAdapter.java
+++ b/core/java/android/bluetooth/BluetoothAdapter.java
@@ -979,17 +979,14 @@ public final class BluetoothAdapter {
                 8, BLUETOOTH_GET_STATE_CACHE_PROPERTY) {
                 @Override
                 protected Integer recompute(Void query) {
+                    // This function must be called while holding the
+                    // mServiceLock, and with mService not null. The public
+                    // getState() method makes this guarantee.
                     try {
-                        mServiceLock.readLock().lock();
-                        if (mService != null) {
-                            return mService.getState();
-                        }
+                        return mService.getState();
                     } catch (RemoteException e) {
-                        Log.e(TAG, "", e);
-                    } finally {
-                        mServiceLock.readLock().unlock();
+                        throw e.rethrowFromSystemServer();
                     }
-                    return BluetoothAdapter.STATE_OFF;
                 }
             };
 
@@ -1016,7 +1013,24 @@ public final class BluetoothAdapter {
     @RequiresPermission(Manifest.permission.BLUETOOTH)
     @AdapterState
     public int getState() {
-        int state = mBluetoothGetStateCache.query(null);
+        int state = BluetoothAdapter.STATE_OFF;
+
+        try {
+            mServiceLock.readLock().lock();
+            // The test for mService must either be outside the cache, or
+            // the cache must be invalidated when mService changes.
+            if (mService != null) {
+                state = mBluetoothGetStateCache.query(null);
+            }
+        } catch (RuntimeException e) {
+            if (e.getCause() instanceof RemoteException) {
+                Log.e(TAG, "", e.getCause());
+            } else {
+                throw e;
+            }
+        } finally {
+            mServiceLock.readLock().unlock();
+        }
 
         // Consider all internal states as OFF
         if (state == BluetoothAdapter.STATE_BLE_ON || state == BluetoothAdapter.STATE_BLE_TURNING_ON