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

Commit c1a4efca authored by Jack He's avatar Jack He
Browse files

HFP: Fix connection colision handling 

* When there is a connection collison, and remote device is trying to
  connect to HFP server channel while we try to connect to remote's
  server channel as a client. If the remote device succssfully connect
  to us, we should drop our connection request to remote device and
  accept remote device's connection request. As result, we will observe
  1. Our connection request fails, this propagate up to Java layer and
  also clean up any states in these layers
  2. Remote connection request succeeds, this propagate up to Java layer
     as well as a new incoming connection
* Added BTA_AG_COLLISION_EVT to formally handle AG collision in state
  machine
* Modified bta_ag_resume_open to re-send BTA_AG_API_OPEN_EVT instead of
  modifying the state machine state directly. State machine states
  should only be modified by state machine handler
* Ignore RFCOMM open event if PORT_CheckConnection fails
* Added StackRfcommTest.TestConnectionCollision to reproduce this
  scenario with 100% certainty
* Add static checks to HFP BTA state machine to make sure action and
  event indices match enum sizes

Bug: 77224743
Test: connect to multiple remote devices with HFP, MAP and PBAP
      StackRfcommTest.TestConnectionCollision
      btestplans/details/158641/3975
Change-Id: Ib3652784b123abe195e7bd30421ef5f4345b1d4d
Merged-In: Ib3652784b123abe195e7bd30421ef5f4345b1d4d
(cherry picked from commit 88cc85a086bf90e1e669fd689276c6e658286af2)
parent 53c559af

system/bta/ag/bta_ag_act.cc

+62 −38
Original line number Diff line number Diff line
@@ -177,11 +177,9 @@ void bta_ag_start_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
  RawAddress pending_bd_addr = {};



  /* store parameters */

  if (!data.IsEmpty()) {

  p_scb->peer_addr = data.api_open.bd_addr;

  p_scb->open_services = data.api_open.services;

  p_scb->cli_sec_mask = data.api_open.sec_mask;

  }



  /* Check if RFCOMM has any incoming connection to avoid collision. */

  if (PORT_IsOpening(pending_bd_addr)) {
@@ -330,6 +328,7 @@ void bta_ag_open_fail(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
 *

 ******************************************************************************/

void bta_ag_rfc_fail(tBTA_AG_SCB* p_scb, UNUSED_ATTR const tBTA_AG_DATA& data) {

  RawAddress peer_addr = p_scb->peer_addr;

  /* reinitialize stuff */

  p_scb->conn_handle = 0;

  p_scb->conn_service = 0;
@@ -346,7 +345,7 @@ void bta_ag_rfc_fail(tBTA_AG_SCB* p_scb, UNUSED_ATTR const tBTA_AG_DATA& data) { 
  bta_ag_start_servers(p_scb, p_scb->reg_services);



  /* call open cback w. failure */

  bta_ag_cback_open(p_scb, RawAddress::kEmpty, BTA_AG_FAIL_RFCOMM);

  bta_ag_cback_open(p_scb, peer_addr, BTA_AG_FAIL_RFCOMM);

}



/*******************************************************************************
@@ -502,12 +501,6 @@ void bta_ag_rfc_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
 *

 ******************************************************************************/

void bta_ag_rfc_acp_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) {

  uint16_t lcid;

  int i;

  tBTA_AG_SCB *ag_scb, *other_scb;

  RawAddress dev_addr = {};

  int status;



  /* set role */

  p_scb->role = BTA_AG_ACP;
@@ -515,43 +508,45 @@ void bta_ag_rfc_acp_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
                   p_scb->serv_handle[0], p_scb->serv_handle[1]);



  /* get bd addr of peer */

  if (PORT_SUCCESS !=

      (status = PORT_CheckConnection(data.rfc.port_handle, dev_addr, &lcid))) {

    APPL_TRACE_DEBUG(

        "bta_ag_rfc_acp_open error PORT_CheckConnection returned status %d",

        status);

  uint16_t lcid = 0;

  RawAddress dev_addr = RawAddress::kEmpty;

  int status = PORT_CheckConnection(data.rfc.port_handle, dev_addr, &lcid);

  if (status != PORT_SUCCESS) {

    LOG(ERROR) << __func__ << ", PORT_CheckConnection returned " << status;

    return;

  }



  /* Collision Handling */

  for (i = 0, ag_scb = &bta_ag_cb.scb[0]; i < BTA_AG_MAX_NUM_CLIENTS;

       i++, ag_scb++) {

    if (ag_scb->in_use && alarm_is_scheduled(ag_scb->collision_timer)) {

      alarm_cancel(ag_scb->collision_timer);



      if (dev_addr == ag_scb->peer_addr) {

        /* If incoming and outgoing device are same, nothing more to do. */

        /* Outgoing conn will be aborted because we have successful incoming

         * conn.  */

      } else {

        /* Resume outgoing connection. */

        other_scb = bta_ag_get_other_idle_scb(p_scb);

        if (other_scb) {

          other_scb->peer_addr = ag_scb->peer_addr;

          other_scb->open_services = ag_scb->open_services;

          other_scb->cli_sec_mask = ag_scb->cli_sec_mask;



          bta_ag_resume_open(other_scb);

  for (tBTA_AG_SCB& ag_scb : bta_ag_cb.scb) {

    // Cancel any pending collision timers

    if (ag_scb.in_use && alarm_is_scheduled(ag_scb.collision_timer)) {

      alarm_cancel(ag_scb.collision_timer);

      if (dev_addr != ag_scb.peer_addr && p_scb != &ag_scb) {

        // Resume outgoing connection if incoming is not on the same device

        bta_ag_resume_open(&ag_scb);

      }

      }



      break;

    }

    if (dev_addr == ag_scb.peer_addr && p_scb != &ag_scb) {

      // Fail the outgoing connection to clean up any upper layer states

      bta_ag_rfc_fail(&ag_scb, tBTA_AG_DATA::kEmpty);

      // If client port is opened, close it

      if (ag_scb.conn_handle > 0) {

        status = RFCOMM_RemoveConnection(ag_scb.conn_handle);

        if (status != PORT_SUCCESS) {

          LOG(WARNING) << __func__ << ": RFCOMM_RemoveConnection failed for "

                       << dev_addr << ", handle "

                       << std::to_string(ag_scb.conn_handle) << ", error "

                       << status;

        }

      }

    }

  }



  p_scb->peer_addr = dev_addr;



  /* determine connected service from port handle */

  for (i = 0; i < BTA_AG_NUM_IDX; i++) {

  for (int i = 0; i < BTA_AG_NUM_IDX; i++) {

    APPL_TRACE_DEBUG(

        "bta_ag_rfc_acp_open: i = %d serv_handle = %d port_handle = %d", i,

        p_scb->serv_handle[i], data.rfc.port_handle);
@@ -813,3 +808,32 @@ void bta_ag_setcodec(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 


  (*bta_ag_cb.p_cback)(BTA_AG_WBS_EVT, (tBTA_AG*)&val);

}



static void bta_ag_collision_timer_cback(void* data) {

  if (data == nullptr) {

    LOG(ERROR) << __func__ << ": data should never be null in a timer callback";

    return;

  }

  /* If the peer haven't opened AG connection     */

  /* we will restart opening process.             */

  bta_ag_resume_open(static_cast<tBTA_AG_SCB*>(data));

}



void bta_ag_handle_collision(tBTA_AG_SCB* p_scb,

                             UNUSED_ATTR const tBTA_AG_DATA& data) {

  /* Cancel SDP if it had been started. */

  if (p_scb->p_disc_db) {

    SDP_CancelServiceSearch(p_scb->p_disc_db);

    bta_ag_free_db(p_scb, tBTA_AG_DATA::kEmpty);

  }



  /* reopen registered servers */

  /* Collision may be detected before or after we close servers. */

  if (bta_ag_is_server_closed(p_scb)) {

    bta_ag_start_servers(p_scb, p_scb->reg_services);

  }



  /* Start timer to han */

  alarm_set_on_mloop(p_scb->collision_timer, BTA_AG_COLLISION_TIMEOUT_MS,

                     bta_ag_collision_timer_cback, p_scb);

}

system/bta/ag/bta_ag_int.h

+3 −1
Original line number Diff line number Diff line
@@ -79,6 +79,7 @@ enum { 
  BTA_AG_DISC_FAIL_EVT,

  BTA_AG_RING_TIMEOUT_EVT,

  BTA_AG_SVC_TIMEOUT_EVT,

  BTA_AG_COLLISION_EVT,

  BTA_AG_MAX_EVT,

};
@@ -306,7 +307,6 @@ extern uint8_t bta_ag_service_to_idx(tBTA_SERVICE_MASK services); 
extern uint16_t bta_ag_idx_by_bdaddr(const RawAddress* peer_addr);

extern bool bta_ag_other_scb_open(tBTA_AG_SCB* p_curr_scb);

extern bool bta_ag_scb_open(tBTA_AG_SCB* p_curr_scb);

extern tBTA_AG_SCB* bta_ag_get_other_idle_scb(tBTA_AG_SCB* p_curr_scb);

extern void bta_ag_sm_execute(tBTA_AG_SCB* p_scb, uint16_t event,

                              const tBTA_AG_DATA& data);

extern void bta_ag_sm_execute_by_handle(uint16_t handle, uint16_t event,
@@ -378,6 +378,8 @@ extern void bta_ag_svc_conn_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data); 
extern void bta_ag_result(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data);

extern void bta_ag_setcodec(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data);

extern void bta_ag_send_ring(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data);

extern void bta_ag_handle_collision(tBTA_AG_SCB* p_scb,

                                    const tBTA_AG_DATA& data);



/* Internal utility functions */

extern void bta_ag_sco_codec_nego(tBTA_AG_SCB* p_scb, bool result);

system/bta/ag/bta_ag_main.cc

+37 −82
Original line number Diff line number Diff line
@@ -68,6 +68,7 @@ enum { 
  BTA_AG_RESULT,

  BTA_AG_SETCODEC,

  BTA_AG_SEND_RING,

  BTA_AG_HANDLE_COLLISION,

  BTA_AG_NUM_ACTIONS

};
@@ -133,6 +134,7 @@ static const char* bta_ag_evt_str(uint16_t event) { 
    CASE_RETURN_STR(BTA_AG_DISC_FAIL_EVT)

    CASE_RETURN_STR(BTA_AG_RING_TIMEOUT_EVT)

    CASE_RETURN_STR(BTA_AG_SVC_TIMEOUT_EVT)

    CASE_RETURN_STR(BTA_AG_COLLISION_EVT)

    default:

      return "Unknown AG Event";

  }
@@ -160,7 +162,11 @@ const tBTA_AG_ACTION bta_ag_action[] = { 
    bta_ag_sco_conn_close, bta_ag_sco_listen,    bta_ag_sco_open,

    bta_ag_sco_close,      bta_ag_sco_shutdown,  bta_ag_post_sco_open,

    bta_ag_post_sco_close, bta_ag_svc_conn_open, bta_ag_result,

    bta_ag_setcodec,       bta_ag_send_ring};

    bta_ag_setcodec,       bta_ag_send_ring,     bta_ag_handle_collision};



static_assert(sizeof(bta_ag_action) / sizeof(tBTA_AG_ACTION) ==

                  BTA_AG_NUM_ACTIONS,

              "bta_ag_action must handle all actions");



/* state table information */

#define BTA_AG_ACTIONS 2    /* number of actions */
@@ -189,7 +195,8 @@ const uint8_t bta_ag_st_init[][BTA_AG_NUM_COLS] = { 
    /* DISC_OK_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_INIT_ST},

    /* DISC_FAIL_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_INIT_ST},

    /* RING_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_INIT_ST},

    /* SVC_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_INIT_ST}};

    /* SVC_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_INIT_ST},

    /* COLLISION_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_INIT_ST}};



/* state table for opening state */

const uint8_t bta_ag_st_opening[][BTA_AG_NUM_COLS] = {
@@ -216,7 +223,9 @@ const uint8_t bta_ag_st_opening[][BTA_AG_NUM_COLS] = { 
    /* DISC_OK_EVT */ {BTA_AG_RFC_DO_OPEN, BTA_AG_IGNORE, BTA_AG_OPENING_ST},

    /* DISC_FAIL_EVT */ {BTA_AG_DISC_FAIL, BTA_AG_IGNORE, BTA_AG_INIT_ST},

    /* RING_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_OPENING_ST},

    /* SVC_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_OPENING_ST}};

    /* SVC_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_OPENING_ST},

    /* COLLISION_EVT */

    {BTA_AG_HANDLE_COLLISION, BTA_AG_IGNORE, BTA_AG_INIT_ST}};



/* state table for open state */

const uint8_t bta_ag_st_open[][BTA_AG_NUM_COLS] = {
@@ -243,7 +252,8 @@ const uint8_t bta_ag_st_open[][BTA_AG_NUM_COLS] = { 
    /* DISC_OK_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_OPEN_ST},

    /* DISC_FAIL_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_OPEN_ST},

    /* RING_TOUT_EVT */ {BTA_AG_SEND_RING, BTA_AG_IGNORE, BTA_AG_OPEN_ST},

    /* SVC_TOUT_EVT */ {BTA_AG_START_CLOSE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST}};

    /* SVC_TOUT_EVT */ {BTA_AG_START_CLOSE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST},

    /* COLLISION_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_OPEN_ST}};



/* state table for closing state */

const uint8_t bta_ag_st_closing[][BTA_AG_NUM_COLS] = {
@@ -269,7 +279,19 @@ const uint8_t bta_ag_st_closing[][BTA_AG_NUM_COLS] = { 
    /* DISC_OK_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST},

    /* DISC_FAIL_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST},

    /* RING_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST},

    /* SVC_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST}};

    /* SVC_TOUT_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST},

    /* COLLISION_EVT */ {BTA_AG_IGNORE, BTA_AG_IGNORE, BTA_AG_CLOSING_ST}};



constexpr size_t BTA_AG_NUM_EVENTS =

    BTA_AG_MAX_EVT - BTA_SYS_EVT_START(BTA_ID_AG);

static_assert(sizeof(bta_ag_st_init) / BTA_AG_NUM_COLS == BTA_AG_NUM_EVENTS,

              "bta_ag_st_init must handle all AG events");

static_assert(sizeof(bta_ag_st_opening) / BTA_AG_NUM_COLS == BTA_AG_NUM_EVENTS,

              "bta_ag_st_opening must handle all AG events");

static_assert(sizeof(bta_ag_st_open) / BTA_AG_NUM_COLS == BTA_AG_NUM_EVENTS,

              "bta_ag_st_open must handle all AG events");

static_assert(sizeof(bta_ag_st_closing) / BTA_AG_NUM_COLS == BTA_AG_NUM_EVENTS,

              "bta_ag_st_closing must handle all AG events");



/* type for state table */

typedef const uint8_t (*tBTA_AG_ST_TBL)[BTA_AG_NUM_COLS];
@@ -494,52 +516,6 @@ bool bta_ag_scb_open(tBTA_AG_SCB* p_curr_scb) { 
         p_curr_scb->state == BTA_AG_OPEN_ST;

}



/*******************************************************************************

 *

 * Function         bta_ag_get_other_idle_scb

 *

 * Description      Return other scb if it is in INIT st.

 *

 *

 * Returns          Pointer to other scb if INIT st, NULL otherwise.

 *

 ******************************************************************************/

tBTA_AG_SCB* bta_ag_get_other_idle_scb(tBTA_AG_SCB* p_curr_scb) {

  tBTA_AG_SCB* p_scb = &bta_ag_cb.scb[0];

  uint8_t xx;



  for (xx = 0; xx < BTA_AG_MAX_NUM_CLIENTS; xx++, p_scb++) {

    if (p_scb->in_use && (p_scb != p_curr_scb) &&

        (p_scb->state == BTA_AG_INIT_ST)) {

      return p_scb;

    }

  }



  /* no other scb found */

  APPL_TRACE_DEBUG("bta_ag_get_other_idle_scb: No idle AG scb");

  return nullptr;

}



/*******************************************************************************

 *

 * Function         bta_ag_collision_timer_cback

 *

 * Description      AG connection collision timer callback

 *

 *

 * Returns          void

 *

 ******************************************************************************/

static void bta_ag_collision_timer_cback(void* data) {

  tBTA_AG_SCB* p_scb = (tBTA_AG_SCB*)data;



  APPL_TRACE_DEBUG("%s", __func__);



  /* If the peer haven't opened AG connection     */

  /* we will restart opening process.             */

  bta_ag_resume_open(p_scb);

}



/*******************************************************************************

 *

 * Function         bta_ag_collision_cback
@@ -568,24 +544,7 @@ void bta_ag_collision_cback(UNUSED_ATTR tBTA_SYS_CONN_STATUS status, uint8_t id, 
      LOG(WARNING) << __func__ << "AG found collision (UNKNOWN) for handle "

                   << unsigned(handle) << " device " << peer_addr;

    }



    p_scb->state = BTA_AG_INIT_ST;



    /* Cancel SDP if it had been started. */

    if (p_scb->p_disc_db) {

      SDP_CancelServiceSearch(p_scb->p_disc_db);

      bta_ag_free_db(p_scb, tBTA_AG_DATA::kEmpty);

    }



    /* reopen registered servers */

    /* Collision may be detected before or after we close servers. */

    if (bta_ag_is_server_closed(p_scb)) {

      bta_ag_start_servers(p_scb, p_scb->reg_services);

    }



    /* Start timer to han */

    alarm_set_on_mloop(p_scb->collision_timer, BTA_AG_COLLISION_TIMEOUT_MS,

                       bta_ag_collision_timer_cback, p_scb);

    bta_ag_sm_execute(p_scb, BTA_AG_COLLISION_EVT, tBTA_AG_DATA::kEmpty);

  }

}
@@ -600,20 +559,16 @@ void bta_ag_collision_cback(UNUSED_ATTR tBTA_SYS_CONN_STATUS status, uint8_t id, 
 *

 ******************************************************************************/

void bta_ag_resume_open(tBTA_AG_SCB* p_scb) {

  if (p_scb) {

    APPL_TRACE_DEBUG("%s: handle=%d, bd_addr=%s", __func__,

                     bta_ag_scb_to_idx(p_scb),

                     p_scb->peer_addr.ToString().c_str());

    /* resume opening process.  */

  if (p_scb->state == BTA_AG_INIT_ST) {

      LOG(WARNING) << __func__

                   << ": handle=" << unsigned(bta_ag_scb_to_idx(p_scb))

                   << ", bd_addr=" << p_scb->peer_addr;

      p_scb->state = BTA_AG_OPENING_ST;

      bta_ag_start_open(p_scb, tBTA_AG_DATA::kEmpty);

    }

    LOG(INFO) << __func__ << ": Resume connection to " << p_scb->peer_addr

              << ", handle" << bta_ag_scb_to_idx(p_scb);

    tBTA_AG_DATA open_data = {.api_open.bd_addr = p_scb->peer_addr,

                              .api_open.services = p_scb->open_services,

                              .api_open.sec_mask = p_scb->cli_sec_mask};

    bta_ag_sm_execute(p_scb, BTA_AG_API_OPEN_EVT, open_data);

  } else {

    LOG(ERROR) << __func__ << ": null p_scb";

    VLOG(1) << __func__ << ": device " << p_scb->peer_addr

            << " is already in state " << std::to_string(p_scb->state);

  }

}

system/btif/src/btif_hf.cc

+6 −5
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ 
#include <cstring>

#include <ctime>



#include <bta/include/bta_ag_api.h>

#include <hardware/bluetooth.h>

#include <hardware/bluetooth_headset_callbacks.h>

#include <hardware/bluetooth_headset_interface.h>
@@ -327,11 +328,11 @@ static void btif_hf_upstreams_evt(uint16_t event, char* p_param) { 
                   << unsigned(p_data->open.status);

        btif_hf_cb[idx].state = BTHF_CONNECTION_STATE_DISCONNECTED;

      } else {

        BTIF_TRACE_WARNING(

            "%s: AG open failed, but another device connected. status=%d "

            "state=%d connected device=%s",

            __func__, p_data->open.status, btif_hf_cb[idx].state,

            btif_hf_cb[idx].connected_bda.ToString().c_str());

        LOG(WARNING) << __func__ << ": AG open failed for "

                     << p_data->open.bd_addr << ", error "

                     << std::to_string(p_data->open.status)

                     << ", local device is " << btif_hf_cb[idx].connected_bda

                     << ". Ignoring as not expecting to open";

        break;

      }

      bt_hf_callbacks->ConnectionStateCallback(btif_hf_cb[idx].state,

system/stack/rfcomm/port_utils.cc

+3 −4
Original line number Diff line number Diff line
@@ -319,8 +319,7 @@ tPORT* port_find_mcb_dlci_port(tRFC_MCB* p_mcb, uint8_t dlci) { 


  uint8_t inx = p_mcb->port_inx[dlci];

  if (inx == 0) {

    LOG(WARNING) << __func__

                 << ": Cannot find allocated RFCOMM app port for DLCI "

    LOG(INFO) << __func__ << ": Cannot find allocated RFCOMM app port for DLCI "

              << std::to_string(dlci) << " on " << p_mcb->bd_addr

              << ", p_mcb=" << p_mcb;

    return nullptr;