On Tue, Nov 22, 2016 at 11:28:48AM -0800, Wan-Teh Chang wrote: > Hi Michael, > > On Mon, Nov 21, 2016 at 4:35 PM, Michael Niedermayer > <mich...@niedermayer.cc> wrote: > > On Mon, Nov 21, 2016 at 03:37:49PM -0800, Wan-Teh Chang wrote: > >> Make the one-time initialization in av_get_cpu_flags() thread-safe. > >> The fix assumes -1 is an invalid value for cpu flags. > > > > please explain the bug / race that occurs and how it is fixed > > Note: I will include the following explanation in the next version of my > patch. > > The static variables |flags| and |checked| in libavutil/cpu.c are read > and written using normal load and store instructions. These are > considered as data races. The fix is to use atomic load and store > instructions. The fix also eliminates the |checked| variable because > the invalid value of -1 for |flags| can be used to indicate the same > condition. > > Our application runs into the data races because two threads call > avformat_find_stream_info() at the same time. > avformat_find_stream_info() calls av_parser_init(), which may > eventually call av_get_cpu_flag(), depending on the codec. > > I just wrote a minimal test case (libavutil/tests/cpu_init.c) that > reproduces the data races. The test program creates two threads that > call av_get_cpu_flags(). When built with --toolchain=clang-tsan, the > test program triggers two ThreadSanitizer warnings:
ok, i see th race but do you really need the atomic operations ? isnt merging the 2 variables into 1 as you do enough ? (i mean the tools might still complain but would there be an actual race remaining?) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
