Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6d1fa6f6 authored by Lee Shombert's avatar Lee Shombert
Browse files

Fix exception handling in getState() binder cache

Bug: 153103051

A binder cache recompute() 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 original getState() method returned OFF if the bluetooth service was
unavailable.  This computation now occurs in the getStateInternal()
method, outside of the binder cache query() method.

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

Test: Create a debug image that enables binder cache VERIFY.  Run the
following tests:
 * atest BluetoothInstrumentationTests
 * atest PtsChreTestCases
 * atest UserLifecycleTests
 * manual testing connecting to bluetooth devices and toggling airplane
   mode.
No cache inconsistencies found.  No test failures seen.

Change-Id: I93b9742587c4eb695d9a11fc6ab145f6a40a0ece
parent d3f12e83
Loading
Loading
Loading
Loading
+28 −10
Original line number Diff line number Diff line
@@ -980,16 +980,10 @@ public final class BluetoothAdapter {
                @Override
                protected Integer recompute(Void query) {
                    try {
                        mServiceLock.readLock().lock();
                        if (mService != null) {
                        return mService.getState();
                        }
                    } catch (RemoteException e) {
                        Log.e(TAG, "", e);
                    } finally {
                        mServiceLock.readLock().unlock();
                        throw e.rethrowFromSystemServer();
                    }
                    return BluetoothAdapter.STATE_OFF;
                }
            };

@@ -1003,6 +997,30 @@ public final class BluetoothAdapter {
        PropertyInvalidatedCache.invalidateCache(BLUETOOTH_GET_STATE_CACHE_PROPERTY);
    }

    /**
     * Fetch the current bluetooth state.  If the service is down, return
     * OFF.
     */
    @AdapterState
    private int getStateInternal() {
        int state = BluetoothAdapter.STATE_OFF;
        try {
            mServiceLock.readLock().lock();
            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();
        }
        return state;
    }

    /**
     * Get the current state of the local Bluetooth adapter.
     * <p>Possible return values are
@@ -1016,7 +1034,7 @@ public final class BluetoothAdapter {
    @RequiresPermission(Manifest.permission.BLUETOOTH)
    @AdapterState
    public int getState() {
        int state = mBluetoothGetStateCache.query(null);
        int state = getStateInternal();

        // Consider all internal states as OFF
        if (state == BluetoothAdapter.STATE_BLE_ON || state == BluetoothAdapter.STATE_BLE_TURNING_ON
@@ -1054,7 +1072,7 @@ public final class BluetoothAdapter {
    @UnsupportedAppUsage(publicAlternatives = "Use {@link #getState()} instead to determine "
            + "whether you can use BLE & BT classic.")
    public int getLeState() {
        int state = mBluetoothGetStateCache.query(null);
        int state = getStateInternal();

        if (VDBG) {
            Log.d(TAG, "getLeState() returning " + BluetoothAdapter.nameForState(state));