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

Commit bf21f6db authored by Jie Jiang's avatar Jie Jiang
Browse files

BondStateMachine: avoid adding device to mDevices 

Currently, BondStateMachine will add the device to mDevices list if the
device is not found in that list when the bond state is changed to
BOND_NONE or BOND_BONDED.

Since the action for a device in mDevices may get deferred based on its
type, this may cause some problems: Suppose two devices (A and B) has
already been paired, and consider the following sequence:
1. removeBond() on A is called, and then the state machine will transit
into PendingCommandState, and A will not be added into mDevices
according to the current logic.
2. removeBond() on B is called, and then it will be added into mDevices.
3. The event about the bond state of A changed to BOND_NONE comes, since
A is not in mDevices, it will be added to the list.
4. The event about the bond state of B changed to BOND_NONE comes, B
will be removed from mDevices, and the state machine will not go back to
StableState since mDevices is not empty now.

Two problems may happen at the end of this example:
a) The state machine is leaving in PendingCommandState, while there are
no devices are in bonding/unbonding state;
b) The further createBond() requests will be deferred since A is still
in mDevices list.

The only usage of mDevices is to avoid sending some commands when the
device is performing another action. It does not make sense that the
event of the state of a device changed to BOND_BONDED or BOND_NONE could
add this device into the list.

Also added a unit test for the above scenario.

Test: the new added unit test passed, and it will fail without this
patch.
Test: atest BondStateMachineTest

Change-Id: I41f5eeb101a88e9afb45c1760b017363d0a08814
parent c86b3419

android/app/src/com/android/bluetooth/btservice/AdapterService.java

+5 −5
Original line number Diff line number Diff line
@@ -2612,15 +2612,15 @@ public class AdapterService extends Service { 
        editor.apply();

    }



    void setPhonebookAccessPermission(BluetoothDevice device, int value) {

    public void setPhonebookAccessPermission(BluetoothDevice device, int value) {

        setDeviceAccessFromPrefs(device, value, PHONEBOOK_ACCESS_PERMISSION_PREFERENCE_FILE);

    }



    void setMessageAccessPermission(BluetoothDevice device, int value) {

    public void setMessageAccessPermission(BluetoothDevice device, int value) {

        setDeviceAccessFromPrefs(device, value, MESSAGE_ACCESS_PERMISSION_PREFERENCE_FILE);

    }



    void setSimAccessPermission(BluetoothDevice device, int value) {

    public void setSimAccessPermission(BluetoothDevice device, int value) {

        setDeviceAccessFromPrefs(device, value, SIM_ACCESS_PERMISSION_PREFERENCE_FILE);

    }
@@ -3077,13 +3077,13 @@ public class AdapterService extends Service { 
    native boolean getDevicePropertyNative(byte[] address, int type);



    /*package*/

    native boolean createBondNative(byte[] address, int transport);

    public native boolean createBondNative(byte[] address, int transport);



    /*package*/

    native boolean createBondOutOfBandNative(byte[] address, int transport, OobData oobData);



    /*package*/

    native boolean removeBondNative(byte[] address);

    public native boolean removeBondNative(byte[] address);



    /*package*/

    native boolean cancelBondNative(byte[] address);

android/app/src/com/android/bluetooth/btservice/BondStateMachine.java

+6 −8
Original line number Diff line number Diff line
@@ -214,14 +214,11 @@ final class BondStateMachine extends StateMachine { 
                    }

                    sendIntent(dev, newState, reason);

                    if (newState != BluetoothDevice.BOND_BONDING) {

                        /* this is either none/bonded, remove and transition */

                        result = !mDevices.remove(dev);

                        if (mDevices.isEmpty()) {

                            // Whenever mDevices is empty, then we need to

                            // set result=false. Else, we will end up adding

                            // the device to the list again. This prevents us

                            // from pairing with a device that we just unpaired

                        // This is either none/bonded, remove and transition, and also set

                        // result=false to avoid adding the device to mDevices.

                        mDevices.remove(dev);

                        result = false;

                        if (mDevices.isEmpty()) {

                            transitionTo(mStableState);

                        }

                        if (newState == BluetoothDevice.BOND_NONE) {
@@ -297,7 +294,8 @@ final class BondStateMachine extends StateMachine { 
    }



    private boolean removeBond(BluetoothDevice dev, boolean transition) {

        if (dev.getBondState() == BluetoothDevice.BOND_BONDED) {

        DeviceProperties devProp = mRemoteDevices.getDeviceProperties(dev);

        if (devProp != null && devProp.getBondState() == BluetoothDevice.BOND_BONDED) {

            byte[] addr = Utils.getBytesFromAddress(dev.getAddress());

            if (!mAdapterService.removeBondNative(addr)) {

                Log.e(TAG, "Unexpected error while removing bond:");

android/app/tests/unit/src/com/android/bluetooth/btservice/BondStateMachineTest.java

+51 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.bluetooth.BluetoothDevice; 
import android.content.Context;

import android.content.Intent;

import android.os.HandlerThread;

import android.os.Message;

import android.os.ParcelUuid;

import android.os.UserHandle;
@@ -44,6 +45,7 @@ import org.mockito.MockitoAnnotations; 
public class BondStateMachineTest {

    private static final int TEST_BOND_REASON = 0;

    private static final byte[] TEST_BT_ADDR_BYTES = {00, 11, 22, 33, 44, 55};

    private static final byte[] TEST_BT_ADDR_BYTES_2 = {00, 11, 22, 33, 44, 66};

    private static final ParcelUuid[] TEST_UUIDS =

            {ParcelUuid.fromString("0000111E-0000-1000-8000-00805F9B34FB")};
@@ -86,6 +88,55 @@ public class BondStateMachineTest { 
        TestUtils.clearAdapterService(mAdapterService);

    }



    @Test

    public void testCreateBondAfterRemoveBond() {

        // Set up two devices already bonded.

        mRemoteDevices.reset();

        RemoteDevices.DeviceProperties deviceProperties1, deviceProperties2;

        deviceProperties1 = mRemoteDevices.addDeviceProperties(TEST_BT_ADDR_BYTES);

        deviceProperties2 = mRemoteDevices.addDeviceProperties(TEST_BT_ADDR_BYTES_2);

        BluetoothDevice device1, device2;

        device1 = mRemoteDevices.getDevice(TEST_BT_ADDR_BYTES);

        device2 = mRemoteDevices.getDevice(TEST_BT_ADDR_BYTES_2);

        deviceProperties1.mBondState = BOND_BONDED;

        deviceProperties2.mBondState = BOND_BONDED;



        doReturn(true).when(mAdapterService).removeBondNative(any(byte[].class));

        doReturn(true).when(mAdapterService).createBondNative(any(byte[].class), anyInt());



        // The removeBond() request for a bonded device should invoke the removeBondNative() call.

        Message removeBondMsg1 = mBondStateMachine.obtainMessage(BondStateMachine.REMOVE_BOND);

        removeBondMsg1.obj = device1;

        mBondStateMachine.sendMessage(removeBondMsg1);

        TestUtils.waitForLooperToFinishScheduledTask(mBondStateMachine.getHandler().getLooper());

        Message removeBondMsg2 = mBondStateMachine.obtainMessage(BondStateMachine.REMOVE_BOND);

        removeBondMsg2.obj = device2;

        mBondStateMachine.sendMessage(removeBondMsg2);

        TestUtils.waitForLooperToFinishScheduledTask(mBondStateMachine.getHandler().getLooper());



        verify(mAdapterService, times(1)).removeBondNative(eq(TEST_BT_ADDR_BYTES));

        verify(mAdapterService, times(1)).removeBondNative(eq(TEST_BT_ADDR_BYTES_2));



        mBondStateMachine.bondStateChangeCallback(AbstractionLayer.BT_STATUS_SUCCESS,

                TEST_BT_ADDR_BYTES, BOND_NONE);

        TestUtils.waitForLooperToFinishScheduledTask(mBondStateMachine.getHandler().getLooper());

        mBondStateMachine.bondStateChangeCallback(AbstractionLayer.BT_STATUS_SUCCESS,

                TEST_BT_ADDR_BYTES_2, BOND_NONE);

        TestUtils.waitForLooperToFinishScheduledTask(mBondStateMachine.getHandler().getLooper());



        // Try to pair these two devices again, createBondNative() should be invoked.

        Message createBondMsg1 = mBondStateMachine.obtainMessage(BondStateMachine.CREATE_BOND);

        createBondMsg1.obj = device1;

        mBondStateMachine.sendMessage(createBondMsg1);

        Message createBondMsg2 = mBondStateMachine.obtainMessage(BondStateMachine.CREATE_BOND);

        createBondMsg2.obj = device2;

        mBondStateMachine.sendMessage(createBondMsg2);

        TestUtils.waitForLooperToFinishScheduledTask(mBondStateMachine.getHandler().getLooper());



        verify(mAdapterService, times(1)).createBondNative(eq(TEST_BT_ADDR_BYTES), anyInt());

        verify(mAdapterService, times(1)).createBondNative(eq(TEST_BT_ADDR_BYTES_2), anyInt());

    }



    @Test

    public void testSendIntent() {

        int badBondState = 42;