From d9a744b0d247f44235aa62d76bf0835ae7929b91 Mon Sep 17 00:00:00 2001 From: Jeremy Wu Date: Thu, 18 Apr 2024 02:35:50 +0000 Subject: Revert "CRAS: use mutex lock to guard |sr| and |sr_buf|" This reverts commit 9f329061a9b759b103c4a6103427d9f98c7723e6. Reason for revert: proper fix in CL:5461198 Original change's description: > CRAS: use mutex lock to guard |sr| and |sr_buf| > > There is a potential data race on the two variables where the audio > thread can be using them while the main thread destructs them. > > BUG=b:333619168 > TEST=test scenario 2 in bug and verify no crash > > Change-Id: Ic1b3e453c39452b17224a7374fb0323e6a7775d1 > Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/5449295 > Commit-Queue: Jeremy Wu > Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com > Reviewed-by: Cranel W BUG=b:333619168 Change-Id: I4d97e84eb34162788865d98a773007d8f54d7a8a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/5465179 Reviewed-by: Hsinyu Chao Bot-Commit: Rubber Stamper Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com Reviewed-by: Cranel W Commit-Queue: Jeremy Wu --- cras/src/server/cras_fl_pcm_iodev.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/cras/src/server/cras_fl_pcm_iodev.c b/cras/src/server/cras_fl_pcm_iodev.c index f80d55bd..ccb92da6 100644 --- a/cras/src/server/cras_fl_pcm_iodev.c +++ b/cras/src/server/cras_fl_pcm_iodev.c @@ -6,7 +6,6 @@ #include "cras/src/server/cras_fl_pcm_iodev.h" #include -#include #include #include #include @@ -110,9 +109,6 @@ struct fl_pcm_io { struct cras_sr* sr; // Buffer to hold the sr input pcm samples. struct byte_buffer* sr_buf; - // The mutex to protect |sr| and |sr_buf| from being destructed - // by the main thread during its processing in the audio thread. - pthread_mutex_t sr_mutex; }; static int flush(const struct cras_iodev* iodev); @@ -468,8 +464,6 @@ static int hfp_socket_read_write_cb(void* arg, int revents) { // Allow last read before handling error or hang-up events. if (revents & POLLIN) { - pthread_mutex_lock(&idev->sr_mutex); - if (idev->sr) { swap_pcm_buf_and_sr_buf(idev); rc = hfp_read(idev); @@ -483,10 +477,7 @@ static int hfp_socket_read_write_cb(void* arg, int revents) { if (idev->sr) { cras_sr_process(idev->sr, idev->sr_buf, idev->pcm_buf); } - - pthread_mutex_unlock(&idev->sr_mutex); } - if (revents & (POLLERR | POLLHUP)) { syslog(LOG_WARNING, "Error polling SCO socket, revents %d", revents); if (revents & POLLHUP) { @@ -515,13 +506,9 @@ static int hfp_socket_read_write_cb(void* arg, int revents) { } static void fl_pcm_io_disable_cras_sr_bt(struct fl_pcm_io* fl_pcm_io) { - pthread_mutex_lock(&fl_pcm_io->sr_mutex); - byte_buffer_destroy(&fl_pcm_io->sr_buf); cras_sr_destroy(fl_pcm_io->sr); fl_pcm_io->sr = NULL; - - pthread_mutex_unlock(&fl_pcm_io->sr_mutex); } static int fl_pcm_io_get_sr_bt_model(struct fl_pcm_io* fl_pcm_io, @@ -1242,12 +1229,10 @@ struct cras_iodev* hfp_pcm_iodev_create(struct cras_hfp* hfp, goto error; } + // At this point, we know which codec is preferred. This will usually + // be the negotiated codec, but the spec allows to fallback to CVSD, + // in which case this will be updated in |hfp_open_dev|. if (iodev->direction == CRAS_STREAM_INPUT) { - pthread_mutex_init(&hfpio->sr_mutex, NULL); - - // At this point, we know which codec is preferred. This will usually - // be the negotiated codec, but the spec allows to fallback to CVSD, - // in which case this will be updated in |hfp_open_dev|. if (cras_floss_hfp_is_codec_format_supported( hfpio->hfp, HFP_CODEC_FORMAT_LC3_TRANSPARENT)) { iodev->active_node->btflags |= CRAS_BT_FLAG_SWB; -- cgit v1.2.3