[Spice-devel] [Xspice 1/2] spiceqxl_audio: Fix a race condition in the audio playback

[Francois Gouget]

can_feed() depends on the time and thus could return false in
process_fifos(), causing it to stop reading from the fifos, and then
true in watch_or_wait() so that the wall_timer would not be set, but
the fifos would not be watched either because they already contain
data to process. The audio playback would then come to a stop.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
 src/spiceqxl_audio.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/src/spiceqxl_audio.c b/src/spiceqxl_audio.c
index eba9b10..1aebe8d 100644
--- a/src/spiceqxl_audio.c
+++ b/src/spiceqxl_audio.c
@@ -146,6 +146,7 @@ static void mix_in_one_fifo(struct fifo_data *f, int16_t *out, int len)
     free(in);
 }
 
+/* a helper for process_fifos() */
 static void mix_in_fifos(qxl_screen_t *qxl)
 {
     int i;
@@ -169,6 +170,7 @@ static void mix_in_fifos(qxl_screen_t *qxl)
     }
 }
 
+/* a helper for process_fifos() */
 static int can_feed(struct audio_data *data)
 {
     struct timeval end, diff;
@@ -190,6 +192,7 @@ static int can_feed(struct audio_data *data)
     return 0;
 }
 
+/* a helper for process_fifos() */
 static void did_feed(struct audio_data *data, int len)
 {
     struct timeval diff;
@@ -223,7 +226,7 @@ static void condense_fifos(struct audio_data *data)
     }
 }
 
-static void watch_or_wait(qxl_screen_t *qxl);
+static void start_watching(qxl_screen_t *qxl);
 static void process_fifos(qxl_screen_t *qxl, struct audio_data *data, int maxlen)
 {
     while (maxlen > 0) {
@@ -236,8 +239,13 @@ static void process_fifos(qxl_screen_t *qxl, struct audio_data *data, int maxlen
         if (! data->spice_buffer)
             break;
 
-        if (! can_feed(data))
-            break;
+        if (! can_feed(data)) {
+            if (! data->wall_timer_live) {
+                qxl->core->timer_start(data->wall_timer, PERIOD_MS);
+                data->wall_timer_live++;
+            }
+            return;
+        }
 
         mix_in_fifos(qxl);
 
@@ -248,7 +256,11 @@ static void process_fifos(qxl_screen_t *qxl, struct audio_data *data, int maxlen
         data->spice_buffer = NULL;
     }
 
-    watch_or_wait(qxl);
+    start_watching(qxl);
+    if (data->wall_timer_live) {
+        qxl->core->timer_cancel(data->wall_timer);
+    }
+    data->wall_timer_live = 0;
 }
 
 static void read_from_fifos(int fd, int event, void *opaque)
@@ -293,6 +305,7 @@ static void read_from_fifos(int fd, int event, void *opaque)
     process_fifos(qxl, data, maxlen);
 }
 
+/* a helper for process_fifos() */
 static void start_watching(qxl_screen_t *qxl)
 {
     struct audio_data *data = qxl->playback_opaque;
@@ -307,24 +320,6 @@ static void start_watching(qxl_screen_t *qxl)
     }
 }
 
-static void watch_or_wait(qxl_screen_t *qxl)
-{
-    struct audio_data *data = qxl->playback_opaque;
-
-    if (! can_feed(data)) {
-        if (! data->wall_timer_live) {
-            qxl->core->timer_start(data->wall_timer, PERIOD_MS);
-            data->wall_timer_live++;
-        }
-    }
-    else {
-        start_watching(qxl);
-        if (data->wall_timer_live)
-            qxl->core->timer_cancel(data->wall_timer);
-        data->wall_timer_live = 0;
-    }
-}
-
 static void wall_ticker(void *opaque)
 {
     qxl_screen_t *qxl = opaque;
-- 
2.7.0