Hi Ronald,
On Wed, Jul 5, 2017 at 3:31 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote:
> Hi Wan-Teh,
>
> On Wed, Jul 5, 2017 at 6:12 PM, Wan-Teh Chang <w...@google.com> wrote:
>>
>> Thank you for all the tsan warning fixes. In the meantime, it would be
>> good to revert 2e664b9c1e73c80aab91070c1eb7676f04bdd12d to avoid
>> confusion.
>
>
> Why? I believe it fixes a subset of the issue.
Could you explain why it fixes a subset of the issue?
The data race that tsan warned about was between a write and a read:
WARNING: ThreadSanitizer: data race (pid=10916)
Write of size 4 at 0x7d64000174fc by main thread (mutexes: write M2313):
#0 update_context_from_user src/libavcodec/pthread_frame.c:335
(ffmpeg+0x000000df7b06)
[..]
Previous read of size 4 at 0x7d64000174fc by thread T1 (mutexes: write M2311):
#0 ff_thread_await_progress src/libavcodec/pthread_frame.c:592
(ffmpeg+0x000000df8b3e)
But the write is not protected by the mutex that
2e664b9c1e73c80aab91070c1eb7676f04bdd12d uses to protect the reads.
I can't reproduce the tsan warning by running fate-h264 (with and
without 2e664b9c1e73c80aab91070c1eb7676f04bdd12d), but I can still
reproduce the tsan warning by running a test program in my project
with a recent (one or two weeks old) snapshot of ffmpeg:
WARNING: ThreadSanitizer: data race (pid=86821)
Read of size 4 at 0x7b640003b8fc by thread T19 (mutexes: write
M525086672791228232, write M524523722837806536):
#0 ff_thread_await_progress
[...]/libavcodec/pthread_frame.c:591:26
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1)
#1 await_references [...]/libavcodec/h264_mb.c
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x9530c4)
#2 hl_motion_420_simple_8 [...]/libavcodec/h264_mc_template.c:80:9
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x95106b)
#3 hl_decode_mb_simple_8 [...]/libavcodec/h264_mb_template.c:180
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x95106b)
#4 ff_h264_hl_decode_mb [...]/libavcodec/h264_mb.c:816:9
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x940953)
#5 decode_slice [...]/libavcodec/h264_slice.c:2572:17
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x982ce7)
#6 ff_h264_execute_decode_slices
[...]/libavcodec/h264_slice.c:2747:15
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x981fb9)
#7 decode_nal_units [...]/libavcodec/h264dec.c:718:27
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x99257b)
#8 h264_decode_frame [...]/libavcodec/h264dec.c:1008
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x99257b)
#9 frame_worker_thread [...]/libavcodec/pthread_frame.c:199:21
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe75b4c)
Previous write of size 4 at 0x7b640003b8fc by main thread (mutexes:
write M524242247861095840):
#0 update_context_from_user
[...]/libavcodec/pthread_frame.c:335:19
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859)
#1 submit_packet [...]/libavcodec/pthread_frame.c:396
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859)
#2 ff_thread_decode_frame [...]/libavcodec/pthread_frame.c:490
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859)
#3 decode_simple_internal [...]/libavcodec/decode.c:415:15
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x7d49b9)
#4 decode_simple_receive_frame [...]/libavcodec/decode.c:620
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x7d49b9)
#5 decode_receive_frame_internal [...]/libavcodec/decode.c:638
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x7d49b9)
#6 avcodec_send_packet [...]/libavcodec/decode.c:678:15
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x7d3ea7)
#7 compat_decode [...]/libavcodec/decode.c:853:15
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x7d627d)
#8 avcodec_decode_video2 [...]/libavcodec/decode.c:914:12
(5ab42bb1a6f4b068d7863dabe9b2bacc+0x7d617e)
...
The tsan warning shows the read and the write are still protected by
different mutexes. The read is now protected by two mutexes because of
2e664b9c1e73c80aab91070c1eb7676f04bdd12d.
I hope this convinces you that
2e664b9c1e73c80aab91070c1eb7676f04bdd12d is not working as intended
and should be reverted to avoid confusion.
Thanks,
Wan-Teh Chang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
