From b087eae2db98c638abe30571f50c7917a628ae32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Rymanowski?= <lukasz.rymanowski@codecoup.pl>
Date: Wed, 3 Nov 2021 07:37:45 +0000
Subject: [PATCH] leaudio: Fix crash when removing device while streaming
With this patch:
1) Device is removed from the group when it is disconnected
2) Empty group is removed only when related CIG does not exist
Bug: 204945451
Bug: 150670922
Tag: #feature
Sponsor: jpawlowski@
Test: atest --host bluetooth_le_audio_test bluetooth_le_audio_client_test
Change-Id: I7079594253a504cd0320c6d4000c3d71cb6f0883
---
system/bta/le_audio/client.cc | 30 ++++---
system/bta/le_audio/le_audio_client_test.cc | 86 ++++++++++++++++++++-
2 files changed, 105 insertions(+), 11 deletions(-)
diff --git a/system/bta/le_audio/client.cc b/system/bta/le_audio/client.cc
index 6522d76a63b..763f8ebf5af 100644
--- a/system/bta/le_audio/client.cc
+++ b/system/bta/le_audio/client.cc
@@ -414,6 +414,12 @@ class LeAudioClientImpl : public LeAudioClient {
group_id);
}
+ void remove_group_if_possible(LeAudioDeviceGroup* group) {
+ if (group && group->IsEmpty() && !group->cig_created_) {
+ aseGroups_.Remove(group->group_id_);
+ }
+ }
+
void group_remove_node(LeAudioDeviceGroup* group, const RawAddress& address,
bool update_group_module = false) {
int group_id = group->group_id_;
@@ -431,8 +437,7 @@ class LeAudioClientImpl : public LeAudioClient {
/* Remove group if this was the last leAudioDevice in this group */
if (group->IsEmpty()) {
- aseGroups_.Remove(group_id);
-
+ remove_group_if_possible(group);
return;
}
@@ -657,6 +662,12 @@ class LeAudioClientImpl : public LeAudioClient {
return;
}
+ if (leAudioDevice->conn_id_ != GATT_INVALID_CONN_ID) {
+ Disconnect(address);
+ leAudioDevice->removing_device_ = true;
+ return;
+ }
+
/* Remove the group assignment if not yet removed. It might happen that the
* group module has already called the appropriate callback and we have
* already removed the group assignment.
@@ -666,12 +677,6 @@ class LeAudioClientImpl : public LeAudioClient {
group_remove_node(group, address, true);
}
- if (leAudioDevice->conn_id_ != GATT_INVALID_CONN_ID) {
- Disconnect(address);
- leAudioDevice->removing_device_ = true;
- return;
- }
-
leAudioDevices_.Remove(address);
}
@@ -1181,7 +1186,13 @@ class LeAudioClientImpl : public LeAudioClient {
leAudioDevice->conn_id_ = GATT_INVALID_CONN_ID;
leAudioDevice->encrypted_ = false;
- if (leAudioDevice->removing_device_) leAudioDevices_.Remove(address);
+ if (leAudioDevice->removing_device_) {
+ if (leAudioDevice->group_id_ != bluetooth::groups::kGroupUnknown) {
+ auto group = aseGroups_.FindById(leAudioDevice->group_id_);
+ group_remove_node(group, address, true);
+ }
+ leAudioDevices_.Remove(address);
+ }
}
bool subscribe_for_indications(uint16_t conn_id, const RawAddress& address,
@@ -2524,6 +2535,7 @@ class LeAudioClientImpl : public LeAudioClient {
auto* evt = static_cast<cig_remove_cmpl_evt*>(data);
LeAudioDeviceGroup* group = aseGroups_.FindById(evt->cig_id);
groupStateMachine_->ProcessHciNotifOnCigRemove(evt->status, group);
+ remove_group_if_possible(group);
} break;
default:
LOG(ERROR) << __func__ << " Invalid event " << int{event_type};
diff --git a/system/bta/le_audio/le_audio_client_test.cc b/system/bta/le_audio/le_audio_client_test.cc
index 8158ef2995e..ce15362bb54 100644
--- a/system/bta/le_audio/le_audio_client_test.cc
+++ b/system/bta/le_audio/le_audio_client_test.cc
@@ -528,6 +528,10 @@ class UnicastTestNoInit : public Test {
state_machine_callbacks_->StatusReportCb(
group->group_id_, GroupStreamStatus::STREAMING);
streaming_groups[group->group_id_] = group;
+
+ /* Assume CIG is created */
+ group->cig_created_ = true;
+
return true;
});
@@ -554,8 +558,8 @@ class UnicastTestNoInit : public Test {
});
ON_CALL(mock_state_machine_, ProcessHciNotifAclDisconnected(_, _))
- .WillByDefault([](LeAudioDeviceGroup* group,
- LeAudioDevice* leAudioDevice) {
+ .WillByDefault([this](LeAudioDeviceGroup* group,
+ LeAudioDevice* leAudioDevice) {
if (!group) return;
auto* stream_conf = &group->stream_conf;
if (stream_conf->valid) {
@@ -584,6 +588,11 @@ class UnicastTestNoInit : public Test {
stream_conf->valid = false;
}
}
+
+ if (group->IsEmpty()) {
+ group->cig_created_ = false;
+ InjectCigRemoved(group->group_id_);
+ }
});
ON_CALL(mock_state_machine_, ProcessHciNotifCisDisconnected(_, _, _))
@@ -1548,6 +1557,16 @@ class UnicastTestNoInit : public Test {
bluetooth::hci::iso_manager::kIsoEventCisDisconnected, &cis_evt);
}
+ void InjectCigRemoved(uint8_t cig_id) {
+ bluetooth::hci::iso_manager::cig_remove_cmpl_evt evt;
+ evt.status = 0;
+ evt.cig_id = cig_id;
+
+ ASSERT_NE(cig_callbacks_, nullptr);
+ cig_callbacks_->OnCisEvent(
+ bluetooth::hci::iso_manager::kIsoEventCigOnRemoveCmpl, &evt);
+ }
+
MockLeAudioClientCallbacks mock_client_callbacks_;
MockLeAudioClientAudioSource mock_audio_source_;
MockLeAudioClientAudioSink mock_audio_sink_;
@@ -2227,6 +2246,69 @@ TEST_F(UnicastTest, RemoveTwoEarbudsCsisGrouped) {
Mock::VerifyAndClearExpectations(&mock_btif_storage_);
}
+TEST_F(UnicastTest, RemoveWhileStreaming) {
+ const RawAddress test_address0 = GetTestAddress(0);
+ int group_id = bluetooth::groups::kGroupUnknown;
+
+ SetSampleDatabaseEarbudsValid(
+ 1, test_address0, codec_spec_conf::kLeAudioLocationStereo,
+ codec_spec_conf::kLeAudioLocationStereo, false /*add_csis*/,
+ true /*add_cas*/, true /*add_pacs*/, true /*add_ascs*/, 1 /*set_size*/,
+ 0 /*rank*/);
+ EXPECT_CALL(mock_client_callbacks_,
+ OnConnectionState(ConnectionState::CONNECTED, test_address0))
+ .Times(1);
+ EXPECT_CALL(mock_client_callbacks_,
+ OnGroupNodeStatus(test_address0, _, GroupNodeStatus::ADDED))
+ .WillOnce(DoAll(SaveArg<1>(&group_id)));
+
+ ConnectLeAudio(test_address0);
+ ASSERT_NE(group_id, bluetooth::groups::kGroupUnknown);
+
+ // Start streaming
+ uint8_t cis_count_out = 1;
+ uint8_t cis_count_in = 0;
+
+ EXPECT_CALL(mock_audio_source_, Start(_, _)).Times(1);
+ LeAudioClient::Get()->GroupSetActive(group_id);
+
+ EXPECT_CALL(mock_state_machine_, StartStream(_, _)).Times(1);
+
+ StartStreaming(AUDIO_USAGE_MEDIA, AUDIO_CONTENT_TYPE_MUSIC, group_id);
+
+ SyncOnMainLoop();
+ Mock::VerifyAndClearExpectations(&mock_client_callbacks_);
+ Mock::VerifyAndClearExpectations(&mock_audio_source_);
+ Mock::VerifyAndClearExpectations(&mock_state_machine_);
+ SyncOnMainLoop();
+
+ // Verify Data transfer on one audio source cis
+ TestAudioDataTransfer(group_id, cis_count_out, cis_count_in, 1920);
+
+ EXPECT_CALL(mock_groups_module_, RemoveDevice(test_address0, group_id))
+ .Times(1);
+
+ LeAudioDeviceGroup* group = nullptr;
+ EXPECT_CALL(mock_state_machine_, ProcessHciNotifAclDisconnected(_, _))
+ .WillOnce(DoAll(SaveArg<0>(&group)));
+ EXPECT_CALL(
+ mock_client_callbacks_,
+ OnGroupNodeStatus(test_address0, group_id, GroupNodeStatus::REMOVED));
+
+ EXPECT_CALL(mock_client_callbacks_,
+ OnConnectionState(ConnectionState::DISCONNECTED, test_address0))
+ .Times(1);
+
+ LeAudioClient::Get()->RemoveDevice(test_address0);
+
+ SyncOnMainLoop();
+ Mock::VerifyAndClearExpectations(&mock_groups_module_);
+ Mock::VerifyAndClearExpectations(&mock_state_machine_);
+ Mock::VerifyAndClearExpectations(&mock_client_callbacks_);
+
+ ASSERT_NE(group, nullptr);
+}
+
TEST_F(UnicastTest, SpeakerStreaming) {
const RawAddress test_address0 = GetTestAddress(0);
int group_id = bluetooth::groups::kGroupUnknown;
--
GitLab