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

Commit 83c6281b authored by Hui Peng's avatar Hui Peng
Browse files

Fix a deadlock bug in Bluetooth Framework

Both BluetoothDevice and BluetoothAdapter maintain a reference to a
IBluetooth proxy object (sService) and each class uses a lock object
to serialize access to their own reference. These references are updated
by BluetoothManagerService with registered callbacks (IBlueoothManagerCallback).

In the current implentaion, when an app thread uses BluetoothDevice#getService
to access its reference to service proxy, with the BluetoothManager updating it
at the same time, as the order of taking the locks is different, deadlock is possible
under certain circumstances (triggered in bug 241212710).

This patch fixes the deadlock issue by removing the reference to service
proxy object in BluetoothDevice class, and accesses are via
the reference in BluetoothAdapter class instead.

Test: existing unit tests
Bug: 241212710
Tag: #stability
Change-Id: I15cd2707acf5caa04d97c6ede3bd5bedd6475c65
parent 958a575a
Loading
Loading
Loading
Loading
+8 −4
Original line number Diff line number Diff line
@@ -4235,14 +4235,18 @@ public final class BluetoothAdapter {

    /*package*/ IBluetooth getBluetoothService() {
        synchronized (sServiceLock) {
            if (sProxyServiceStateCallbacks.isEmpty()) {
                throw new IllegalStateException(
                        "Anonymous service access requires at least one lifecycle in process");
            }
            return sService;
        }
    }

    /**
     * Registers a IBluetoothManagerCallback and returns the cached
     * Bluetooth service proxy object.
     *
     * TODO: rename this API to registerBlueoothManagerCallback or something?
     * the current name does not match what it does very well.
     *
     * /
    @UnsupportedAppUsage
    /*package*/ IBluetooth getBluetoothService(IBluetoothManagerCallback cb) {
        Objects.requireNonNull(cb);
+34 −76
Original line number Diff line number Diff line
@@ -1278,55 +1278,14 @@ public final class BluetoothDevice implements Parcelable, Attributable {

    private static final String NULL_MAC_ADDRESS = "00:00:00:00:00:00";

    /**
     * Lazy initialization. Guaranteed final after first object constructed, or
     * getService() called.
     * TODO: Unify implementation of sService amongst BluetoothFoo API's
     */
    private static volatile IBluetooth sService;

    private final String mAddress;
    @AddressType private final int mAddressType;

    private AttributionSource mAttributionSource;

    /*package*/
    static IBluetooth getService() {
        synchronized (BluetoothDevice.class) {
            if (sService == null) {
                BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter();
                sService = adapter.getBluetoothService(sStateChangeCallback);
            }
        }
        return sService;
    }

    static IBluetoothManagerCallback sStateChangeCallback = new IBluetoothManagerCallback.Stub() {

        public void onBluetoothServiceUp(IBluetooth bluetoothService)
                throws RemoteException {
            synchronized (BluetoothDevice.class) {
                if (sService == null) {
                    sService = bluetoothService;
                }
        return BluetoothAdapter.getDefaultAdapter().getBluetoothService();
    }
        }

        public void onBluetoothServiceDown()
                throws RemoteException {
            synchronized (BluetoothDevice.class) {
                sService = null;
            }
        }

        public void onBrEdrDown() {
            if (DBG) Log.d(TAG, "onBrEdrDown: reached BLE ON state");
        }

        public void onOobData(@Transport int transport, OobData oobData) {
            if (DBG) Log.d(TAG, "onOobData: got data");
        }
    };

    /**
     * Create a new BluetoothDevice.
@@ -1340,7 +1299,6 @@ public final class BluetoothDevice implements Parcelable, Attributable {
     * @hide
     */
    /*package*/ BluetoothDevice(String address, int addressType) {
        getService();  // ensures sService is initialized
        if (!BluetoothAdapter.checkBluetoothAddress(address)) {
            throw new IllegalArgumentException(address + " is not a valid Bluetooth address");
        }
@@ -1488,7 +1446,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public @Nullable String getIdentityAddress() {
        if (DBG) log("getIdentityAddress()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final String defaultValue = null;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot get identity address");
@@ -1518,7 +1476,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public String getName() {
        if (DBG) log("getName()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final String defaultValue = null;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot get Remote Device name");
@@ -1553,7 +1511,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public int getType() {
        if (DBG) log("getType()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = DEVICE_TYPE_UNKNOWN;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot get Remote Device type");
@@ -1582,7 +1540,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public String getAlias() {
        if (DBG) log("getAlias()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final String defaultValue = null;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot get Remote Device Alias");
@@ -1643,7 +1601,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
            throw new IllegalArgumentException("alias cannot be the empty string");
        }
        if (DBG) log("setAlias(" + alias + ")");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = BluetoothStatusCodes.ERROR_BLUETOOTH_NOT_ENABLED;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot set Remote Device name");
@@ -1677,7 +1635,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public @IntRange(from = -100, to = 100) int getBatteryLevel() {
        if (DBG) log("getBatteryLevel()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = BATTERY_LEVEL_BLUETOOTH_OFF;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "Bluetooth disabled. Cannot get remote device battery level");
@@ -1772,7 +1730,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    private boolean createBondInternal(int transport, @Nullable OobData remoteP192Data,
            @Nullable OobData remoteP256Data) {
        if (DBG) log("createBondOutOfBand()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "BT not enabled, createBondOutOfBand failed");
@@ -1805,7 +1763,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean isBondingInitiatedLocally() {
        if (DBG) log("isBondingInitiatedLocally()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "BT not enabled, isBondingInitiatedLocally failed");
@@ -1832,7 +1790,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean cancelBondProcess() {
        if (DBG) log("cancelBondProcess()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot cancel Remote Device bond");
@@ -1865,7 +1823,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean removeBond() {
        if (DBG) log("removeBond()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot remove Remote Device bond");
@@ -1960,7 +1918,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @SuppressLint("AndroidFrameworkRequiresPermission")
    public int getBondState() {
        if (DBG) log("getBondState(" + getAnonymizedAddress() + ")");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        if (service == null) {
            Log.e(TAG, "BT not enabled. Cannot get bond state");
            if (DBG) log(Log.getStackTraceString(new Throwable()));
@@ -1994,7 +1952,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean canBondWithoutDialog() {
        if (DBG) log("canBondWithoutDialog, device: " + this);
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot check if we can skip pairing dialog");
@@ -2048,7 +2006,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
        if (!BluetoothAdapter.checkBluetoothAddress(getAddress())) {
            throw new IllegalArgumentException("device cannot have an invalid address");
        }
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = BluetoothStatusCodes.ERROR_BLUETOOTH_NOT_ENABLED;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot connect to remote device.");
@@ -2095,7 +2053,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
        if (!BluetoothAdapter.checkBluetoothAddress(getAddress())) {
            throw new IllegalArgumentException("device cannot have an invalid address");
        }
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = BluetoothStatusCodes.ERROR_BLUETOOTH_NOT_ENABLED;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot disconnect to remote device.");
@@ -2127,7 +2085,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean isConnected() {
        if (DBG) log("isConnected()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = CONNECTION_STATE_DISCONNECTED;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2159,7 +2117,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean isEncrypted() {
        if (DBG) log("isEncrypted()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = CONNECTION_STATE_DISCONNECTED;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2188,7 +2146,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public BluetoothClass getBluetoothClass() {
        if (DBG) log("getBluetoothClass()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = 0;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot get Bluetooth Class");
@@ -2222,7 +2180,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public ParcelUuid[] getUuids() {
        if (DBG) log("getUuids()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final ParcelUuid[] defaultValue = null;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot get remote device Uuids");
@@ -2289,7 +2247,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean fetchUuidsWithSdp(@Transport int transport) {
        if (DBG) log("fetchUuidsWithSdp()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot fetchUuidsWithSdp");
@@ -2331,7 +2289,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean sdpSearch(ParcelUuid uuid) {
        if (DBG) log("sdpSearch()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot query remote device sdp records");
@@ -2358,7 +2316,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public boolean setPin(byte[] pin) {
        if (DBG) log("setPin()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot set Remote Device pin");
@@ -2404,7 +2362,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean setPairingConfirmation(boolean confirm) {
        if (DBG) log("setPairingConfirmation()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "BT not enabled. Cannot set pairing confirmation");
@@ -2443,7 +2401,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public @AccessPermission int getPhonebookAccessPermission() {
        if (DBG) log("getPhonebookAccessPermission()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = ACCESS_UNKNOWN;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2490,7 +2448,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean setSilenceMode(boolean silence) {
        if (DBG) log("setSilenceMode()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            throw new IllegalStateException("Bluetooth is not turned ON");
@@ -2520,7 +2478,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean isInSilenceMode() {
        if (DBG) log("isInSilenceMode()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            throw new IllegalStateException("Bluetooth is not turned ON");
@@ -2551,7 +2509,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean setPhonebookAccessPermission(@AccessPermission int value) {
        if (DBG) log("setPhonebookAccessPermission()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2580,7 +2538,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public @AccessPermission int getMessageAccessPermission() {
        if (DBG) log("getMessageAccessPermission()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = ACCESS_UNKNOWN;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2617,7 +2575,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
            throw new IllegalArgumentException(value + "is not a valid AccessPermission value");
        }
        if (DBG) log("setMessageAccessPermission()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2646,7 +2604,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)
    public @AccessPermission int getSimAccessPermission() {
        if (DBG) log("getSimAccessPermission()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final int defaultValue = ACCESS_UNKNOWN;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -2679,7 +2637,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean setSimAccessPermission(int value) {
        if (DBG) log("setSimAccessPermission()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.w(TAG, "Proxy not attached to service");
@@ -3190,7 +3148,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean setMetadata(@MetadataKey int key, @NonNull byte[] value) {
        if (DBG) log("setMetadata()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "Bluetooth is not enabled. Cannot set metadata");
@@ -3225,7 +3183,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public byte[] getMetadata(@MetadataKey int key) {
        if (DBG) log("getMetadata()");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final byte[] defaultValue = null;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "Bluetooth is not enabled. Cannot get metadata");
@@ -3268,7 +3226,7 @@ public final class BluetoothDevice implements Parcelable, Attributable {
    })
    public boolean setLowLatencyAudioAllowed(boolean allowed) {
        if (DBG) log("setLowLatencyAudioAllowed(" + allowed + ")");
        final IBluetooth service = sService;
        final IBluetooth service = getService();
        final boolean defaultValue = false;
        if (service == null || !isBluetoothEnabled()) {
            Log.e(TAG, "Bluetooth is not enabled. Cannot allow low latency");