From f92966049c67cae0edcea4e8d5648579512d79c1 Mon Sep 17 00:00:00 2001 From: Cheney Ni Date: Fri, 27 Mar 2020 11:54:28 +0800 Subject: [PATCH] A2DP: Be more careful in allocating a BtifAvPeer instance MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit While a new peer is coming either by in-coming or out-going connection, the stack first tries to find or create an associated BtifAvPeer object for it, but uses an unknown BTA handle if BTA_AV is still registering. This handle will be updated after BTA_AV registered, but causes a mismatch in the handle usage between BTIF and BTA layers since the register process just finished. This change disallows to use an unknown BTA handle, so it prevents such abnormal cases. Besides, it adds more log messages about BTA_AV registering, and also makes sure the disabling flag won't be activated after disabled. Bug: 135655859 Bug: 152597903 Test: Switch BT state quickly between STATE_BLE_ON and STATE_ON Change-Id: I9df3d64f301dffbecdeaf3de18dd455be1c63ce2 Merged-In: I9df3d64f301dffbecdeaf3de18dd455be1c63ce2 (cherry picked from commit 714e639cb7291ecfb9feb12acad9bc7c8f455ba0) --- bta/ar/bta_ar.cc | 3 +++ bta/av/bta_av_act.cc | 14 +++++++++++--- bta/av/bta_av_main.cc | 5 ++++- btif/src/btif_av.cc | 45 +++++++++++++++++++++++++++++++++++++++------ 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/bta/ar/bta_ar.cc b/bta/ar/bta_ar.cc index cdb65db00..8e02aa45b 100644 --- a/bta/ar/bta_ar.cc +++ b/bta/ar/bta_ar.cc @@ -110,6 +110,9 @@ void bta_ar_reg_avdt(AvdtpRcb* p_reg, tAVDT_CTRL_CBACK* p_cback, if (mask) { if (bta_ar_cb.avdt_registered == 0) { AVDT_Register(p_reg, bta_ar_avdt_cback); + } else { + APPL_TRACE_WARNING("%s: sys_id:%d doesn't register again (registered:%d)", + __func__, sys_id, bta_ar_cb.avdt_registered); } bta_ar_cb.avdt_registered |= mask; } diff --git a/bta/av/bta_av_act.cc b/bta/av/bta_av_act.cc index 9449fb679..827cdbb5b 100644 --- a/bta/av/bta_av_act.cc +++ b/bta/av/bta_av_act.cc @@ -1304,6 +1304,7 @@ void bta_av_conn_chg(tBTA_AV_DATA* p_data) { ******************************************************************************/ void bta_av_disable(tBTA_AV_CB* p_cb, UNUSED_ATTR tBTA_AV_DATA* p_data) { BT_HDR hdr; + bool disabling_in_progress = false; uint16_t xx; p_cb->disabling = true; @@ -1318,8 +1319,13 @@ void bta_av_disable(tBTA_AV_CB* p_cb, UNUSED_ATTR tBTA_AV_DATA* p_data) { if (p_cb->p_scb[xx] != NULL) { hdr.layer_specific = xx + 1; bta_av_api_deregister((tBTA_AV_DATA*)&hdr); + disabling_in_progress = true; } } + // Since All channels are deregistering by API_DEREGISTER, the DEREG_COMP_EVT + // would come first before API_DISABLE if there is no connections, and it is + // no needed to setup this disabling flag. + p_cb->disabling = disabling_in_progress; alarm_free(p_cb->link_signalling_timer); p_cb->link_signalling_timer = NULL; @@ -2250,9 +2256,9 @@ void bta_av_dereg_comp(tBTA_AV_DATA* p_data) { p_scb->hndl); mask = BTA_AV_HNDL_TO_MSK(p_scb->hdi); p_cb->reg_audio &= ~mask; - if ((p_cb->conn_audio & mask) && bta_av_cb.audio_open_cnt) { + if ((p_cb->conn_audio & mask) && p_cb->audio_open_cnt) { /* this channel is still marked as open. decrease the count */ - bta_av_cb.audio_open_cnt--; + p_cb->audio_open_cnt--; } p_cb->conn_audio &= ~mask; @@ -2303,7 +2309,9 @@ void bta_av_dereg_comp(tBTA_AV_DATA* p_data) { if (p_cb->disabling) { p_cb->disabling = false; - bta_av_cb.features = 0; + // reset enabling parameters + p_cb->features = 0; + p_cb->sec_mask = 0; } /* Clear the Capturing service class bit */ diff --git a/bta/av/bta_av_main.cc b/bta/av/bta_av_main.cc index 7d31fcfe8..83677479f 100644 --- a/bta/av/bta_av_main.cc +++ b/bta/av/bta_av_main.cc @@ -203,7 +203,7 @@ const tBTA_AV_NSM_ACT bta_av_nsm_act[] = { ****************************************************************************/ /* AV control block */ -tBTA_AV_CB bta_av_cb; +tBTA_AV_CB bta_av_cb = {}; static const char* bta_av_st_code(uint8_t state); @@ -641,6 +641,9 @@ static void bta_av_api_register(tBTA_AV_DATA* p_data) { } if (AVDT_CreateStream(p_scb->app_id, &p_scb->seps[codec_index].av_handle, avdtp_stream_config) != AVDT_SUCCESS) { + APPL_TRACE_WARNING( + "%s: bta_handle=0x%x (app_id %d) failed to alloc an SEP index:%d", + __func__, p_scb->hndl, p_scb->app_id, codec_index); continue; } /* Save a copy of the codec */ diff --git a/btif/src/btif_av.cc b/btif/src/btif_av.cc index 97366dd5a..ae5333332 100644 --- a/btif/src/btif_av.cc +++ b/btif/src/btif_av.cc @@ -1046,12 +1046,18 @@ BtifAvPeer* BtifAvSource::FindOrCreatePeer(const RawAddress& peer_address, __PRETTY_FUNCTION__, peer_address.ToString().c_str()); return nullptr; } + // Get the BTA Handle (if known) if (bta_handle == kBtaHandleUnknown) { auto it = peer_id2bta_handle_.find(peer_id); - if (it != peer_id2bta_handle_.end()) { - bta_handle = it->second; + if (it == peer_id2bta_handle_.end() || it->second == kBtaHandleUnknown) { + BTIF_TRACE_ERROR( + "%s: Cannot create peer for peer_address=%s : " + "cannot convert Peer ID=%d to unique BTA Handle", + __PRETTY_FUNCTION__, peer_address.ToString().c_str(), peer_id); + return nullptr; } + bta_handle = it->second; } LOG_INFO(LOG_TAG, @@ -1140,7 +1146,18 @@ void BtifAvSource::BtaHandleRegistered(uint8_t peer_id, // Set the BTA Handle for the Peer (if exists) BtifAvPeer* peer = FindPeerByPeerId(peer_id); - if (peer != nullptr) { + if (peer != nullptr && peer->BtaHandle() != bta_handle) { + if (peer->BtaHandle() == kBtaHandleUnknown) { + BTIF_TRACE_EVENT( + "%s: Assign peer: peer_address=%s bta_handle=0x%x peer_id=%d", + __PRETTY_FUNCTION__, peer->PeerAddress().ToString().c_str(), + bta_handle, peer_id); + } else { + BTIF_TRACE_WARNING( + "%s: Correct peer: peer_address=%s bta_handle=0x%x->0x%x peer_id=%d", + __PRETTY_FUNCTION__, peer->PeerAddress().ToString().c_str(), + peer->BtaHandle(), bta_handle, peer_id); + } peer->SetBtaHandle(bta_handle); } } @@ -1237,9 +1254,14 @@ BtifAvPeer* BtifAvSink::FindOrCreatePeer(const RawAddress& peer_address, // Get the BTA Handle (if known) if (bta_handle == kBtaHandleUnknown) { auto it = peer_id2bta_handle_.find(peer_id); - if (it != peer_id2bta_handle_.end()) { - bta_handle = it->second; + if (it == peer_id2bta_handle_.end() || it->second == kBtaHandleUnknown) { + BTIF_TRACE_ERROR( + "%s: Cannot create peer for peer_address=%s : " + "cannot convert Peer ID=%d to unique BTA Handle", + __PRETTY_FUNCTION__, peer_address.ToString().c_str(), peer_id); + return nullptr; } + bta_handle = it->second; } LOG_INFO(LOG_TAG, @@ -1330,7 +1352,18 @@ void BtifAvSink::BtaHandleRegistered(uint8_t peer_id, tBTA_AV_HNDL bta_handle) { // Set the BTA Handle for the Peer (if exists) BtifAvPeer* peer = FindPeerByPeerId(peer_id); - if (peer != nullptr) { + if (peer != nullptr && peer->BtaHandle() != bta_handle) { + if (peer->BtaHandle() == kBtaHandleUnknown) { + BTIF_TRACE_EVENT( + "%s: Assign peer: peer_address=%s bta_handle=0x%x peer_id=%d", + __PRETTY_FUNCTION__, peer->PeerAddress().ToString().c_str(), + bta_handle, peer_id); + } else { + BTIF_TRACE_WARNING( + "%s: Correct peer: peer_address=%s bta_handle=0x%x->0x%x peer_id=%d", + __PRETTY_FUNCTION__, peer->PeerAddress().ToString().c_str(), + peer->BtaHandle(), bta_handle, peer_id); + } peer->SetBtaHandle(bta_handle); } } -- 2.11.0