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

[Reimar Döffinger]

On Thu, Aug 25, 2011 at 11:26:09AM -0700, Aaron Colwell wrote:
> Hi,
> 
> Some recent changes to Chromium unit tests have caused FFmpeg decoding to
> get regularly scrutinized by our
> ThreadSanitizer<http://www.chromium.org/developers/how-tos/using-valgrind/threadsanitizer>
> tool.
> This has led to the detection of a variety of race
> conditions<http://code.google.com/p/chromium/issues/detail?id=93932>
> in
> libavcodec/pthread.c . Quick inspection of the code reveals that there is
> significant use of the "double-checked locking" anti-pattern and
> inconsistent use of PerThreadContext::mutex and
> PerThreadContext::progress_mutex which are likely causing of most of the
> race conditions. I wanted to ask a few questions before jumping in and
> trying to fix this.
> 
> 1. Are you already aware of these issues?

No.

> 2. Is someone already working on fixing these?

No.

> 3. Who is the expert on libavcodec/pthread.c so I can direct questions to
> them?

Possibly whoever wrote that part, but possibly nobody.

> 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.

> 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.

Reimar