[Spice-devel] Help with solving a thread safety issue

[Jeremy White]

I'm hoping to ask for some advice on the spice server.

Background:
I've hit a bug with Xspice.  The current implementation uses a separate 
thread to grab pcm data from a fifo.  That same thread creates and 
drives the playback channel, and then pushes that data out using 
spice_server_playback_put_samples.

(That may have been a poor choice; I no longer recall if there was a 
careful understanding of the inter thread issues at that time.  But 
please bear with me).

If you have adaptive streaming turned on, you will eventually get a 
update_client_playback_latency message on the display channel (which in 
the Xspice case is being driven by the main, Xorg, thread).  Run the 
stream long enough, and the lack of thread safety will eventually bite 
you.  (It tends to expose some lovely behaviors in the snd_worker.c 
code; snd_send_data will spin loop nicely if some basic assumptions 
don't line up).

It is interesting (to me, at any rate), that main_dispatcher.c is 
provided as a mechanism to ensure that a message is sent by the right 
thread.  Of course, in this case, it is used to make sure the main 
thread is called, which doesn't help.

Questions:

What's the best way to fix this?  The alternates I'm considering are:
   1.  Rewrite the spiceqxl_audio code to run out of the main loop.
       That's a fair bit of work, and risks discarding some potentially 
hard earned wisdom from the past.  (afair, that was the first 
implementation attempt, and it had issues).

   2.  Surgically modify the snd_worker.c calls that handle the Playback 
channel to introduce a mutex; lock and release that mutex around 
operations.  This is the direction I'm leaning, but I don't see a lot of 
evidence that this is commonly done.

   3.  Somehow modify or parallel main_dispatcher.c so that the latency 
update call is invoked in the context of the correct thread.

Suggestions or clue bats greatly appreciated.

Cheers,

Jeremy