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

Commit 968838af 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
Merged-In: I8c017d2016c9596948bf5eb27f152284b3d375e3
Change-Id: I15c05cede2ead3d95099c8cbe189a8f3cc383864
parent 87faf603
Loading
Loading
Loading
Loading
+55 −6
Original line number Diff line number Diff line
@@ -80,6 +80,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;

/**
@@ -204,6 +205,12 @@ public class GattService extends ProfileService {
     */
    private Set<String> mReliableQueue = new HashSet<String>();

    /**
     * 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<>();

    static {
        classInitNative();
    }
@@ -622,13 +629,14 @@ 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) {
            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);
        }

        @Override
@@ -1215,6 +1223,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) {
@@ -1233,6 +1248,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);
        }
@@ -1524,6 +1550,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);
@@ -2545,7 +2576,7 @@ public class GattService extends ProfileService {
                uuid.getMostSignificantBits(), startHandle, endHandle, authReq);
    }

    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) {
        enforceCallingOrSelfPermission(BLUETOOTH_PERM, "Need BLUETOOTH permission");

@@ -2560,15 +2591,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;
    }

    void readDescriptor(int clientIf, String address, int handle, int authReq) {