[le audio] Introduce the source sync queue to avoid race conditions
In the current design, we might hit some race conditions cases when
starting the source syncing from app and stack sides.
This commit introduce the source sync queue to address this:
1. Unify one place to start source selection/sync.
2. Check the pending sync request for adding to the queue.
3. Once previous syncing finished, sync to the next.
Bug: 317280116
Bug: 316005152
Test: atest BassClientServiceTest BassClientStateMachineTest
Test: manual test with le audio broadcast QR code path
Change-Id: I64c507bcbf1f68479a0dc11680f2b5000f481d97
diff --git a/android/app/src/com/android/bluetooth/bass_client/BassClientService.java b/android/app/src/com/android/bluetooth/bass_client/BassClientService.java
index 4831aa8..af32ed6 100644
--- a/android/app/src/com/android/bluetooth/bass_client/BassClientService.java
+++ b/android/app/src/com/android/bluetooth/bass_client/BassClientService.java
@@ -198,10 +198,10 @@
}
if (syncHandle != BassConstants.INVALID_SYNC_HANDLE) {
paRes.updateSyncHandle(syncHandle);
- if (mSyncHandleToBroadcastIdMap != null
- && paRes.getBroadcastId() != BassConstants.INVALID_BROADCAST_ID) {
- // broadcast successfully synced, update the map
- mSyncHandleToBroadcastIdMap.put(syncHandle, paRes.getBroadcastId());
+ if (paRes.getBroadcastId() != BassConstants.INVALID_BROADCAST_ID) {
+ // broadcast successfully synced
+ // update the sync handle for the broadcast source
+ updateSyncHandleForBroadcastId(syncHandle, paRes.getBroadcastId());
}
}
if (addressType != BassConstants.INVALID_ADV_ADDRESS_TYPE) {
@@ -471,6 +471,17 @@
return BassConstants.INVALID_BROADCAST_ID;
}
+ void updateSyncHandleForBroadcastId(int syncHandle, int broadcastId) {
+ if (mSyncHandleToBroadcastIdMap == null) {
+ Log.e(TAG, "mSyncHandleToBroadcastIdMap is null");
+ return;
+ }
+
+ mSyncHandleToBroadcastIdMap.entrySet().removeIf(entry -> entry.getValue() == broadcastId);
+ mSyncHandleToBroadcastIdMap.put(syncHandle, broadcastId);
+ log("Updated mSyncHandleToBroadcastIdMap: " + mSyncHandleToBroadcastIdMap);
+ }
+
private static synchronized void setBassClientService(BassClientService instance) {
if (DBG) {
Log.d(TAG, "setBassClientService(): set to: " + instance);
diff --git a/android/app/src/com/android/bluetooth/bass_client/BassClientStateMachine.java b/android/app/src/com/android/bluetooth/bass_client/BassClientStateMachine.java
index bb366ca..5d34b43 100644
--- a/android/app/src/com/android/bluetooth/bass_client/BassClientStateMachine.java
+++ b/android/app/src/com/android/bluetooth/bass_client/BassClientStateMachine.java
@@ -49,6 +49,7 @@
import android.os.ParcelUuid;
import android.provider.DeviceConfig;
import android.util.Log;
+import android.util.Pair;
import com.android.bluetooth.BluetoothMethodProxy;
import com.android.bluetooth.Utils;
@@ -130,6 +131,8 @@
private final Connecting mConnecting = new Connecting();
private final ConnectedProcessing mConnectedProcessing = new ConnectedProcessing();
private final FeatureFlags mFeatureFlags;
+ private final List<Pair<ScanResult, Integer>> mSourceSyncRequestsQueue =
+ new ArrayList<Pair<ScanResult, Integer>>();
@VisibleForTesting
final List<BluetoothGattCharacteristic> mBroadcastCharacteristics =
@@ -263,6 +266,7 @@
mCurrentMetadata.clear();
mPendingRemove.clear();
mPeriodicAdvCallbacksMap.clear();
+ mSourceSyncRequestsQueue.clear();
}
Boolean hasPendingSourceOperation() {
@@ -427,8 +431,8 @@
// null if no name present
String broadcastName = checkAndParseBroadcastName(scanRecord);
- // Avoid duplicated sync requests for the same broadcast BIG
- if (isDuplicatedSyncRequest(broadcastId)) {
+ // Avoid duplicated sync request if the same broadcast BIG is synced
+ if (isSourceSynced(broadcastId)) {
log("Skip duplicated sync request to broadcast id: " + broadcastId);
return false;
}
@@ -462,22 +466,18 @@
return true;
}
- private boolean isDuplicatedSyncRequest(int broadcastId) {
- // Either there is active sync to the same broadcast id or pending operation
- // This is required because selectSource can be triggered from both scanning(user)
- // and adding inactive source(auto)
+ private boolean isSourceSynced(int broadcastId) {
List<Integer> activeSyncedSrc = mService.getActiveSyncedSources(mDevice);
- if ((mPendingSourceToAdd != null && broadcastId == mPendingSourceToAdd.getBroadcastId())
- || (activeSyncedSrc != null
- && activeSyncedSrc.contains(
- mService.getSyncHandleForBroadcastId(broadcastId)))) {
- return true;
- }
- return false;
+ return (activeSyncedSrc != null
+ && activeSyncedSrc.contains(mService.getSyncHandleForBroadcastId(broadcastId)));
}
private void cancelActiveSync(Integer syncHandle) {
log("cancelActiveSync: syncHandle = " + syncHandle);
+ if (syncHandle == null) {
+ // clean up the pending sync request if syncHandle is null
+ mPeriodicAdvCallbacksMap.remove(BassConstants.INVALID_SYNC_HANDLE);
+ }
List<Integer> activeSyncedSrc = mService.getActiveSyncedSources(mDevice);
/* Stop sync if there is some running */
@@ -514,6 +514,7 @@
Log.w(TAG, "unregisterSync:IllegalArgumentException");
return false;
}
+ mPeriodicAdvCallbacksMap.remove(syncHandle);
} else {
log("calling unregisterSync, not found syncHandle: " + syncHandle);
}
@@ -1130,6 +1131,14 @@
mPeriodicAdvCallbacksMap.remove(BassConstants.INVALID_SYNC_HANDLE);
}
mPendingSourceToAdd = null;
+ if (!mSourceSyncRequestsQueue.isEmpty()) {
+ log("Processing the next source to sync");
+ Pair<ScanResult, Integer> queuedSourceToSync = mSourceSyncRequestsQueue.remove(0);
+ Message msg = obtainMessage(SELECT_BCAST_SOURCE);
+ msg.obj = queuedSourceToSync.first;
+ msg.arg1 = queuedSourceToSync.second;
+ sendMessage(msg);
+ }
}
@Override
@@ -1727,7 +1736,16 @@
case SELECT_BCAST_SOURCE:
ScanResult scanRes = (ScanResult) message.obj;
boolean auto = ((int) message.arg1) == BassConstants.AUTO;
- selectSource(scanRes, auto);
+ // check if invalid sync handle exists indicating a pending sync request
+ if (mPeriodicAdvCallbacksMap.containsKey(BassConstants.INVALID_SYNC_HANDLE)) {
+ log(
+ "SELECT_BCAST_SOURCE queued due to waiting for a previous sync"
+ + " response");
+ mSourceSyncRequestsQueue.add(
+ new Pair<ScanResult, Integer>(scanRes, message.arg1));
+ } else {
+ selectSource(scanRes, auto);
+ }
break;
case REACHED_MAX_SOURCE_LIMIT:
int handle = message.arg1;
@@ -1777,7 +1795,10 @@
&& mService.getCachedBroadcast(broadcastId) != null) {
// If the source has been synced before, try to re-sync(auto/true)
// with the source by previously cached scan result
- selectSource(mService.getCachedBroadcast(broadcastId), true);
+ Message msg = obtainMessage(SELECT_BCAST_SOURCE);
+ msg.obj = mService.getCachedBroadcast(broadcastId);
+ msg.arg1 = BassConstants.AUTO;
+ sendMessage(msg);
mPendingSourceToAdd = metaData;
} else {
mService.getCallbacks().notifySourceAddFailed(mDevice, metaData,
diff --git a/android/app/tests/unit/src/com/android/bluetooth/bass_client/BassClientStateMachineTest.java b/android/app/tests/unit/src/com/android/bluetooth/bass_client/BassClientStateMachineTest.java
index b51bc5b..05a85f2 100644
--- a/android/app/tests/unit/src/com/android/bluetooth/bass_client/BassClientStateMachineTest.java
+++ b/android/app/tests/unit/src/com/android/bluetooth/bass_client/BassClientStateMachineTest.java
@@ -1741,25 +1741,16 @@
};
ScanRecord record = ScanRecord.parseFromBytes(scanRecord);
ScanResult scanResult = new ScanResult(mTestDevice, 0, 0, 0, 0, 0, 0, 0, record, 0);
- // validate pending duplicated request will be skipped
- mBassClientStateMachine.mPendingSourceToAdd = createBroadcastMetadata();
-
- doNothing().when(mMethodProxy).periodicAdvertisingManagerRegisterSync(
- any(), any(), anyInt(), anyInt(), any(), any());
- mBassClientStateMachine.sendMessage(
- SELECT_BCAST_SOURCE, BassConstants.AUTO, 0, scanResult);
- TestUtils.waitForLooperToFinishScheduledTask(mHandlerThread.getLooper());
- verify(mBassClientService, never()).updatePeriodicAdvertisementResultMap(
- any(), anyInt(), anyInt(), anyInt(), anyInt(), anyInt(),
- any(), any());
-
- mBassClientStateMachine.mPendingSourceToAdd = null;
List<Integer> activeSyncedSrc = new ArrayList<>();
activeSyncedSrc.add(testSyncHandle);
// need this to ensure expected mock behavior for getActiveSyncedSource
when(mBassClientService.getActiveSyncedSources(any())).thenReturn(activeSyncedSrc);
when(mBassClientService.getSyncHandleForBroadcastId(anyInt())).thenReturn(testSyncHandle);
+ doNothing()
+ .when(mMethodProxy)
+ .periodicAdvertisingManagerRegisterSync(
+ any(), any(), anyInt(), anyInt(), any(), any());
mBassClientStateMachine.sendMessage(SELECT_BCAST_SOURCE, BassConstants.AUTO, 0, scanResult);
TestUtils.waitForLooperToFinishScheduledTask(mHandlerThread.getLooper());
