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

Commit d491620a authored by Johanna Ye's avatar Johanna Ye
Browse files

Fix long characteristic write concurrency bug. 

Tag: #stability
Test: manual with test app
Bug: 169559728
Ignore-AOSP-First: Current security annotation parity

Merged-In: I8c017d2016c9596948bf5eb27f152284b3d375e3
Change-Id: I8c017d2016c9596948bf5eb27f152284b3d375e3
parent 12104374

android/app/src/com/android/bluetooth/gatt/GattService.java

+55 −7
Original line number Diff line number Diff line
@@ -86,6 +86,7 @@ import java.util.List; 
import java.util.Map;

import java.util.Set;

import java.util.UUID;

import java.util.concurrent.atomic.AtomicBoolean;

import java.util.concurrent.TimeUnit;

import java.util.function.Predicate;
@@ -207,6 +208,12 @@ public class GattService extends ProfileService { 
     */

    private final Map<Integer, Set<Integer>> mRestrictedHandles = new HashMap<>();



    /**

     * HashMap used to synchronize writeCharacteristic calls mapping remote device address to

     * available permit (either 1 or 0).

     */

    private final HashMap<String, AtomicBoolean> mPermits = new HashMap<>();



    private AdapterService mAdapterService;

    private AdvertiseManager mAdvertiseManager;

    private PeriodicScanManager mPeriodicScanManager;
@@ -706,13 +713,13 @@ public class GattService extends ProfileService { 
        }



        @Override

        public void writeCharacteristic(int clientIf, String address, int handle, int writeType,

        public int writeCharacteristic(int clientIf, String address, int handle, int writeType,

                int authReq, byte[] value, AttributionSource attributionSource) {

            GattService service = getService();

            if (service == null) {

                return;

                return BluetoothGatt.GATT_WRITE_REQUEST_FAIL;

            }

            service.writeCharacteristic(clientIf, address, handle, writeType, authReq, value,

            return service.writeCharacteristic(clientIf, address, handle, writeType, authReq, value,

                    attributionSource);

        }
@@ -1346,6 +1353,13 @@ public class GattService extends ProfileService { 


        if (status == 0) {

            mClientMap.addConnection(clientIf, connId, address);



            // Allow one writeCharacteristic operation at a time for each connected remote device.

            synchronized (mPermits) {

                Log.d(TAG, "onConnected() - adding permit for address="

                    + address);

                mPermits.putIfAbsent(address, new AtomicBoolean(true));

            }

        }

        ClientMap.App app = mClientMap.getById(clientIf);

        if (app != null) {
@@ -1364,6 +1378,17 @@ public class GattService extends ProfileService { 


        mClientMap.removeConnection(clientIf, connId);

        ClientMap.App app = mClientMap.getById(clientIf);



        // Remove AtomicBoolean representing permit if no other connections rely on

        // this remote device.

        if (!mClientMap.getConnectedDevices().contains(address)) {

            synchronized (mPermits) {

                Log.d(TAG, "onDisconnected() - removing permit for address="

                    + address);

                mPermits.remove(address);

            }

        }



        if (app != null) {

            app.callback.onClientConnectionState(status, clientIf, false, address);

        }
@@ -1655,6 +1680,11 @@ public class GattService extends ProfileService { 


    void onWriteCharacteristic(int connId, int status, int handle) throws RemoteException {

        String address = mClientMap.addressByConnId(connId);

        synchronized (mPermits) {

            Log.d(TAG, "onWriteCharacteristic() - increasing permit for address="

                    + address);

            mPermits.get(address).set(true);

        }



        if (VDBG) {

            Log.d(TAG, "onWriteCharacteristic() - address=" + address + ", status=" + status);
@@ -2856,11 +2886,11 @@ public class GattService extends ProfileService { 
    }



    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)

    void writeCharacteristic(int clientIf, String address, int handle, int writeType, int authReq,

    int writeCharacteristic(int clientIf, String address, int handle, int writeType, int authReq,

            byte[] value, AttributionSource attributionSource) {

        if (!Utils.checkConnectPermissionForDataDelivery(

                this, attributionSource, "GattService writeCharacteristic")) {

            return;

            return BluetoothGatt.GATT_WRITE_REQUEST_FAIL;

        }



        if (VDBG) {
@@ -2874,15 +2904,33 @@ public class GattService extends ProfileService { 
        Integer connId = mClientMap.connIdByAddress(clientIf, address);

        if (connId == null) {

            Log.e(TAG, "writeCharacteristic() - No connection for " + address + "...");

            return;

            return BluetoothGatt.GATT_WRITE_REQUEST_FAIL;

        }



        if (!permissionCheck(connId, handle)) {

            Log.w(TAG, "writeCharacteristic() - permission check failed!");

            return;

            return BluetoothGatt.GATT_WRITE_REQUEST_FAIL;

        }



        Log.d(TAG, "writeCharacteristic() - trying to acquire permit.");

        // Lock the thread until onCharacteristicWrite callback comes back.

        synchronized (mPermits) {

            AtomicBoolean atomicBoolean = mPermits.get(address);

            if (atomicBoolean == null) {

                Log.d(TAG, "writeCharacteristic() -  atomicBoolean uninitialized!");

                return BluetoothGatt.GATT_WRITE_REQUEST_FAIL;

            }



            boolean success = atomicBoolean.get();

            if (!success) {

                 Log.d(TAG, "writeCharacteristic() - no permit available.");

                 return BluetoothGatt.GATT_WRITE_REQUEST_BUSY;

            }

            atomicBoolean.set(false);

        }



        gattClientWriteCharacteristicNative(connId, handle, writeType, authReq, value);

        return BluetoothGatt.GATT_WRITE_REQUEST_SUCCESS;

    }



    @RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)