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

Commit cf9fa088 authored by Pavlin Radoslavov's avatar Pavlin Radoslavov
Browse files

Don't mix internal AVDTP state when connecting to two devices

There is a race condition when connecting to two devices, and
the connection to one of the devices is not powered on.

Because of the race condition, the wrong tBTA_AV_LCB entry inside
bta_av_sig_chg() might be allocated for the connection to
the device that is powered on. As a result of that, when
the connection attempt to the powered off device times out,
we will use the wrong bta_handle to close the existing AVDTP connection to
the device that is connected.

Also:
 - Add a new function BTA_AvObtainPeerChannelIndex() to select the
   appropriate AvdtpCcb entry if there an outgoing connection request
   while an incoming connection is received.
 - Added/updated log messages to help debugging similar issues in the future
 - Removed unnecessary SetActivePeer() call inside the BtifAv state
   machine for A2DP Source to avoid additional internal/transient glitches

Test: Manual
Bug: 79697137
Change-Id: Ie55e3dcf6c6a15637ce3631f2828548a2423d881
parent 3b00cc75
Loading
Loading
Loading
Loading
+6 −4
Original line number Diff line number Diff line
@@ -266,7 +266,8 @@ static uint8_t bta_av_get_scb_sep_type(tBTA_AV_SCB* p_scb,
 *
 ******************************************************************************/
static void bta_av_save_addr(tBTA_AV_SCB* p_scb, const RawAddress& bd_addr) {
  APPL_TRACE_DEBUG("%s: r:%d, s:%d", __func__, p_scb->recfg_sup,
  APPL_TRACE_DEBUG("%s: peer=%s recfg_sup:%d, suspend_sup:%d", __func__,
                   bd_addr.ToString().c_str(), p_scb->recfg_sup,
                   p_scb->suspend_sup);
  if (p_scb->PeerAddress() != bd_addr) {
    LOG_INFO(LOG_TAG, "%s: reset flags old_addr=%s new_addr=%s", __func__,
@@ -423,8 +424,8 @@ void bta_av_proc_stream_evt(uint8_t handle, const RawAddress& bd_addr,

    p_msg->bd_addr = bd_addr;
    p_msg->scb_index = scb_index;
    APPL_TRACE_EVENT(LOG_TAG, "%s: stream event bd_addr: %s scb_index: %u",
                     __func__, p_msg->bd_addr.ToString().c_str(), scb_index);
    APPL_TRACE_EVENT("%s: stream event bd_addr: %s scb_index: %u", __func__,
                     p_msg->bd_addr.ToString().c_str(), scb_index);

    if (p_data != NULL) {
      memcpy(&p_msg->msg, p_data, sizeof(tAVDT_CTRL));
@@ -769,7 +770,8 @@ void bta_av_do_disc_a2dp(tBTA_AV_SCB* p_scb, tBTA_AV_DATA* p_data) {
                          ATTR_ID_BT_PROFILE_DESC_LIST};
  uint16_t sdp_uuid = 0; /* UUID for which SDP has to be done */

  APPL_TRACE_DEBUG("%s: use_rc: %d switch_res:%d, oc:%d", __func__,
  APPL_TRACE_DEBUG("%s: peer_addr: %s use_rc: %d switch_res:%d, oc:%d",
                   __func__, p_data->api_open.bd_addr.ToString().c_str(),
                   p_data->api_open.use_rc, p_data->api_open.switch_res,
                   bta_av_cb.audio_open_cnt);

+109 −53
Original line number Diff line number Diff line
@@ -1323,6 +1323,56 @@ void bta_av_api_disconnect(tBTA_AV_DATA* p_data) {
  alarm_cancel(bta_av_cb.link_signalling_timer);
}

/**
 * Find the index for the free LCB entry to use.
 *
 * The selection order is:
 * (1) Find the index if there is already SCB entry for the peer address
 * (2) If there is no SCB entry for the peer address, find the first
 * SCB entry that is not assigned.
 *
 * @param peer_address the peer address to use
 * @return the index for the free LCB entry to use or BTA_AV_NUM_LINKS
 * if no entry is found
 */
static uint8_t bta_av_find_lcb_index_by_scb_and_address(
    const RawAddress& peer_address) {
  APPL_TRACE_DEBUG("%s: peer_address: %s conn_lcb: 0x%x", __func__,
                   peer_address.ToString().c_str(), bta_av_cb.conn_lcb);

  // Find the index if there is already SCB entry for the peer address
  for (uint8_t index = 0; index < BTA_AV_NUM_LINKS; index++) {
    uint8_t mask = 1 << index;
    if (mask & bta_av_cb.conn_lcb) {
      continue;
    }
    tBTA_AV_SCB* p_scb = bta_av_cb.p_scb[index];
    if (p_scb == nullptr) {
      continue;
    }
    if (p_scb->PeerAddress() == peer_address) {
      return index;
    }
  }

  // Find the first SCB entry that is not assigned.
  for (uint8_t index = 0; index < BTA_AV_NUM_LINKS; index++) {
    uint8_t mask = 1 << index;
    if (mask & bta_av_cb.conn_lcb) {
      continue;
    }
    tBTA_AV_SCB* p_scb = bta_av_cb.p_scb[index];
    if (p_scb == nullptr) {
      continue;
    }
    if (!p_scb->IsAssigned()) {
      return index;
    }
  }

  return BTA_AV_NUM_LINKS;
}

/*******************************************************************************
 *
 * Function         bta_av_sig_chg
@@ -1341,14 +1391,29 @@ void bta_av_sig_chg(tBTA_AV_DATA* p_data) {

  APPL_TRACE_DEBUG("%s: event: %d", __func__, event);
  if (event == AVDT_CONNECT_IND_EVT) {
    APPL_TRACE_DEBUG("%s: AVDT_CONNECT_IND_EVT: peer %s", __func__,
                     p_data->str_msg.bd_addr.ToString().c_str());

    p_lcb = bta_av_find_lcb(p_data->str_msg.bd_addr, BTA_AV_LCB_FIND);
    if (!p_lcb) {
      /* if the address does not have an LCB yet, alloc one */
      for (xx = 0; xx < BTA_AV_NUM_LINKS; xx++) {
      xx = bta_av_find_lcb_index_by_scb_and_address(p_data->str_msg.bd_addr);

      /* check if we found something */
      if (xx >= BTA_AV_NUM_LINKS) {
        /* We do not have scb for this avdt connection.     */
        /* Silently close the connection.                   */
        APPL_TRACE_ERROR("%s: av scb not available for avdt connection for %s",
                         __func__, p_data->str_msg.bd_addr.ToString().c_str());
        AVDT_DisconnectReq(p_data->str_msg.bd_addr, NULL);
        return;
      }
      LOG_INFO(LOG_TAG,
               "%s: AVDT_CONNECT_IND_EVT: peer %s selected lcb_index %d",
               __func__, p_data->str_msg.bd_addr.ToString().c_str(), xx);

      tBTA_AV_SCB* p_scb = p_cb->p_scb[xx];
      mask = 1 << xx;
        APPL_TRACE_DEBUG("%s: conn_lcb: 0x%x", __func__, p_cb->conn_lcb);
        /* look for a p_lcb with its p_scb registered */
        if ((!(mask & p_cb->conn_lcb)) && (p_cb->p_scb[xx] != NULL)) {
      p_lcb = &p_cb->lcb[xx];
      p_lcb->lidx = xx + 1;
      p_lcb->addr = p_data->str_msg.bd_addr;
@@ -1359,15 +1424,13 @@ void bta_av_sig_chg(tBTA_AV_DATA* p_data) {
      }
      /* this entry is not used yet. */
      p_cb->conn_lcb |= mask; /* mark it as used */
          APPL_TRACE_DEBUG("%s: start sig timer %d", __func__,
                           p_data->hdr.offset);
      APPL_TRACE_DEBUG("%s: start sig timer %d", __func__, p_data->hdr.offset);
      if (p_data->hdr.offset == AVDT_ACP) {
            APPL_TRACE_DEBUG(
                "%s: Incoming L2CAP acquired, set state as incoming", __func__);
            p_cb->p_scb[xx]->OnConnected(p_data->str_msg.bd_addr);
            p_cb->p_scb[xx]->use_rc =
                true; /* allowing RC for incoming connection */
            bta_av_ssm_execute(p_cb->p_scb[xx], BTA_AV_ACP_CONNECT_EVT, p_data);
        APPL_TRACE_DEBUG("%s: Incoming L2CAP acquired, set state as incoming",
                         __func__);
        p_scb->OnConnected(p_data->str_msg.bd_addr);
        p_scb->use_rc = true; /* allowing RC for incoming connection */
        bta_av_ssm_execute(p_scb, BTA_AV_ACP_CONNECT_EVT, p_data);

        /* The Pending Event should be sent as soon as the L2CAP signalling
         * channel
@@ -1379,28 +1442,13 @@ void bta_av_sig_chg(tBTA_AV_DATA* p_data) {
        bta_av_signalling_timer(NULL);

        APPL_TRACE_DEBUG("%s: Re-start timer for AVDTP service", __func__);
            bta_sys_conn_open(BTA_ID_AV, p_cb->p_scb[xx]->app_id,
                              p_cb->p_scb[xx]->PeerAddress());
        bta_sys_conn_open(BTA_ID_AV, p_scb->app_id, p_scb->PeerAddress());
        /* Possible collision : need to avoid outgoing processing while the
         * timer is running */
            p_cb->p_scb[xx]->coll_mask = BTA_AV_COLL_INC_TMR;
            alarm_set_on_mloop(p_cb->accept_signalling_timer,
                               BTA_AV_ACCEPT_SIGNALLING_TIMEOUT_MS,
                               bta_av_accept_signalling_timer_cback,
                               UINT_TO_PTR(xx));
          }
          break;
        }
      }

      /* check if we found something */
      if (xx == BTA_AV_NUM_LINKS) {
        /* We do not have scb for this avdt connection.     */
        /* Silently close the connection.                   */
        APPL_TRACE_ERROR("%s: av scb not available for avdt connection",
                         __func__);
        AVDT_DisconnectReq(p_data->str_msg.bd_addr, NULL);
        return;
        p_scb->coll_mask = BTA_AV_COLL_INC_TMR;
        alarm_set_on_mloop(
            p_cb->accept_signalling_timer, BTA_AV_ACCEPT_SIGNALLING_TIMEOUT_MS,
            bta_av_accept_signalling_timer_cback, UINT_TO_PTR(xx));
      }
    }
  }
@@ -1458,12 +1506,16 @@ void bta_av_signalling_timer(UNUSED_ATTR tBTA_AV_DATA* p_data) {
  uint8_t mask;
  tBTA_AV_LCB* p_lcb = NULL;

  APPL_TRACE_DEBUG("%s", __func__);
  APPL_TRACE_DEBUG("%s: conn_lcb=0x%x", __func__, p_cb->conn_lcb);
  for (xx = 0; xx < BTA_AV_NUM_LINKS; xx++) {
    p_lcb = &p_cb->lcb[xx];
    mask = 1 << xx;
    APPL_TRACE_DEBUG(
        "%s: index=%d conn_lcb=0x%x peer=%s conn_mask=0x%x lidx=%d", __func__,
        xx, p_cb->conn_lcb, p_lcb->addr.ToString().c_str(), p_lcb->conn_msk,
        p_lcb->lidx);
    if (mask & p_cb->conn_lcb) {
      /* this entry is used. check if it is connected */
      p_lcb = &p_cb->lcb[xx];
      if (!p_lcb->conn_msk) {
        bta_sys_start_timer(p_cb->link_signalling_timer,
                            BTA_AV_SIGNALLING_TIMEOUT_MS,
@@ -1472,6 +1524,10 @@ void bta_av_signalling_timer(UNUSED_ATTR tBTA_AV_DATA* p_data) {
        pend.bd_addr = p_lcb->addr;
        tBTA_AV bta_av_data;
        bta_av_data.pend = pend;
        APPL_TRACE_DEBUG(
            "%s: BTA_AV_PENDING_EVT for %s index=%d conn_mask=0x%x lidx=%d",
            __func__, pend.bd_addr.ToString().c_str(), xx, p_lcb->conn_msk,
            p_lcb->lidx);
        (*p_cb->p_cback)(BTA_AV_PENDING_EVT, &bta_av_data);
      }
    }
+7 −0
Original line number Diff line number Diff line
@@ -545,6 +545,13 @@ struct tBTA_AV_SCB final {
   */
  void SetAvdtpVersion(uint16_t avdtp_version);

  /**
   * Check whether the entry is assigned and currenty used.
   *
   * @return true if the entry is assigned and currently used
   */
  bool IsAssigned() const { return !peer_address_.IsEmpty(); }

 private:
  RawAddress peer_address_;  // Peer address
  uint16_t avdtp_version_;   // The AVDTP version of the peer device
+22 −1
Original line number Diff line number Diff line
@@ -277,6 +277,27 @@ static tBTA_AV_SCB* bta_av_addr_to_scb(const RawAddress& bd_addr) {
  return p_scb;
}

int BTA_AvObtainPeerChannelIndex(const RawAddress& peer_address) {
  // Find the entry for the peer (if exists)
  tBTA_AV_SCB* p_scb = bta_av_addr_to_scb(peer_address);
  if (p_scb != nullptr) {
    return p_scb->hdi;
  }

  // Find the index for an entry that is not used
  for (int index = 0; index < BTA_AV_NUM_STRS; index++) {
    tBTA_AV_SCB* p_scb = bta_av_cb.p_scb[index];
    if (p_scb == nullptr) {
      continue;
    }
    if (p_scb->PeerAddress().IsEmpty()) {
      return p_scb->hdi;
    }
  }

  return -1;
}

/*******************************************************************************
 *
 * Function         bta_av_hndl_to_scb
@@ -1422,7 +1443,7 @@ void bta_debug_av_dump(int fd) {
    dprintf(fd, "    Congested: %s\n", p_scb->cong ? "true" : "false");
    dprintf(fd, "    Open status: %d\n", p_scb->open_status);
    dprintf(fd, "    Channel: %d\n", p_scb->chnl);
    dprintf(fd, "    BTA handle: %d\n", p_scb->hndl);
    dprintf(fd, "    BTA handle: 0x%x\n", p_scb->hndl);
    dprintf(fd, "    Protocol service capabilities mask: 0x%x\n",
            p_scb->cur_psc_mask);
    dprintf(fd, "    AVDTP handle: %d\n", p_scb->avdt_handle);
+11 −0
Original line number Diff line number Diff line
@@ -703,6 +703,17 @@ void BTA_AvOffloadStart(tBTA_AV_HNDL hndl);
 ******************************************************************************/
void BTA_AvOffloadStartRsp(tBTA_AV_HNDL hndl, tBTA_AV_STATUS status);

/**
 * Obtain the Channel Index for a peer.
 * If the peer already has associated internal state, the corresponding
 * Channel Index for that state is returned. Otherwise, the Channel Index
 * for unused internal state is returned instead.
 *
 * @param peer_address the peer address
 * @return the peer Channel Index index if obtained, otherwise -1
 */
int BTA_AvObtainPeerChannelIndex(const RawAddress& peer_address);

/**
 * Dump debug-related information for the BTA AV module.
 *
Loading