bluez-alsa: Added a patch to fix the crash at PCM close 65/19365/1
authorThierry Bultel <thierry.bultel@iot.bzh>
Wed, 26 Dec 2018 20:33:45 +0000 (21:33 +0100)
committerThierry Bultel <thierry.bultel@iot.bzh>
Wed, 26 Dec 2018 20:33:45 +0000 (21:33 +0100)
This fixes the random crash at PCM close

Change-Id: I373e4b7e55d6d7d4c3be4a4a7a9f460c00758215
Signed-off-by: Thierry Bultel <thierry.bultel@iot.bzh>
meta-audio-4a-framework/recipes-connectivity/bluez-alsa/bluez-alsa/0004-Fixed-the-SIGSEGV-at-PCM-close.patch [new file with mode: 0644]
meta-audio-4a-framework/recipes-connectivity/bluez-alsa/bluez-alsa_git.bbappend

diff --git a/meta-audio-4a-framework/recipes-connectivity/bluez-alsa/bluez-alsa/0004-Fixed-the-SIGSEGV-at-PCM-close.patch b/meta-audio-4a-framework/recipes-connectivity/bluez-alsa/bluez-alsa/0004-Fixed-the-SIGSEGV-at-PCM-close.patch
new file mode 100644 (file)
index 0000000..52dbf77
--- /dev/null
@@ -0,0 +1,180 @@
+From 23a1f554565c0cb9fade1a8f86ab9a09bf3d04e7 Mon Sep 17 00:00:00 2001
+From: Thierry Bultel <thierry.bultel@iot.bzh>
+Date: Wed, 26 Dec 2018 10:06:38 +0100
+Subject: [PATCH] Fixed the SIGSEGV at PCM close
+
+There is a (still not identified) race condition at PCM
+close, producing a SIGSEGV when pthread_cancel is called.
+Instead of proposing a trick with pthread_setcancelstate,
+this fix introduces a cancellation pipe, on which the io_thread
+polls. This guaranties a better, safer, exit path.
+
+Signed-off-by: Thierry Bultel <thierry.bultel@iot.bzh>
+---
+ src/asound/bluealsa-pcm.c | 82 +++++++++++++++++++++++++++++++++++++++++------
+ 1 file changed, 72 insertions(+), 10 deletions(-)
+
+diff --git a/src/asound/bluealsa-pcm.c b/src/asound/bluealsa-pcm.c
+index 2136964..013aebb 100644
+--- a/src/asound/bluealsa-pcm.c
++++ b/src/asound/bluealsa-pcm.c
+@@ -67,6 +67,8 @@ struct bluealsa_pcm {
+       snd_pcm_uframes_t io_hw_boundary;
+       snd_pcm_uframes_t io_hw_ptr;
++      int cancelPipe[2];
++
+ };
+@@ -107,7 +109,9 @@ static void *io_thread(void *arg) {
+       struct asrsync asrs;
+       asrsync_init(&asrs, io->rate);
+-      debug("Starting IO loop");
++      const char * type = io->stream== SND_PCM_STREAM_CAPTURE?"CAPTURE":"PLAYBACK";
++
++      debug("------------ Starting IO loop (%s)-----------------------", type);
+       for (;;) {
+               int tmp;
+@@ -124,6 +128,11 @@ static void *io_thread(void *arg) {
+                       debug("IO thread resumed: %d", io->state);
+               }
++              if (pcm->io_started == false) {
++                      debug("io_thread not started -> exit");
++                      goto final;
++              }
++
+               snd_pcm_uframes_t io_ptr = pcm->io_ptr;
+               snd_pcm_uframes_t io_buffer_size = io->buffer_size;
+               snd_pcm_uframes_t io_hw_ptr = pcm->io_hw_ptr;
+@@ -152,21 +161,44 @@ static void *io_thread(void *arg) {
+               if (io_hw_ptr >= io_hw_boundary)
+                       io_hw_ptr -= io_hw_boundary;
++              struct pollfd fds[2];
++
++              fds[0].fd = pcm->pcm_fd;
++              fds[1].fd = pcm->cancelPipe[0];
++
++              if (io->stream == SND_PCM_STREAM_CAPTURE)
++                      fds[0].events |= POLLIN;
++              else
++                      fds[0].events |= POLLOUT;
++
++              fds[1].events = POLLIN | POLLHUP;
++
+               if (io->stream == SND_PCM_STREAM_CAPTURE) {
+                       /* Read the whole period "atomically". This will assure, that frames
+                        * are not fragmented, so the pointer can be correctly updated. */
+-                      while (len != 0 && (ret = read(pcm->pcm_fd, head, len)) != 0) {
++                      while (len != 0 &&
++                                      pcm->io_started) {
++
++                              int pollres = poll(fds, 2, -1);
++
++                              if ((fds[1].revents & POLLIN) != 0) {
++                                      debug("io_thread: CANCELLED");
++                                      goto final;
++                              }
++
++                              ret = read(pcm->pcm_fd, head, len);
+                               if (ret == -1) {
+-                                      if (errno == EINTR)
+-                                              continue;
++                                      if (errno == EINTR) {
++                                              break;
++                                      }
+                                       SNDERR("PCM FIFO read error: %s", strerror(errno));
+                                       goto final;
+                               }
+                               head += ret;
+                               len -= ret;
+-                      }
++                      }
+                       if (ret == 0)
+                               goto final;
+@@ -182,9 +214,18 @@ static void *io_thread(void *arg) {
+                       /* Perform atomic write - see the explanation above. */
+                       do {
++
++                              int pollres = poll(fds, 2, -1);
++
++                              if ((fds[1].revents & POLLIN) != 0) {
++                                      debug("io_thread: CANCELLED");
++                                      goto final;
++                              }
++
+                               if ((ret = write(pcm->pcm_fd, head, len)) == -1) {
+-                                      if (errno == EINTR)
+-                                              continue;
++                                      if (errno == EINTR) {
++                                              break;
++                                      }
+                                       SNDERR("PCM FIFO write error: %s", strerror(errno));
+                                       goto final;
+                               }
+@@ -251,12 +292,24 @@ static int bluealsa_start(snd_pcm_ioplug_t *io) {
+ static int bluealsa_stop(snd_pcm_ioplug_t *io) {
+       struct bluealsa_pcm *pcm = io->private_data;
+-      debug("Stopping");
++      debug("Stopping %p", pcm);
++      if (!pcm) {
++              debug("PCM ALREADY CLOSED !");
++              return 0;
++      }
+       if (pcm->io_started) {
+               pcm->io_started = false;
+-              pthread_cancel(pcm->io_thread);
++
++              char dummy = 0;
++
++              if (write(pcm->cancelPipe[1], &dummy, sizeof(dummy)) < 0)
++                      debug("Failed to write to the cancellation pipe");
++
+               pthread_join(pcm->io_thread, NULL);
+       }
++done:
++      debug("Stop done");
++
+       return 0;
+ }
+@@ -269,10 +322,14 @@ static snd_pcm_sframes_t bluealsa_pointer(snd_pcm_ioplug_t *io) {
+ static int bluealsa_close(snd_pcm_ioplug_t *io) {
+       struct bluealsa_pcm *pcm = io->private_data;
+-      debug("Closing plugin");
++      debug("Closing plugin %p", pcm);
+       close(pcm->fd);
+       close(pcm->event_fd);
++      close(pcm->cancelPipe[0]);
++      close(pcm->cancelPipe[1]);
+       free(pcm);
++      io->private_data = NULL;
++      debug("PCM closed");
+       return 0;
+ }
+@@ -649,6 +706,11 @@ SND_PCM_PLUGIN_DEFINE_FUNC(bluealsa) {
+       pcm->pcm_fd = -1;
+       pcm->delay_ex = delay;
++      if (pipe(pcm->cancelPipe) == -1) {
++              SNDERR("BlueALSA failed to create cancellation pipe: %s", strerror(errno));
++              ret = -errno;
++      }
++
+       if ((pcm->fd = bluealsa_open(interface)) == -1) {
+               SNDERR("BlueALSA connection failed: %s", strerror(errno));
+               ret = -errno;
+-- 
+2.16.4
+
index 43e4632..8c67037 100644 (file)
@@ -7,5 +7,4 @@ FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
 SRC_URI += "file://0001-build-and-link-with-a-shared-library.patch"
 SRC_URI += "file://0002-log-add-calling-function-name.patch"
 SRC_URI += "file://0003-increased-the-number-of-connexions-to-16.patch"
-
-
+SRC_URI += "file://0004-Fixed-the-SIGSEGV-at-PCM-close.patch"