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

Commit b4c8f305 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

connection_manager: Fix device no being in Allow List 

In following scenario, connection manager could get out of sync
with gd connection manager about device being in allow list.

1. One client is doing backgroun connect
2. Second client is doing direct connect
3. Direct connect failed due to peer being not available
4. Issue happen here and device was not added back to Allow list.

Bug: 294615104
Tag: #feature
Test: atest --host net_test_gatt_conn_multiplexing
Change-Id: I70b32bdbd391f81a2cfd0a529bd83765a2b72ba8
parent a3de0fb8

system/stack/gatt/connection_manager.cc

+16 −2
Original line number Diff line number Diff line
@@ -441,6 +441,7 @@ void on_connection_complete(const RawAddress& address) { 
}



void on_connection_timed_out_from_shim(const RawAddress& address) {

  LOG_INFO("Connection failed %s", ADDRESS_TO_LOGGABLE_CSTR(address));

  on_connection_timed_out(0x00, address);

}
@@ -461,7 +462,7 @@ void wl_direct_connect_timeout_cb(uint8_t app_id, const RawAddress& address) { 


  // TODO: this would free the timer, from within the timer callback, which is

  // bad.

  direct_connect_remove(app_id, address);

  direct_connect_remove(app_id, address, true);

}



/** Add a device to the direct connection list. Returns true if device
@@ -517,7 +518,8 @@ static void schedule_direct_connect_add(uint8_t app_id, 
  direct_connect_add(app_id, address);

}



bool direct_connect_remove(uint8_t app_id, const RawAddress& address) {

bool direct_connect_remove(uint8_t app_id, const RawAddress& address,

                           bool connection_timeout) {

  LOG_DEBUG("app_id=%d, address=%s", static_cast<int>(app_id),

            ADDRESS_TO_LOGGABLE_CSTR(address));

  auto it = bgconn_dev.find(address);
@@ -542,6 +544,18 @@ bool direct_connect_remove(uint8_t app_id, const RawAddress& address) { 
  it->second.doing_direct_conn.erase(app_it);



  if (is_anyone_interested_to_use_accept_list(it)) {

    if (connection_timeout) {

      /* In such case we need to add device back to allow list because,

       * when connection timeout out, the lower layer removes device from

       * the allow list.

       */

      if (!BTM_AcceptlistAdd(address)) {

        LOG_WARN(

            "Failed to re-add device %s to accept list after connection "

            "timeout",

            ADDRESS_TO_LOGGABLE_CSTR(address));

      }

    }

    return true;

  }

system/stack/gatt/connection_manager.h

+2 −1
Original line number Diff line number Diff line
@@ -51,7 +51,8 @@ void on_connection_complete(const RawAddress& address); 
std::set<tAPP_ID> get_apps_connecting_to(const RawAddress& remote_bda);



bool direct_connect_add(tAPP_ID app_id, const RawAddress& address);

bool direct_connect_remove(tAPP_ID app_id, const RawAddress& address);

bool direct_connect_remove(tAPP_ID app_id, const RawAddress& address,

                           bool connection_timeout = false);



void dump(int fd);

system/stack/test/gatt/gatt_sr_test.cc

+2 −1
Original line number Diff line number Diff line
@@ -58,7 +58,8 @@ namespace connection_manager { 
bool background_connect_remove(uint8_t app_id, const RawAddress& address) {

  return false;

}

bool direct_connect_remove(uint8_t app_id, const RawAddress& address) {

bool direct_connect_remove(uint8_t app_id, const RawAddress& address,

                           bool connection_timeout) {

  return false;

}

bool is_background_connection(const RawAddress& address) { return false; }

system/stack/test/gatt/mock_gatt_utils_ref.cc

+2 −1
Original line number Diff line number Diff line
@@ -25,7 +25,8 @@ namespace connection_manager { 
bool background_connect_remove(uint8_t app_id, const RawAddress& address) {

  return false;

}

bool direct_connect_remove(uint8_t app_id, const RawAddress& address) {

bool direct_connect_remove(uint8_t app_id, const RawAddress& address,

                           bool connection_timeout) {

  return false;

}

bool is_background_connection(const RawAddress& address) { return false; }

system/stack/test/gatt_connection_manager_test.cc

+37 −0
Original line number Diff line number Diff line
@@ -354,4 +354,41 @@ TEST_F(BleConnectionManager, test_re_add_background_connect_to_allow_list) { 
  EXPECT_TRUE(background_connect_remove(CLIENT1, address1));

  Mock::VerifyAndClearExpectations(localAcceptlistMock.get());

}



TEST_F(BleConnectionManager,

       test_re_add_to_allow_list_after_timeout_with_multiple_clients) {

  EXPECT_CALL(*AlarmMock::Get(), AlarmNew(_)).Times(1);

  alarm_callback_t alarm_callback = nullptr;

  void* alarm_data = nullptr;



  /* Accept adding to allow list */

  ON_CALL(*localAcceptlistMock, AcceptlistAdd(address1))

      .WillByDefault(Return(true));



  EXPECT_CALL(*localAcceptlistMock, AcceptlistAdd(address1)).Times(1);

  EXPECT_CALL(*localAcceptlistMock, AcceptlistRemove(_)).Times(0);



  EXPECT_TRUE(background_connect_add(CLIENT1, address1));



  Mock::VerifyAndClearExpectations(localAcceptlistMock.get());



  EXPECT_CALL(*AlarmMock::Get(), AlarmSetOnMloop(_, _, _, _))

      .Times(1)

      .WillOnce(DoAll(SaveArg<2>(&alarm_callback), SaveArg<3>(&alarm_data)));

  // Start direct connect attempt...

  EXPECT_TRUE(direct_connect_add(CLIENT2, address1));



  Mock::VerifyAndClearExpectations(localAcceptlistMock.get());



  // simulate timeout seconds passed, alarm executing

  EXPECT_CALL(*localAcceptlistMock, OnConnectionTimedOut(CLIENT2, address1))

      .Times(1);

  EXPECT_CALL(*localAcceptlistMock, AcceptlistRemove(_)).Times(0);

  EXPECT_CALL(*localAcceptlistMock, AcceptlistAdd(address1)).Times(1);

  EXPECT_CALL(*AlarmMock::Get(), AlarmFree(_)).Times(1);

  alarm_callback(alarm_data);



  Mock::VerifyAndClearExpectations(localAcceptlistMock.get());

}



}  // namespace connection_manager