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

Commit a2c917d8 authored by Marie Janssen's avatar Marie Janssen
Browse files

Replace pthread_mutex with std::mutex 

In an effort to simplify and reduce errors, replace pthread_mutexes
with std equivalents.

Test: run unit tests & manual sanity checks
Change-Id: I5c33e0b4f4c85133ae4f7415cd19372fc4c7ca1a
parent 604ae13b

system/audio_a2dp_hw/audio_a2dp_hw.cc

+30 −30
Original line number Diff line number Diff line
@@ -29,7 +29,6 @@ 
#include <errno.h>

#include <fcntl.h>

#include <inttypes.h>

#include <pthread.h>

#include <stdint.h>

#include <sys/errno.h>

#include <sys/socket.h>
@@ -38,6 +37,8 @@ 
#include <sys/un.h>

#include <unistd.h>



#include <mutex>



#include <hardware/audio.h>

#include <hardware/hardware.h>

#include <system/audio.h>
@@ -107,7 +108,7 @@ struct a2dp_config { 
/* move ctrl_fd outside output stream and keep open until HAL unloaded ? */



struct a2dp_stream_common {

  pthread_mutex_t lock;

  std::recursive_mutex* mutex;

  int ctrl_fd;

  int audio_fd;

  size_t buffer_sz;
@@ -551,13 +552,9 @@ static void a2dp_open_ctrl_path(struct a2dp_stream_common* common) { 
 ****************************************************************************/



static void a2dp_stream_common_init(struct a2dp_stream_common* common) {

  pthread_mutexattr_t lock_attr;



  FNLOG();



  pthread_mutexattr_init(&lock_attr);

  pthread_mutexattr_settype(&lock_attr, PTHREAD_MUTEX_RECURSIVE);

  pthread_mutex_init(&common->lock, &lock_attr);

  common->mutex = new std::recursive_mutex;



  common->ctrl_fd = AUDIO_SKT_DISCONNECTED;

  common->audio_fd = AUDIO_SKT_DISCONNECTED;
@@ -567,6 +564,13 @@ static void a2dp_stream_common_init(struct a2dp_stream_common* common) { 
  common->buffer_sz = AUDIO_STREAM_OUTPUT_BUFFER_SZ;

}



static void a2dp_stream_common_destroy(struct a2dp_stream_common* common) {

  FNLOG();



  delete common->mutex;

  common->mutex = NULL;

}



static int start_audio_datapath(struct a2dp_stream_common* common) {

  INFO("state %d", common->state);
@@ -656,7 +660,7 @@ static ssize_t out_write(struct audio_stream_out* stream, const void* buffer, 


  DEBUG("write %zu bytes (fd %d)", bytes, out->common.audio_fd);



  pthread_mutex_lock(&out->common.lock);

  std::unique_lock<std::recursive_mutex> lock(*out->common.mutex);

  if (out->common.state == AUDIO_A2DP_STATE_SUSPENDED ||

      out->common.state == AUDIO_A2DP_STATE_STOPPING) {

    DEBUG("stream suspended or closing");
@@ -674,9 +678,9 @@ static ssize_t out_write(struct audio_stream_out* stream, const void* buffer, 
    goto finish;

  }



  pthread_mutex_unlock(&out->common.lock);

  lock.unlock();

  sent = skt_write(out->common.audio_fd, buffer, bytes);

  pthread_mutex_lock(&out->common.lock);

  lock.lock();



  if (sent == -1) {

    skt_disconnect(out->common.audio_fd);
@@ -694,7 +698,7 @@ finish:; 
  const size_t frames = bytes / audio_stream_out_frame_size(stream);

  out->frames_rendered += frames;

  out->frames_presented += frames;

  pthread_mutex_unlock(&out->common.lock);

  lock.unlock();



  // If send didn't work out, sleep to emulate write delay.

  if (sent == -1) {
@@ -766,12 +770,11 @@ static int out_standby(struct audio_stream* stream) { 


  FNLOG();



  pthread_mutex_lock(&out->common.lock);

  std::lock_guard<std::recursive_mutex> lock(*out->common.mutex);

  // Do nothing in SUSPENDED state.

  if (out->common.state != AUDIO_A2DP_STATE_SUSPENDED)

    retVal = suspend_audio_datapath(&out->common, true);

  out->frames_rendered = 0;  // rendered is reset, presented is not

  pthread_mutex_unlock(&out->common.lock);



  return retVal;

}
@@ -794,7 +797,7 @@ static int out_set_parameters(struct audio_stream* stream, 


  if (params.empty()) return status;



  pthread_mutex_lock(&out->common.lock);

  std::lock_guard<std::recursive_mutex> lock(*out->common.mutex);



  /* dump params */

  hash_map_utils_dump_string_keys_string_values(params);
@@ -816,8 +819,6 @@ static int out_set_parameters(struct audio_stream* stream, 
    /* Irrespective of the state, return 0 */

  }



  pthread_mutex_unlock(&out->common.lock);



  return status;

}
@@ -833,7 +834,7 @@ static char* out_get_parameters(const struct audio_stream* stream, 


  if (params.empty()) return strdup("");



  pthread_mutex_lock(&out->common.lock);

  std::lock_guard<std::recursive_mutex> lock(*out->common.mutex);



  if (a2dp_read_output_audio_config(&out->common) < 0) {

    ERROR("a2dp_read_output_audio_config failed");
@@ -904,8 +905,6 @@ static char* out_get_parameters(const struct audio_stream* stream, 
  }



done:

  pthread_mutex_unlock(&out->common.lock);



  std::string result;

  for (const auto& ptr : return_params) {

    result += ptr.first + "=" + ptr.second + ";";
@@ -949,7 +948,7 @@ static int out_get_presentation_position(const struct audio_stream_out* stream, 
  if (stream == NULL || frames == NULL || timestamp == NULL) return -EINVAL;



  int ret = -EWOULDBLOCK;

  pthread_mutex_lock(&out->common.lock);

  std::lock_guard<std::recursive_mutex> lock(*out->common.mutex);

  uint64_t latency_frames =

      (uint64_t)out_get_latency(stream) * out->common.cfg.rate / 1000;

  if (out->frames_presented >= latency_frames) {
@@ -958,7 +957,6 @@ static int out_get_presentation_position(const struct audio_stream_out* stream, 
                  timestamp);  // could also be associated with out_write().

    ret = 0;

  }

  pthread_mutex_unlock(&out->common.lock);

  return ret;

}
@@ -969,7 +967,7 @@ static int out_get_render_position(const struct audio_stream_out* stream, 
  FNLOG();

  if (stream == NULL || dsp_frames == NULL) return -EINVAL;



  pthread_mutex_lock(&out->common.lock);

  std::lock_guard<std::recursive_mutex> lock(*out->common.mutex);

  uint64_t latency_frames =

      (uint64_t)out_get_latency(stream) * out->common.cfg.rate / 1000;

  if (out->frames_rendered >= latency_frames) {
@@ -977,7 +975,6 @@ static int out_get_render_position(const struct audio_stream_out* stream, 
  } else {

    *dsp_frames = 0;

  }

  pthread_mutex_unlock(&out->common.lock);

  return 0;

}
@@ -1081,7 +1078,7 @@ static ssize_t in_read(struct audio_stream_in* stream, void* buffer, 


  DEBUG("read %zu bytes, state: %d", bytes, in->common.state);



  pthread_mutex_lock(&in->common.lock);

  std::unique_lock<std::recursive_mutex> lock(*in->common.mutex);

  if (in->common.state == AUDIO_A2DP_STATE_SUSPENDED ||

      in->common.state == AUDIO_A2DP_STATE_STOPPING) {

    DEBUG("stream suspended");
@@ -1099,9 +1096,9 @@ static ssize_t in_read(struct audio_stream_in* stream, void* buffer, 
    goto error;

  }



  pthread_mutex_unlock(&in->common.lock);

  lock.unlock();

  read = skt_read(in->common.audio_fd, buffer, bytes);

  pthread_mutex_lock(&in->common.lock);

  lock.lock();

  if (read == -1) {

    skt_disconnect(in->common.audio_fd);

    in->common.audio_fd = AUDIO_SKT_DISCONNECTED;
@@ -1117,13 +1114,12 @@ static ssize_t in_read(struct audio_stream_in* stream, void* buffer, 
    memset(buffer, 0, bytes);

    read = bytes;

  }

  pthread_mutex_unlock(&in->common.lock);

  lock.unlock();



  DEBUG("read %d bytes out of %zu bytes", read, bytes);

  return read;



error:

  pthread_mutex_unlock(&in->common.lock);

  memset(buffer, 0, bytes);

  us_delay = calc_audiotime_usec(in->common.cfg, bytes);

  DEBUG("emulate a2dp read delay (%d us)", us_delay);
@@ -1222,6 +1218,7 @@ static int adev_open_output_stream(struct audio_hw_device* dev, 
  return 0;



err_open:

  a2dp_stream_common_destroy(&out->common);

  free(out);

  *stream_out = NULL;

  a2dp_dev->output = NULL;
@@ -1236,7 +1233,7 @@ static void adev_close_output_stream(struct audio_hw_device* dev, 


  INFO("closing output (state %d)", out->common.state);



  pthread_mutex_lock(&out->common.lock);

  std::unique_lock<std::recursive_mutex> lock(*out->common.mutex);

  if ((out->common.state == AUDIO_A2DP_STATE_STARTED) ||

      (out->common.state == AUDIO_A2DP_STATE_STOPPING)) {

    stop_audio_datapath(&out->common);
@@ -1244,9 +1241,10 @@ static void adev_close_output_stream(struct audio_hw_device* dev, 


  skt_disconnect(out->common.ctrl_fd);

  out->common.ctrl_fd = AUDIO_SKT_DISCONNECTED;

  lock.unlock();

  a2dp_stream_common_destroy(&out->common);

  free(stream);

  a2dp_dev->output = NULL;

  pthread_mutex_unlock(&out->common.lock);



  DEBUG("done");

}
@@ -1384,6 +1382,7 @@ static int adev_open_input_stream(struct audio_hw_device* dev, 
  return 0;



err_open:

  a2dp_stream_common_destroy(&in->common);

  free(in);

  *stream_in = NULL;

  a2dp_dev->input = NULL;
@@ -1405,6 +1404,7 @@ static void adev_close_input_stream(struct audio_hw_device* dev, 


  skt_disconnect(in->common.ctrl_fd);

  in->common.ctrl_fd = AUDIO_SKT_DISCONNECTED;

  a2dp_stream_common_destroy(&in->common);

  free(stream);

  a2dp_dev->input = NULL;

system/btcore/src/module.cc

+7 −11
Original line number Diff line number Diff line
@@ -20,8 +20,9 @@ 


#include <assert.h>

#include <dlfcn.h>

#include <pthread.h>

#include <string.h>



#include <mutex>

#include <unordered_map>



#include "btcore/include/module.h"
@@ -37,20 +38,17 @@ typedef enum { 


static std::unordered_map<const module_t*, module_state_t> metadata;



// Include this lock for now for correctness, while the startup sequence is

// being refactored

static pthread_mutex_t metadata_lock;

// TODO(jamuraa): remove this lock after the startup sequence is clean

static std::mutex metadata_mutex;



static bool call_lifecycle_function(module_lifecycle_fn function);

static module_state_t get_module_state(const module_t* module);

static void set_module_state(const module_t* module, module_state_t state);



void module_management_start(void) { pthread_mutex_init(&metadata_lock, NULL); }

void module_management_start(void) {}



void module_management_stop(void) {

  metadata.clear();



  pthread_mutex_destroy(&metadata_lock);

}



const module_t* get_module(const char* name) {
@@ -150,17 +148,15 @@ static bool call_lifecycle_function(module_lifecycle_fn function) { 
}



static module_state_t get_module_state(const module_t* module) {

  pthread_mutex_lock(&metadata_lock);

  std::lock_guard<std::mutex> lock(metadata_mutex);

  auto map_ptr = metadata.find(module);

  pthread_mutex_unlock(&metadata_lock);



  return (map_ptr != metadata.end()) ? map_ptr->second : MODULE_STATE_NONE;

}



static void set_module_state(const module_t* module, module_state_t state) {

  pthread_mutex_lock(&metadata_lock);

  std::lock_guard<std::mutex> lock(metadata_mutex);

  metadata[module] = state;

  pthread_mutex_unlock(&metadata_lock);

}



// TODO(zachoverflow): remove when everything modulized

system/btcore/src/osi_module.cc

+0 −3
Original line number Diff line number Diff line
@@ -23,19 +23,16 @@ 
#include "osi/include/alarm.h"

#include "osi/include/future.h"

#include "osi/include/log.h"

#include "osi/include/mutex.h"

#include "osi/include/osi.h"

#include "osi/include/wakelock.h"



future_t* osi_init(void) {

  mutex_init();

  return future_new_immediate(FUTURE_SUCCESS);

}



future_t* osi_clean_up(void) {

  alarm_cleanup();

  wakelock_cleanup();

  mutex_cleanup();

  return future_new_immediate(FUTURE_SUCCESS);

}

system/hci/src/btsnoop_net.cc

+5 −5
Original line number Diff line number Diff line
@@ -30,6 +30,8 @@ 
#include <sys/types.h>

#include <unistd.h>



#include <mutex>



#include "osi/include/log.h"

#include "osi/include/osi.h"
@@ -42,7 +44,7 @@ static const int LISTEN_PORT_ = 8872; 


static pthread_t listen_thread_;

static bool listen_thread_valid_ = false;

static pthread_mutex_t client_socket_lock_ = PTHREAD_MUTEX_INITIALIZER;

static std::mutex client_socket_mutex_;

static int listen_socket_ = -1;

static int client_socket_ = -1;
@@ -79,7 +81,7 @@ void btsnoop_net_write(const void* data, size_t length) { 
  return;  // Disable using network sockets for security reasons

#endif



  pthread_mutex_lock(&client_socket_lock_);

  std::lock_guard<std::mutex> lock(client_socket_mutex_);

  if (client_socket_ != -1) {

    ssize_t ret;

    OSI_NO_INTR(ret = send(client_socket_, data, length, 0));
@@ -88,7 +90,6 @@ void btsnoop_net_write(const void* data, size_t length) { 
      safe_close_(&client_socket_);

    }

  }

  pthread_mutex_unlock(&client_socket_lock_);

}



static void* listen_fn_(UNUSED_ATTR void* context) {
@@ -139,12 +140,11 @@ static void* listen_fn_(UNUSED_ATTR void* context) { 


    /* When a new client connects, we have to send the btsnoop file header. This

     * allows a decoder to treat the session as a new, valid btsnoop file. */

    pthread_mutex_lock(&client_socket_lock_);

    std::lock_guard<std::mutex> lock(client_socket_mutex_);

    safe_close_(&client_socket_);

    client_socket_ = client_socket;



    OSI_NO_INTR(send(client_socket_, "btsnoop\0\0\0\0\1\0\0\x3\xea", 16, 0));

    pthread_mutex_unlock(&client_socket_lock_);

  }



cleanup:

system/hci/src/hci_layer.cc

+12 −20
Original line number Diff line number Diff line
@@ -21,12 +21,13 @@ 
#include "hci_layer.h"



#include <assert.h>

#include <pthread.h>

#include <signal.h>

#include <string.h>

#include <sys/types.h>

#include <unistd.h>



#include <mutex>



#include "btcore/include/module.h"

#include "btsnoop.h"

#include "buffer_allocator.h"
@@ -152,7 +153,7 @@ static fixed_queue_t* packet_queue; 
// Inbound-related

static alarm_t* command_response_timer;

static list_t* commands_pending_response;

static pthread_mutex_t commands_pending_response_lock;

static std::mutex commands_pending_response_mutex;

static packet_receive_data_t incoming_packets[INBOUND_PACKET_TYPE_COUNT];



// The hand-off point for data going to a higher layer, set by the higher layer
@@ -173,8 +174,6 @@ static future_t* start_up(void) { 
  command_credits = 1;

  firmware_is_configured = false;



  pthread_mutex_init(&commands_pending_response_lock, NULL);



  // For now, always use the default timeout on non-Android builds.

  period_ms_t startup_timeout_ms = DEFAULT_STARTUP_TIMEOUT_MS;
@@ -303,8 +302,6 @@ static future_t* shut_down() { 
  list_free(commands_pending_response);

  commands_pending_response = NULL;



  pthread_mutex_destroy(&commands_pending_response_lock);



  packet_fragmenter->cleanup();



  // Free the timers
@@ -410,27 +407,23 @@ static void firmware_config_callback(bool success) { 


  alarm_cancel(startup_timer);



  pthread_mutex_lock(&commands_pending_response_lock);

  std::lock_guard<std::mutex> lock(commands_pending_response_mutex);



  if (startup_future == NULL) {

    // The firmware configuration took too long - ignore the callback

    pthread_mutex_unlock(&commands_pending_response_lock);

    return;

  }

  firmware_is_configured = success;

  future_ready(startup_future, success ? FUTURE_SUCCESS : FUTURE_FAIL);

  startup_future = NULL;



  pthread_mutex_unlock(&commands_pending_response_lock);

}



static void startup_timer_expired(UNUSED_ATTR void* context) {

  LOG_ERROR(LOG_TAG, "%s", __func__);



  pthread_mutex_lock(&commands_pending_response_lock);

  std::lock_guard<std::mutex> lock(commands_pending_response_mutex);

  future_ready(startup_future, FUTURE_FAIL);

  startup_future = NULL;

  pthread_mutex_unlock(&commands_pending_response_lock);

}



// Postload functions
@@ -475,9 +468,10 @@ static void event_command_ready(fixed_queue_t* queue, 
    command_credits--;



    // Move it to the list of commands awaiting response

    pthread_mutex_lock(&commands_pending_response_lock);

    {

      std::lock_guard<std::mutex> lock(commands_pending_response_mutex);

      list_append(commands_pending_response, wait_entry);

    pthread_mutex_unlock(&commands_pending_response_lock);

    }



    // Send it off

    low_power_manager->wake_assert();
@@ -524,14 +518,14 @@ static void fragmenter_transmit_finished(BT_HDR* packet, 
}



static void command_timed_out(UNUSED_ATTR void* context) {

  pthread_mutex_lock(&commands_pending_response_lock);

  std::unique_lock<std::mutex> lock(commands_pending_response_mutex);



  if (list_is_empty(commands_pending_response)) {

    LOG_ERROR(LOG_TAG, "%s with no commands pending response", __func__);

  } else {

    waiting_command_t* wait_entry =

        static_cast<waiting_command_t*>(list_front(commands_pending_response));

    pthread_mutex_unlock(&commands_pending_response_lock);

    lock.unlock();



    // We shouldn't try to recover the stack from this command timeout.

    // If it's caused by a software bug, fix it. If it's a hardware bug, fix it.
@@ -779,7 +773,7 @@ static serial_data_type_t event_to_data_type(uint16_t event) { 
}



static waiting_command_t* get_waiting_command(command_opcode_t opcode) {

  pthread_mutex_lock(&commands_pending_response_lock);

  std::lock_guard<std::mutex> lock(commands_pending_response_mutex);



  for (const list_node_t* node = list_begin(commands_pending_response);

       node != list_end(commands_pending_response); node = list_next(node)) {
@@ -790,11 +784,9 @@ static waiting_command_t* get_waiting_command(command_opcode_t opcode) { 


    list_remove(commands_pending_response, wait_entry);



    pthread_mutex_unlock(&commands_pending_response_lock);

    return wait_entry;

  }



  pthread_mutex_unlock(&commands_pending_response_lock);

  return NULL;

}