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

Commit 7703328a authored by Pavlin Radoslavov's avatar Pavlin Radoslavov
Browse files

Update the A2dpService logic for creating/deleting state machine instances 

* Create a new state machine instance for native stack events only if the
  event is CONNECTION_STATE_CONNECTED or CONNECTION_STATE_CONNECTING
* Add an early check to reject connect() request if there are too many
  connected devices.
* Fixed the logic inside updateOptionalCodecsSupport() so it cannot
  create a new state machine instance
* Unbonding a device will remove the state machine instance only
  if the device is disconnected
* If a device is disconnected, remove the state machine if
  the device is already unbond

Also:
 * Renamed canConnectToDevice() to connectionAllowedCheckMaxDevices()
 * Add new method A2dpService.getDevices() - used only for testing
 * Add a missing transition in the A2dpStateMachine from Connected
   to Disconnecting on A2dpStackEvent.CONNECTION_STATE_DISCONNECTING
   event.
 * Fix the logic for checking the maximum number of state machines
   to avoid DoS attack
 * Add checks that bond state events and native stack events
   contain a device
 * Change the A2dpServiceTest setting to use MAX_CONNECTED_AUDIO_DEVICES
   of 5 by default.
 * Add internal mechanism in A2dpServiceTest to
   - waitForNoIntent() - Wait and verify that no intent has been received
   - verifyNoConnectionStateIntent() - Wait and verify that no connection
     state intent has been received
 * Add unit tests:
   - testOutgoingConnectPriorityOff()
   - testMaxConnectDevices()
   - testCreateStateMachineStackEvents()
   - testDeleteStateMachineUnbondEvents()
   - testDeleteStateMachineDisconnectEvents()
 * Add a new helper method TestUtils.getTestDevice()
 * Minor cleanup: arguments renaming, etc.

Bug: 73212853
Test: Unit tests added:
    runtest bluetooth --test-class com.android.bluetooth.a2dp.A2dpServiceTest
Change-Id: I484fcc04b7ce800df1a0e1cf8930816edae6ab91
parent be03bfe5

android/app/src/com/android/bluetooth/a2dp/A2dpService.java

+108 −26
Original line number Diff line number Diff line
@@ -207,6 +207,7 @@ public class A2dpService extends ProfileService { 
        }



        if (getPriority(device) == BluetoothProfile.PRIORITY_OFF) {

            Log.e(TAG, "Cannot connect to " + device + " : PRIORITY_OFF");

            return false;

        }

        if (!BluetoothUuid.isUuidPresent(mAdapterService.getRemoteUuids(device),
@@ -216,6 +217,10 @@ public class A2dpService extends ProfileService { 
        }



        synchronized (mStateMachines) {

            if (!connectionAllowedCheckMaxDevices(device)) {

                Log.e(TAG, "Cannot connect to " + device + " : too many connected devices");

                return false;

            }

            A2dpStateMachine smConnect = getOrCreateStateMachine(device);

            if (smConnect == null) {

                Log.e(TAG, "Cannot connect to " + device + " : no state machine");
@@ -263,8 +268,7 @@ public class A2dpService extends ProfileService { 
     * @param device the peer device to connect to

     * @return true if connection is allowed, otherwise false

     */

    @VisibleForTesting

    public boolean canConnectToDevice(BluetoothDevice device) {

    private boolean connectionAllowedCheckMaxDevices(BluetoothDevice device) {

        int connected = 0;

        // Count devices that are in the process of connecting or already connected

        synchronized (mStateMachines) {
@@ -300,7 +304,7 @@ public class A2dpService extends ProfileService { 
            return false;

        }

        // Check if too many devices

        if (!canConnectToDevice(device)) {

        if (!connectionAllowedCheckMaxDevices(device)) {

            Log.e(TAG, "okToConnect: cannot connect to " + device

                    + " : too many connected devices");

            return false;
@@ -348,6 +352,22 @@ public class A2dpService extends ProfileService { 
        }

    }



    /**

     * Get the list of devices that have state machines.

     *

     * @return the list of devices that have state machines

     */

    @VisibleForTesting

    List<BluetoothDevice> getDevices() {

        List<BluetoothDevice> devices = new ArrayList<>();

        synchronized (mStateMachines) {

            for (A2dpStateMachine sm : mStateMachines.values()) {

                devices.add(sm.getDevice());

            }

            return devices;

        }

    }



    public int getConnectionState(BluetoothDevice device) {

        enforceCallingOrSelfPermission(BLUETOOTH_PERM, "Need BLUETOOTH permission");

        synchronized (mStateMachines) {
@@ -639,12 +659,30 @@ public class A2dpService extends ProfileService { 


    // Handle messages from native (JNI) to Java

    void messageFromNative(A2dpStackEvent stackEvent) {

        Objects.requireNonNull(stackEvent.device,

                               "Device should never be null, event: " + stackEvent);

        synchronized (mStateMachines) {

            A2dpStateMachine sm = null;

            BluetoothDevice device = stackEvent.device;

            A2dpStateMachine sm = getOrCreateStateMachine(device);

            if (stackEvent.type == A2dpStackEvent.EVENT_TYPE_CONNECTION_STATE_CHANGED) {

                switch (stackEvent.valueInt) {

                    case A2dpStackEvent.CONNECTION_STATE_CONNECTED:

                    case A2dpStackEvent.CONNECTION_STATE_CONNECTING:

                        // Create a new state machine only when connecting to a device

                        if (!connectionAllowedCheckMaxDevices(device)) {

                            Log.e(TAG, "Cannot connect to " + device

                                    + " : too many connected devices");

                            return;

                        }

                        sm = getOrCreateStateMachine(device);

                        break;

                    default:

                        sm = mStateMachines.get(device);

                        break;

                }

            }

            if (sm == null) {

                Log.e(TAG, "Cannot process stack event: no state machine: "

                        + stackEvent);

                Log.e(TAG, "Cannot process stack event: no state machine: " + stackEvent);

                return;

            }

            sm.sendMessage(A2dpStateMachine.STACK_EVENT, stackEvent);
@@ -682,7 +720,7 @@ public class A2dpService extends ProfileService { 
                return sm;

            }

            // Limit the maximum number of state machines to avoid DoS attack

            if (mStateMachines.size() > MAX_A2DP_STATE_MACHINES) {

            if (mStateMachines.size() >= MAX_A2DP_STATE_MACHINES) {

                Log.e(TAG, "Maximum number of A2DP state machines reached: "

                        + MAX_A2DP_STATE_MACHINES);

                return null;
@@ -721,7 +759,6 @@ public class A2dpService extends ProfileService { 
        sendBroadcast(intent, A2dpService.BLUETOOTH_PERM);

    }



    // Remove state machine if the bonding for a device is removed

    private class BondStateChangedReceiver extends BroadcastReceiver {

        @Override

        public void onReceive(Context context, Intent intent) {
@@ -731,10 +768,27 @@ public class A2dpService extends ProfileService { 
            int state = intent.getIntExtra(BluetoothDevice.EXTRA_BOND_STATE,

                                           BluetoothDevice.ERROR);

            BluetoothDevice device = intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE);

            Objects.requireNonNull(device, "ACTION_BOND_STATE_CHANGED with no EXTRA_DEVICE");

            bondStateChanged(device, state);

        }

    }



    /**

     * Process a change in the bonding state for a device.

     *

     * @param device the device whose bonding state has changed

     * @param bondState the new bond state for the device. Possible values are:

     * {@link BluetoothDevice#BOND_NONE},

     * {@link BluetoothDevice#BOND_BONDING},

     * {@link BluetoothDevice#BOND_BONDED}.

     */

    @VisibleForTesting

    void bondStateChanged(BluetoothDevice device, int bondState) {

        if (DBG) {

                Log.d(TAG, "Bond state changed for device: " + device + " state: " + state);

            Log.d(TAG, "Bond state changed for device: " + device + " state: " + bondState);

        }

            if (state != BluetoothDevice.BOND_NONE) {

        // Remove state machine if the bonding for a device is removed

        if (bondState != BluetoothDevice.BOND_NONE) {

            return;

        }

        synchronized (mStateMachines) {
@@ -742,22 +796,34 @@ public class A2dpService extends ProfileService { 
            if (sm == null) {

                return;

            }

                if (DBG) {

                    Log.d(TAG, "Removing state machine for device: " + device);

            if (sm.getConnectionState() != BluetoothProfile.STATE_DISCONNECTED) {

                return;

            }

            removeStateMachine(device);

        }

    }



    private void removeStateMachine(BluetoothDevice device) {

        synchronized (mStateMachines) {

            A2dpStateMachine sm = mStateMachines.get(device);

            if (sm == null) {

                Log.w(TAG, "removeStateMachine: device " + device

                        + " does not have a state machine");

                return;

            }

            Log.i(TAG, "removeStateMachine: removing state machine for device: " + device);

            sm.doQuit();

            sm.cleanup();

            mStateMachines.remove(device);

        }

    }

    }



    private void updateOptionalCodecsSupport(BluetoothDevice device) {

        int previousSupport = getSupportsOptionalCodecs(device);

        boolean supportsOptional = false;



        synchronized (mStateMachines) {

            A2dpStateMachine sm = getOrCreateStateMachine(device);

            A2dpStateMachine sm = mStateMachines.get(device);

            if (sm == null) {

                return;

            }
@@ -801,14 +867,28 @@ public class A2dpService extends ProfileService { 
            if (toState == BluetoothProfile.STATE_CONNECTED && (mMaxConnectedAudioDevices == 1)) {

                setActiveDevice(device);

            }

            // Check if the active device has been disconnected

            // Check if the active device is not connected anymore

            if (isActiveDevice(device) && (fromState == BluetoothProfile.STATE_CONNECTED)) {

                setActiveDevice(null);

            }

            // Check if the device is disconnected - if unbond, remove the state machine

            if (toState == BluetoothProfile.STATE_DISCONNECTED) {

                int bondState = mAdapterService.getBondState(device);

                if (bondState == BluetoothDevice.BOND_NONE) {

                    removeStateMachine(device);

                }

            }

        }

    }



    // Update codec support per device when device is (re)connected.

    /**

     * Receiver for processing device connection state changes.

     *

     * <ul>

     * <li> Update codec support per device when device is (re)connected

     * <li> Delete the state machine instance if the device is disconnected and unbond

     * </ul>

     */

    private class ConnectionStateChangedReceiver extends BroadcastReceiver {

        @Override

        public void onReceive(Context context, Intent intent) {
@@ -822,7 +902,9 @@ public class A2dpService extends ProfileService { 
        }

    }



    // Binder object: Must be static class or memory leak may occur

    /**

     * Binder object: must be a static class or memory leak may occur.

     */

    @VisibleForTesting

    static class BluetoothA2dpBinder extends IBluetoothA2dp.Stub

            implements IProfileServiceBinder {

android/app/src/com/android/bluetooth/a2dp/A2dpStateMachine.java

+22 −12
Original line number Diff line number Diff line
@@ -223,8 +223,8 @@ final class A2dpStateMachine extends StateMachine { 
        }



        // in Disconnected state

        private void processConnectionEvent(int state) {

            switch (state) {

        private void processConnectionEvent(int event) {

            switch (event) {

                case A2dpStackEvent.CONNECTION_STATE_DISCONNECTED:

                    Log.w(TAG, "Ignore A2DP DISCONNECTED event: " + mDevice);

                    break;
@@ -253,7 +253,7 @@ final class A2dpStateMachine extends StateMachine { 
                    Log.w(TAG, "Ignore A2DP DISCONNECTING event: " + mDevice);

                    break;

                default:

                    Log.e(TAG, "Incorrect state: " + state + " device: " + mDevice);

                    Log.e(TAG, "Incorrect event: " + event + " device: " + mDevice);

                    break;

            }

        }
@@ -333,8 +333,8 @@ final class A2dpStateMachine extends StateMachine { 
        }



        // in Connecting state

        private void processConnectionEvent(int state) {

            switch (state) {

        private void processConnectionEvent(int event) {

            switch (event) {

                case A2dpStackEvent.CONNECTION_STATE_DISCONNECTED:

                    Log.w(TAG, "Connecting device disconnected: " + mDevice);

                    transitionTo(mDisconnected);
@@ -350,7 +350,7 @@ final class A2dpStateMachine extends StateMachine { 
                    transitionTo(mDisconnecting);

                    break;

                default:

                    Log.e(TAG, "Incorrect state: " + state);

                    Log.e(TAG, "Incorrect event: " + event);

                    break;

            }

        }
@@ -427,8 +427,8 @@ final class A2dpStateMachine extends StateMachine { 
        }



        // in Disconnecting state

        private void processConnectionEvent(int state) {

            switch (state) {

        private void processConnectionEvent(int event) {

            switch (event) {

                case A2dpStackEvent.CONNECTION_STATE_DISCONNECTED:

                    Log.i(TAG, "Disconnected: " + mDevice);

                    transitionTo(mDisconnected);
@@ -457,7 +457,7 @@ final class A2dpStateMachine extends StateMachine { 
                    // We are already disconnecting, do nothing

                    break;

                default:

                    Log.e(TAG, "Incorrect state: " + state);

                    Log.e(TAG, "Incorrect event: " + event);

                    break;

            }

        }
@@ -546,14 +546,24 @@ final class A2dpStateMachine extends StateMachine { 
        }



        // in Connected state

        private void processConnectionEvent(int state) {

            switch (state) {

        private void processConnectionEvent(int event) {

            switch (event) {

                case A2dpStackEvent.CONNECTION_STATE_DISCONNECTED:

                    Log.i(TAG, "Disconnected from " + mDevice);

                    transitionTo(mDisconnected);

                    break;

                case A2dpStackEvent.CONNECTION_STATE_CONNECTED:

                    Log.w(TAG, "Ignore A2DP CONNECTED event: " + mDevice);

                    break;

                case A2dpStackEvent.CONNECTION_STATE_CONNECTING:

                    Log.w(TAG, "Ignore A2DP CONNECTING event: " + mDevice);

                    break;

                case A2dpStackEvent.CONNECTION_STATE_DISCONNECTING:

                    Log.i(TAG, "Disconnecting from " + mDevice);

                    transitionTo(mDisconnecting);

                    break;

                default:

                    Log.e(TAG, "Connection State Device: " + mDevice + " bad state: " + state);

                    Log.e(TAG, "Connection State Device: " + mDevice + " bad event: " + event);

                    break;

            }

        }

android/app/tests/unit/src/com/android/bluetooth/TestUtils.java

+17 −0
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ import static org.mockito.ArgumentMatchers.eq; 
import static org.mockito.Mockito.*;



import android.bluetooth.BluetoothAdapter;

import android.bluetooth.BluetoothDevice;

import android.content.Intent;

import android.support.test.InstrumentationRegistry;

import android.support.test.rule.ServiceTestRule;
@@ -148,4 +149,20 @@ public class TestUtils { 
        verify(adapterService, timeout(SERVICE_TOGGLE_TIMEOUT_MS)).onProfileServiceStateChanged(

                eq(profileServiceClass.getName()), eq(BluetoothAdapter.STATE_OFF));

    }



    /**

     * Create a test device.

     *

     * @param bluetoothAdapter the Bluetooth adapter to use

     * @param id the test device ID. It must be an integer in the interval [0, 0xFF].

     * @return {@link BluetoothDevice} test device for the device ID

     */

    public static BluetoothDevice getTestDevice(BluetoothAdapter bluetoothAdapter, int id) {

        Assert.assertTrue(id <= 0xFF);

        Assert.assertNotNull(bluetoothAdapter);

        BluetoothDevice testDevice =

                bluetoothAdapter.getRemoteDevice(String.format("00:01:02:03:04:%02X", id));

        Assert.assertNotNull(testDevice);

        return testDevice;

    }

}

android/app/tests/unit/src/com/android/bluetooth/a2dp/A2dpServiceTest.java

+285 −3

File changed.

Preview size limit exceeded, changes collapsed.