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

Commit b087eae2 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski Committed by Alice Kuo
Browse files

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
parent 8e3dbd16

system/bta/le_audio/client.cc

+21 −9
Original line number Diff line number Diff line
@@ -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};

system/bta/le_audio/le_audio_client_test.cc

+84 −2
Original line number Diff line number Diff line
@@ -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,7 +558,7 @@ class UnicastTestNoInit : public Test { 
        });



    ON_CALL(mock_state_machine_, ProcessHciNotifAclDisconnected(_, _))

        .WillByDefault([](LeAudioDeviceGroup* group,

        .WillByDefault([this](LeAudioDeviceGroup* group,

                              LeAudioDevice* leAudioDevice) {

          if (!group) return;

          auto* stream_conf = &group->stream_conf;
@@ -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;