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

Commit 1bc74c5e authored by Jakub Pawlowski's avatar Jakub Pawlowski
Browse files

Call BTA_GATTS_AddService on correct thread 

Right now BTA_GATTS_AddService is incorrectly called on btif thread.
This might lead to situations where it is executed while bta thread is
already cleaning up, and deleted the control blocks, resulting in crashes.
From now on, execute it on bta thread. This ensures that it is executed
before control blocks are freed.

Test: sl4a GattClientTest
Bug: 73054849
Change-Id: Ie2bf80414bceba2590c21d3825c78fbb58449520
parent 0e6e31ae

system/bta/gatt/bta_gatts_api.cc

+44 −27
Original line number Diff line number Diff line
@@ -24,9 +24,11 @@ 


#include "bt_target.h"



#include <base/bind.h>

#include <string.h>



#include "bt_common.h"

#include "bta_closure_api.h"

#include "bta_gatt_api.h"

#include "bta_gatts_int.h"

#include "bta_sys.h"
@@ -111,48 +113,63 @@ void BTA_GATTS_AppDeregister(tGATT_IF server_if) { 
  bta_sys_sendmsg(p_buf);

}



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

 *

 * Function         BTA_GATTS_AddService

 *

 * Description      Add the given |service| and all included elements to the

 *                  GATT database. a |BTA_GATTS_ADD_SRVC_EVT| is triggered to

 *                  report the status and attribute handles.

 *

 * Parameters       server_if: server interface.

 *                  service: pointer vector describing service.

 *

 * Returns          Returns |GATT_SUCCESS| on success or |GATT_ERROR| if the

 *                  service cannot be added.

 *

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

extern uint16_t BTA_GATTS_AddService(

    tGATT_IF server_if, std::vector<btgatt_db_element_t>& service) {

void bta_gatts_add_service_impl(tGATT_IF server_if,

                                std::vector<btgatt_db_element_t> service,

                                BTA_GATTS_AddServiceCb cb) {

  uint8_t rcb_idx =

      bta_gatts_find_app_rcb_idx_by_app_if(&bta_gatts_cb, server_if);



  LOG(INFO) << __func__ << ": rcb_idx=" << +rcb_idx;



  if (rcb_idx == BTA_GATTS_INVALID_APP) return GATT_ERROR;

  if (rcb_idx == BTA_GATTS_INVALID_APP) {

    cb.Run(GATT_ERROR, server_if, std::move(service));

    return;

  }



  uint8_t srvc_idx = bta_gatts_alloc_srvc_cb(&bta_gatts_cb, rcb_idx);

  if (srvc_idx == BTA_GATTS_INVALID_APP) return GATT_ERROR;

  if (srvc_idx == BTA_GATTS_INVALID_APP) {

    cb.Run(GATT_ERROR, server_if, std::move(service));

    return;

  }



  uint16_t status = GATTS_AddService(server_if, service.data(), service.size());

  if (status != GATT_SERVICE_STARTED) {

    memset(&bta_gatts_cb.srvc_cb[srvc_idx], 0, sizeof(tBTA_GATTS_SRVC_CB));

    LOG(ERROR) << __func__ << ": service creation failed.";

    cb.Run(GATT_ERROR, server_if, std::move(service));

    return;

  }



  if (status == GATT_SERVICE_STARTED) {

  bta_gatts_cb.srvc_cb[srvc_idx].service_uuid = service[0].uuid;



  // service_id is equal to service start handle

  bta_gatts_cb.srvc_cb[srvc_idx].service_id = service[0].attribute_handle;

  bta_gatts_cb.srvc_cb[srvc_idx].idx = srvc_idx;



    return GATT_SUCCESS;

  } else {

    memset(&bta_gatts_cb.srvc_cb[srvc_idx], 0, sizeof(tBTA_GATTS_SRVC_CB));

    LOG(ERROR) << __func__ << ": service creation failed.";

    return GATT_ERROR;

  cb.Run(GATT_SUCCESS, server_if, std::move(service));

  return;

}



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

 *

 * Function         BTA_GATTS_AddService

 *

 * Description      Add the given |service| and all included elements to the

 *                  GATT database. a |BTA_GATTS_ADD_SRVC_EVT| is triggered to

 *                  report the status and attribute handles.

 *

 * Parameters       server_if: server interface.

 *                  service: pointer vector describing service.

 *

 * Returns          Returns |GATT_SUCCESS| on success or |GATT_ERROR| if the

 *                  service cannot be added.

 *

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

extern void BTA_GATTS_AddService(tGATT_IF server_if,

                                 std::vector<btgatt_db_element_t> service,

                                 BTA_GATTS_AddServiceCb cb) {

  do_in_bta_thread(FROM_HERE, base::Bind(&bta_gatts_add_service_impl, server_if,

                                         std::move(service), std::move(cb)));

}



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

system/bta/include/bta_gatt_api.h

+7 −2
Original line number Diff line number Diff line
@@ -891,8 +891,13 @@ extern void BTA_GATTS_AppDeregister(tGATT_IF server_if); 
 *                  service cannot be added.

 *

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

extern uint16_t BTA_GATTS_AddService(tGATT_IF server_if,

                                     std::vector<btgatt_db_element_t>& service);

typedef base::Callback<void(uint8_t status, int server_if,

                            std::vector<btgatt_db_element_t> service)>

    BTA_GATTS_AddServiceCb;



extern void BTA_GATTS_AddService(tGATT_IF server_if,

                                 std::vector<btgatt_db_element_t> service,

                                 BTA_GATTS_AddServiceCb cb);



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

 *

system/btif/src/btif_gatt_server.cc

+10 −4
Original line number Diff line number Diff line
@@ -346,6 +346,12 @@ static bt_status_t btif_gatts_close(int server_if, const RawAddress& bd_addr, 
      Bind(&btif_gatts_close_impl, server_if, bd_addr, conn_id));

}



static void on_service_added_cb(uint8_t status, int server_if,

                                vector<btgatt_db_element_t> service) {

  HAL_CBACK(bt_gatt_callbacks, server->service_added_cb, status, server_if,

            std::move(service));

}



static void add_service_impl(int server_if,

                             vector<btgatt_db_element_t> service) {

  // TODO(jpawlowski): btif should be a pass through layer, and no checks should
@@ -359,16 +365,16 @@ static void add_service_impl(int server_if, 
    return;

  }



  int status = BTA_GATTS_AddService(server_if, service);

  HAL_CBACK(bt_gatt_callbacks, server->service_added_cb, status, server_if,

            std::move(service));

  BTA_GATTS_AddService(

      server_if, service,

      jni_thread_wrapper(FROM_HERE, base::Bind(&on_service_added_cb)));

}



static bt_status_t btif_gatts_add_service(int server_if,

                                          vector<btgatt_db_element_t> service) {

  CHECK_BTGATT_INIT();

  return do_in_jni_thread(

      Bind(&add_service_impl, server_if, std::move(service)));

      FROM_HERE, Bind(&add_service_impl, server_if, std::move(service)));

}



static bt_status_t btif_gatts_stop_service(int server_if, int service_handle) {