Skip to content

Commit

Permalink
Fix some data races reported by TSAN in our tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@arkq
arkq committed Nov 9, 2022
1 parent 1326cb1 commit c0b1d29
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 36 deletions.
18 changes: 8 additions & 10 deletions src/ba-transport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1087,10 +1087,9 @@ int ba_transport_select_codec_sco(
return 0;
}

static void ba_transport_set_codec_a2dp(struct ba_transport *t) {

const uint16_t codec_id = t->type.codec;

static void ba_transport_set_codec_a2dp(
struct ba_transport *t,
uint16_t codec_id) {
switch (codec_id) {
case A2DP_CODEC_SBC:
a2dp_sbc_transport_init(t);
Expand Down Expand Up @@ -1134,12 +1133,11 @@ static void ba_transport_set_codec_a2dp(struct ba_transport *t) {
error("Unsupported A2DP codec: %#x", codec_id);
g_assert_not_reached();
}

}

static void ba_transport_set_codec_sco(struct ba_transport *t) {

const uint16_t codec_id = t->type.codec;
static void ba_transport_set_codec_sco(
struct ba_transport *t,
uint16_t codec_id) {

t->sco.spk_pcm.format = BA_TRANSPORT_PCM_FORMAT_S16_2LE;
t->sco.spk_pcm.channels = 1;
Expand Down Expand Up @@ -1187,10 +1185,10 @@ void ba_transport_set_codec(
t->type.codec = codec_id;

if (t->type.profile & BA_TRANSPORT_PROFILE_MASK_A2DP)
ba_transport_set_codec_a2dp(t);
ba_transport_set_codec_a2dp(t, codec_id);

if (t->type.profile & BA_TRANSPORT_PROFILE_MASK_SCO)
ba_transport_set_codec_sco(t);
ba_transport_set_codec_sco(t, codec_id);

}

Expand Down
4 changes: 3 additions & 1 deletion src/io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,10 @@ ssize_t io_poll_and_read_pcm(
/* Allow escaping from the poll() by thread cancellation. */
pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);

pthread_mutex_lock(&pcm->mutex);
/* Add PCM socket to the poll if it is active. */
fds[1].fd = ba_transport_pcm_is_active(pcm) ? pcm->fd : -1;
fds[1].fd = pcm->active ? pcm->fd : -1;
pthread_mutex_unlock(&pcm->mutex);

/* Poll for reading with optional sync timeout. */
switch (poll(fds, ARRAYSIZE(fds), io->timeout)) {
Expand Down
17 changes: 12 additions & 5 deletions test/test-io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,10 +442,7 @@ static void *test_io_thread_dump_pcm(struct ba_transport_thread *th) {
g_assert_not_reached();
}

struct pollfd pfds[] = {{ t_pcm->fd, POLLIN, 0 }};
size_t decoded_samples_total = 0;
uint8_t buffer[2048];
ssize_t len;

#if HAVE_SNDFILE
SNDFILE *sf = NULL;
Expand Down Expand Up @@ -483,8 +480,18 @@ static void *test_io_thread_dump_pcm(struct ba_transport_thread *th) {
}

debug_transport_thread_loop(th, "START");
ba_transport_thread_set_state_running(th);
while (poll(pfds, ARRAYSIZE(pfds), 500) > 0) {
for (ba_transport_thread_set_state_running(th);;) {

struct pollfd pfds[] = {{ -1, POLLIN, 0 }};
uint8_t buffer[2048];
ssize_t len;

pthread_mutex_lock(&t_pcm->mutex);
pfds[0].fd = t_pcm->fd;
pthread_mutex_unlock(&t_pcm->mutex);

if (poll(pfds, ARRAYSIZE(pfds), 500) <= 0)
break;

if ((len = read(pfds[0].fd, buffer, sizeof(buffer))) == -1) {
debug("PCM read error: %s", strerror(errno));
Expand Down
47 changes: 27 additions & 20 deletions test/test-rfcomm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ bool bluez_a2dp_set_configuration(const char *current_dbus_sep_path,
void bluez_battery_provider_update(struct ba_device *device) {
debug("%s: %p", __func__, device); }

static uint16_t get_codec_id(struct ba_transport *t) {
pthread_mutex_lock(&t->type_mtx);
uint16_t codec_id = t->type.codec;
pthread_mutex_unlock(&t->type_mtx);
return codec_id;
}

START_TEST(test_rfcomm) {

transport_codec_updated_cnt = 0;
Expand All @@ -105,8 +112,8 @@ START_TEST(test_rfcomm) {
struct ba_transport_type ttype_hf = { .profile = BA_TRANSPORT_PROFILE_HFP_HF };
struct ba_transport *hf = ba_transport_new_sco(device2, ttype_hf, ":test", "/sco/hf", fds[1]);

ck_assert_int_eq(ag->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_CVSD);

pthread_mutex_lock(&transport_codec_updated_mtx);
/* wait for codec selection (SLC established) signals */
Expand All @@ -117,8 +124,8 @@ START_TEST(test_rfcomm) {
ck_assert_int_eq(device1->ref_count, 1 + 1);
ck_assert_int_eq(device2->ref_count, 1 + 1);

ck_assert_int_eq(ag->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_CVSD);

debug("Audio gateway destroying");
ba_transport_destroy(ag);
Expand Down Expand Up @@ -154,8 +161,8 @@ START_TEST(test_rfcomm_esco) {

#if ENABLE_MSBC

ck_assert_int_eq(ag->type.codec, HFP_CODEC_UNDEFINED);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_UNDEFINED);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_UNDEFINED);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_UNDEFINED);

/* wait for SLC established */
while (ag->sco.rfcomm->state != HFP_SLC_CONNECTED)
Expand All @@ -165,8 +172,8 @@ START_TEST(test_rfcomm_esco) {

#else

ck_assert_int_eq(ag->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_CVSD);

pthread_mutex_lock(&transport_codec_updated_mtx);
/* wait for codec selection (SLC establishment) signals */
Expand All @@ -188,11 +195,11 @@ START_TEST(test_rfcomm_esco) {
#endif

#if ENABLE_MSBC
ck_assert_int_eq(ag->type.codec, HFP_CODEC_MSBC);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_MSBC);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_MSBC);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_MSBC);
#else
ck_assert_int_eq(ag->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_CVSD);
#endif

debug("Audio gateway destroying");
Expand Down Expand Up @@ -236,8 +243,8 @@ START_TEST(test_rfcomm_set_codec) {
* function. */
usleep(10000);

ck_assert_int_eq(ag->type.codec, HFP_CODEC_MSBC);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_MSBC);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_MSBC);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_MSBC);

/* select different audio codec */
ck_assert_int_eq(ba_transport_select_codec_sco(ag, HFP_CODEC_CVSD), 0);
Expand All @@ -248,14 +255,14 @@ START_TEST(test_rfcomm_set_codec) {
pthread_cond_wait(&transport_codec_updated, &transport_codec_updated_mtx);
pthread_mutex_unlock(&transport_codec_updated_mtx);

ck_assert_int_eq(ag->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_CVSD);

/* select already selected audio codec */
ck_assert_int_eq(ba_transport_select_codec_sco(ag, HFP_CODEC_CVSD), 0);

ck_assert_int_eq(ag->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_CVSD);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_CVSD);

/* switch back audio codec */
ck_assert_int_eq(ba_transport_select_codec_sco(ag, HFP_CODEC_MSBC), 0);
Expand All @@ -266,8 +273,8 @@ START_TEST(test_rfcomm_set_codec) {
pthread_cond_wait(&transport_codec_updated, &transport_codec_updated_mtx);
pthread_mutex_unlock(&transport_codec_updated_mtx);

ck_assert_int_eq(ag->type.codec, HFP_CODEC_MSBC);
ck_assert_int_eq(hf->type.codec, HFP_CODEC_MSBC);
ck_assert_int_eq(get_codec_id(ag), HFP_CODEC_MSBC);
ck_assert_int_eq(get_codec_id(hf), HFP_CODEC_MSBC);

} END_TEST
#endif
Expand Down

0 comments on commit c0b1d29

Please sign in to comment.