Merge "Fix long characteristic write concurrency bug." into sc-dev-plus-aosp (2ad31b74) · Commits · e / os / android_packages_modules_Bluetooth

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;

		@@ -215,6 +216,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;
		@@ -714,13 +721,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);
		}

		@@ -1354,6 +1361,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) {
		@@ -1372,6 +1386,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);
		}
		@@ -1663,6 +1688,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);
		@@ -2868,11 +2898,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) {
		@@ -2886,15 +2916,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)