On Wed, Nov 14, 2012 at 3:02 AM, David Henningsson david.henningsson@canonical.com wrote:
On 11/13/2012 08:41 PM, Trent Piepho wrote:
On Tue, Nov 13, 2012 at 8:50 AM, Takashi Iwai tiwai@suse.de wrote:
At Tue, 13 Nov 2012 13:39:24 +0000, Krakora Robert-B42341 wrote:
The way that the GStreamer ALSA Sink Plugin is using ALSA Lib assumes that it is thread safe. The fix crafted by one of Trent's colleagues (attached) seems to be the way to go. However, I don't know how it would impact other functionality within ALSA Lib.
No, sorry, we don't want to introduce pthread_lock in alsa-lib PCM stuff.
The problem with introduction serialization outside alsa-lib is that you must now serialize entire ALSA calls. The locks must be held for far too long.
Consider snd_pcm_writei(), most of the time is usually spent blocked waiting for a period to elapse. It is perfectly ok to call snd_pcm_delay() during this time. But if one isn't allowed to make any other pcm calls during snd_pcm_writei() then this can't be done.
It's a pretty big problem. Most code using blocking mode is going to write another period as soon as the writei call returns from the last write. It will spend almost all its time inside snd_pcm_writei() and thus will always be holding the app's pcm stream lock. As soon as it releases the lock it grabs it again for the next write. Another thread trying to call snd_pcm_delay() will virtually NEVER get a chance. Not only is it unnecessary stopped from getting the delay while another thread is blocked inside writei(), but it won't get a chance to call it between writei() calls either.
But there doesn't need to be a conflict, since the actual critical section that needs locking is very small, far smaller than the time spent sleeping inside writei() or wait(). How can just the critical section be protected without placing the locking inside alsa-lib?
Hmm, but the great majority of programs are not interested in accessing alsa-lib from multiple threads in the way that's currently unsafe. That includes programs who are very dependent on low latency. I would not like to see all these programs to suffer from the overhead of adding locks here and there, even if those locks are small.
Maybe they just don't know it's unsafe and don't hit a race often enough to notice?
You claim that the Gstreamer ALSA sink plugin accesses alsa-lib from two threads simultaneously. Could you elaborate on how this can happen, maybe it is easy to fix?
gstreamer has no lock around the call to snd_pcm_delay(). So it can race with snd_pcm_wait() or snd_pcm_writei(), which are called in another thread. There is a lock around the block of code calling wait()/writei(), but this lock isn't used for calling delay(). It seems that they didn't know there would be a problem.
And if you're not using the alsa rate plugin, then I don't think snd_pcm_delay() has a race with snd_pcm_writei(). delay for a hw device will call an ioctl to just get the delay. I think that's ok, assuming the kernel part has no races. It's only when using the rate plugin that snd_pcm_delay() will try to update the pointer and race with writei.
So unless you use the rate plugin, you'll never notice this race.