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

Commit ec7a3f8f 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: Ia6492b0007dca311ebd1579f52b206993b7535fd
parent c5108022
Loading
Loading
Loading
Loading
+30 −27
Original line number Diff line number Diff line
@@ -29,7 +29,7 @@
#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <pthread.h>
#include <mutex>
#include <stdint.h>
#include <sys/errno.h>
#include <sys/socket.h>
@@ -100,7 +100,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;
@@ -454,13 +454,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;
@@ -470,6 +466,14 @@ 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);
@@ -571,7 +575,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");
@@ -593,9 +597,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);
@@ -613,7 +617,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) {
@@ -695,12 +699,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;
}
@@ -725,7 +728,7 @@ static int out_set_parameters(struct audio_stream *stream, const char *kvpairs)
    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);
@@ -751,8 +754,6 @@ static int out_set_parameters(struct audio_stream *stream, const char *kvpairs)
        /* Irrespective of the state, return 0 */
    }

    pthread_mutex_unlock(&out->common.lock);

    return status;
}

@@ -803,14 +804,13 @@ static int out_get_presentation_position(const struct audio_stream_out *stream,
        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) {
        *frames = out->frames_presented - latency_frames;
        clock_gettime(CLOCK_MONOTONIC, timestamp); // could also be associated with out_write().
        ret = 0;
    }
    pthread_mutex_unlock(&out->common.lock);
    return ret;
}

@@ -823,14 +823,13 @@ static int out_get_render_position(const struct audio_stream_out *stream,
    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) {
        *dsp_frames = (uint32_t)(out->frames_rendered - latency_frames);
    } else {
        *dsp_frames = 0;
    }
    pthread_mutex_unlock(&out->common.lock);
    return 0;
}

@@ -951,7 +950,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)
    {
@@ -974,9 +973,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);
@@ -993,13 +992,13 @@ 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);
    lock.unlock();
    memset(buffer, 0, bytes);
    us_delay = calc_audiotime(in->common.cfg, bytes);
    DEBUG("emulate a2dp read delay (%d us)", us_delay);
@@ -1101,6 +1100,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;
@@ -1116,7 +1116,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);
@@ -1124,9 +1124,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");
}
@@ -1275,6 +1276,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;
@@ -1296,6 +1298,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;

+6 −10
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,8 +38,8 @@ 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);
@@ -46,13 +47,10 @@ 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_stop(void) {
  metadata.clear();

  pthread_mutex_destroy(&metadata_lock);
}

const module_t *get_module(const char *name) {
@@ -152,17 +150,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
+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);
}

+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:
+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;
}

Loading