On Thu, Feb 25, 2016 at 4:15 PM, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
>
> Similar to the other patch, please explain what this actually fixes,
> those sanitizer tools produce a lot of false positives and nonsensical
> warnings.
Yes, certainly.
In libavcodec/pthread_frame.c, we have:
46 /**
47 * Context used by codec threads and stored in their
AVCodecInternal thread_ ctx.
48 */
49 typedef struct PerThreadContext {
...
59 pthread_mutex_t progress_mutex; ///< Mutex used to protect
frame progress values and progress_cond.
The comment on line 59 documents that progress_mutex guards frame
progress values and progress_cond. (In fact, progress_mutex also
guards the |state| field in many places, but that's not the subject of
this patch.)
I assume "frame progess values" mean f->progress->data, where |f|
points to a ThreadFrame. I inspected libavcodec/pthread_frame.c and
concluded the code (ff_thread_report_progress and
ff_thread_await_progress) matches that comment, except that it also
does a double-checked locking style performance optimization. For
example, here is ff_thread_report_progress:
472 void ff_thread_report_progress(ThreadFrame *f, int n, int field)
473 {
474 PerThreadContext *p;
475 volatile int *progress = f->progress ? (int*)f->progress->data : NULL;
476
477 if (!progress || progress[field] >= n) return;
478
479 p = f->owner->internal->thread_ctx;
480
481 if (f->owner->debug&FF_DEBUG_THREADS)
482 av_log(f->owner, AV_LOG_DEBUG, "%p finished %d field
%d\n", progress , n, field);
483
484 pthread_mutex_lock(&p->progress_mutex);
485 progress[field] = n;
486 pthread_cond_broadcast(&p->progress_cond);
487 pthread_mutex_unlock(&p->progress_mutex);
488 }
progress[field] can only increase. So, both ff_thread_report_progress
and ff_thread_await_progress have a fast code path (line 477 and line
495, respectively) that reads progress[field] without acquiring
progress_mutex. If the speculatively read value of progress[field] is
>= |n|, then the functions return immediately. Otherwise, the
functions take the slow code path, which acquires progress_mutex
properly and does what the functions need to do.
The fast code path is in the style of double-checked locking and has a
data race. ThreadSanitizer is right in this case.
Wan-Teh Chang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
