[FFmpeg-devel] [PATCH 2/3] pthread : Ensure all state access is protected by progress_mutex

[Reimar Döffinger]

On Thu, Mar 22, 2012 at 09:56:00AM -0700, Aaron Colwell wrote:
> The current code appears to be inconsistent about holding the
> progress_mutex when it checks or updates the state member variable. This
> patch makes sure that progress_mutex is always held when state is checked
> or modified. I've tried to be conservative in my approach here under the
> assumption that protecting all sites is the safest initial strategy. If
> other conditions make the locking unnecessarly I'd really like to
> understand why and what mechanisms are in place to make sure those
> preconditions are always maintained.

As far as I can tell each assignment and check is associated with a
condition variable and its associated lock.
The lock at the assignment and related check place are the same.
Thus if you want to avoid issues with your tool without making a
mess of the code I'd expect splitting up the state variable into
multiple variables might work, though is likely to run into other
"issues".

> Questions:
> Why is double-check locking being used in several places? This seems like a
> popular threading anti-pattern.

To avoid needless performance loss.
anti-patterns are excuses for "I want to do something without
understanding it". They might actually be used in a buggy way,
but being declared an "anti-pattern" is not a reasonable argument.
They should be safe because the same thread assigned a value failing
the check before, and that thread is also the only one that can make
the condition fail again, thus the worst case should be that the check
fails right away which is the same as if you didn't have it at all.
In the more likely case you save a lock/unlock cycle.