[Spice-devel] [PATCH spice] Provide thread safety between spice_server_playback_put_samples and snd_set_playback_latency.

[Jeremy White]

A MsgMainMultiMediaTime message on the main channel will be relayed
through main_dispatcher so as to be fired in the context of the
main (not worker) thread.

In qemu, that thread happens to also be the thread that drives the
audio channel, so it works.

In XSpice, it is a different thread, which leads to unpleasant
side effects.  The predominant side effect I noticed was an infinite loop
in snd_send_data.

This patch uses a mutex to prevent such collisions.

Signed-off-by: Jeremy White <jwhite at codeweavers.com>
---
 server/snd_worker.c |   30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/server/snd_worker.c b/server/snd_worker.c
index 70148b7..b977bed 100644
--- a/server/snd_worker.c
+++ b/server/snd_worker.c
@@ -115,6 +115,8 @@ struct SndChannel {
     snd_channel_handle_message_proc handle_message;
     snd_channel_on_message_done_proc on_message_done;
     snd_channel_cleanup_channel_proc cleanup;
+
+    pthread_mutex_t lock;
 };
 
 typedef struct PlaybackChannel PlaybackChannel;
@@ -220,8 +222,21 @@ static void snd_disconnect_channel(SndChannel *channel)
     spice_marshaller_destroy(channel->send_data.marshaller);
     snd_channel_put(channel);
     worker->connection = NULL;
+
+    pthread_mutex_destroy(&channel->lock);
+}
+
+static int snd_lock(SndChannel *channel)
+{
+    return pthread_mutex_lock(&channel->lock);
+}
+
+static void snd_unlock(SndChannel *channel)
+{
+    pthread_mutex_unlock(&channel->lock);
 }
 
+
 static void snd_playback_free_frame(PlaybackChannel *playback_channel, AudioFrame *frame)
 {
     frame->channel = playback_channel;
@@ -967,6 +982,8 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i
     if (!channel->channel_client) {
         goto error2;
     }
+
+    pthread_mutex_init(&channel->lock, NULL);
     return channel;
 
 error2:
@@ -1105,10 +1122,15 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
     frame = SPICE_CONTAINEROF(samples, AudioFrame, samples);
     playback_channel = frame->channel;
     spice_assert(playback_channel);
+
+    if (snd_lock(&playback_channel->base) != 0)
+        return;
+
     if (!snd_channel_put(&playback_channel->base) ||
         sin->st->worker.connection != &playback_channel->base) {
         /* lost last reference, channel has been destroyed previously */
         spice_info("audio samples belong to a disconnected channel");
+        snd_unlock(&playback_channel->base);
         return;
     }
     spice_assert(playback_channel->base.active);
@@ -1121,6 +1143,7 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance
     playback_channel->pending_frame = frame;
     snd_set_command(&playback_channel->base, SND_PLAYBACK_PCM_MASK);
     snd_playback_send(&playback_channel->base);
+    snd_unlock(&playback_channel->base);
 }
 
 void snd_set_playback_latency(RedClient *client, uint32_t latency)
@@ -1136,8 +1159,11 @@ void snd_set_playback_latency(RedClient *client, uint32_t latency)
                 PlaybackChannel* playback = (PlaybackChannel*)now->connection;
 
                 playback->latency = latency;
-                snd_set_command(now->connection, SND_PLAYBACK_LATENCY_MASK);
-                snd_playback_send(now->connection);
+                if (snd_lock(now->connection) == 0) {
+                    snd_set_command(now->connection, SND_PLAYBACK_LATENCY_MASK);
+                    snd_playback_send(now->connection);
+                    snd_unlock(now->connection);
+                }
             } else {
                 spice_debug("client doesn't not support SPICE_PLAYBACK_CAP_LATENCY");
             }
-- 
1.7.10.4