On Wed, Sep 24, 2014 at 8:06 PM, Jeremy White <jwhite@xxxxxxxxxxxxxxx> wrote:
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.
Is this difficult to solve? It seems the thread is reading from a fd and pushing to spice server. Couldn't this be done from X main loop?
This patch uses a mutex to prevent such collisions.
Is the mutex supposed to protect the whole SndChannel struct?It seems there are various code path that could be called with or without the lock held (snd_playback_send callback, for example). I would rather move the protected bits into a substructure, and make sure only access is made with the lock.
But I think we are going in a dangerous place if we start making spice server MT-safe api
Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
---
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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel