On Fri, Aug 26, 2011 at 5:56 AM, Ronald S. Bultje <rsbul...@gmail.com> wrote: > Hi Aaron, > > On Thu, Aug 25, 2011 at 5:01 PM, Aaron Colwell <acolw...@chromium.org> wrote: >> Some recent changes to Chromium unit tests have caused FFmpeg decoding to >> get regularly scrutinized by our ThreadSanitizer tool. This has led to the >> detection of a variety of race conditions in libavcodec/pthread.c . > > Before we start: are these _actual_ race conditions, i.e. stuff that > we can reproduce in terms of "this file generates invalid output when > played back with this many threads on this platform" (ideally > reproducibly), or just theoretical findings by a tool? > > (Note: I want to again emphasize how much I'd love to get my hands on > a tool like chess (by MS for VC++) on a relevant developer platform.) > >> Quick >> inspection of the code reveals that there is significant use of the >> "double-checked locking" anti-pattern > > Yeah, I've seen that during review. In my opinion it's fine, it leads > to not having to grab locks (sometimes) when not necessary. It does > look a little weird... I have no strong opinion on it. > >> and inconsistent use of >> PerThreadContext::mutex and PerThreadContext::progress_mutex which are >> likely causing of most of the race conditions. > > The mutexes don't protect variables, they protect particular state > transitions. Unfortunately, race-finder tools aren't very good at > these kind of concepts. Vitor played a little with them also, but > didn't have much luck in terms of finding actual bugs (Vitor, is this > accurate?).
I had a look at it, but I was more concerned with races in the picture buffers than in pthread.c, since it is where I expect to find real bugs. Since the threading model in FFmpeg is based on variables that says how much of each frame is decoded (and that the parts that finished decoding can be read without races), this is normally incompatible with ThreadSanitizer or helgrind, I've tried for a while to use helgrind client requests so it can understand ffmpeg threading model since it doesn't look incompatible with happens-before checkers, but I didn't try much. -Vitor _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
