[FFmpeg-devel] Race conditions in libavcodec/pthread.c

[Aaron Colwell]

On Thu, Aug 25, 2011 at 1:31 PM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de>wrote:

> On Thu, Aug 25, 2011 at 11:26:09AM -0700, Aaron Colwell wrote:> 4. Why is
> "double-checked locking" being used? Will there be significant
> > protest if I remove it?
>
> It would help if you pointed out there. I don't see any case of it
> at a quick glance.
>

park_frame_worker_threads() has an instance. p->state shouldn't be checked
outside of the lock.
frame_worker_thread() has an instance at the top of the while loop.
submit_packet() has an instance after the if(prev_thread)  line.
ff_thread_decode_frame() has an instance at the top of the do loop.


> > 5. What is the relationship between PerThreadContext::mutex
> > & PerThreadContext::progress_mutex and what member variables are they
> > supposed to protect?
>
> Obviously input_cond and progress_cond.
> Beyond that it get murky, however it is likely progress_mutex would be
> for things that change during frame process while mutex for things
> that change between frames.
>
> > At times these 3 cases appear to modify the same state variables.
>
> Being more specific would allow us to give more specific answers.
>

Sorry about that. The main one I've seen so far is in frame_worker_thread().
ff_thread_finish_setup() is called with and w/o p->mutex protection in that
method. ff_thread_finish_setup() locks p->progress_mutex so it sets up a
situation where locks may be acquired out of order. I haven't completely
proven to myself this will lead to deadlock, but it seems like a red flag
that something isn't right.


Aaron