On 08/12/2016 01:16, Wan-Teh Chang wrote:
> Make the one-time initialization in av_get_cpu_flags() thread-safe. The
> static variables |flags|, |cpuflags_mask|, and |checked| in
> libavutil/cpu.c are read and written using normal load and store
> operations. These are considered as data races. The fix is to use atomic
> load and store operations.
> 
> Remove the |checked| variable because the invalid value of -1 for
> |flags| can be used to indicate the same condition. Rename |flags| to
> |cpu_flags| and move it to file scope.
> 
> The fix can be verified by running the libavutil/tests/cpu_init.c test
> program under ThreadSanitizer:
>     ./configure --toolchain=clang-tsan
>     make libavutil/tests/cpu_init
>     libavutil/tests/cpu_init
> 
> There should be no warnings from ThreadSanitizer.
> 
> Co-author: Dmitry Vyukov of Google, who suggested the data race fix.
> 
> Signed-off-by: Wan-Teh Chang <[email protected]>
> ---
>  libavutil/Makefile         |  2 ++
>  libavutil/cpu.c            | 34 ++++++++++++------------
>  libavutil/cpu.h            |  2 --
>  libavutil/tests/.gitignore |  1 +
>  libavutil/tests/cpu_init.c | 65 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/fate/libavutil.mak   |  6 +++++
>  6 files changed, 92 insertions(+), 18 deletions(-)
>  create mode 100644 libavutil/tests/cpu_init.c 

Looks fine to me overall (I considered if leveraging pthread_once would
make it simpler, but I think does not)

I'll push it tomorrow if nobody is against it.

lu

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to