[vlc-devel] [vlc-commits] auhal: implement drain

[Rémi Denis-Courmont]

Le mardi 26 mars 2013 22:58:25, David Fuhrmann a écrit :
> It covers the fill state of the buffer. And I am assume that loading a 32
> bit integer from RAM can be also considered as atomic (at least in the
> sense that the result is not inconsistent).

To disable compiler optimization you would need to add the volatile keyword at 
the very least. In other words, no. But it's irrelevant anyway.

> This is why I thought that if we use this result (stored into in another
> variable), it will be ok?
> 
> >> For this scenario I assumed that these will suffice for
> >> proper thread synchronization.
> > 
> > Locking the mutex makes no sense if you don't change state. You could
> > remove the locking and unlocking calls to the same effect.
> > 
> > Then it would be obvious that scheduling predictability was missing.
> > Hence I strongly suspect that waiting can still deadlock with
> > unfortunate timings. In particular, I do not see what synchronization
> > directive prevents the render thread from signalling the condition
> > variable in the short time window after the audio thread checks the
> > buffer tial but before it starts waiting in the condition.
> 
> But If I understand right, this is *exactly* the scenario where a mutex
> lock around vlc_cond_signal makes sense!?

> In this short time you are
> mentioning (between getting the size of the buffer and calling
> vlc_cond_wait again on the thread which calls flush) the mutex is locked.

The whole point is that this is the same problem and locking the mutex changes 
nothing. It just moves the race to the time between moving the tail and 
locking the mutex, instead of the time between moving the tail and signaling 
the variable. 

Hence the locking is useless and the code is wrong.

-- 
Rémi Denis-Courmont
http://www.remlab.net/