On Tue, Aug 15, 2017 at 5:53 AM, Martin Ågren <martin.agren@xxxxxxxxx> wrote: > Using SANITIZE=thread made t5400-send-pack.sh hit the potential race > below. > > This is set_try_to_free_routine in wrapper.c. The race relates to the > reading of the "old" value. The caller doesn't care about the "old" > value, so this should be harmless right now. But it seems that using > this mechanism from multiple threads and restoring the earlier value > will probably not work out every time. (Not necessarily because of the > race in set_try_to_free_routine, but, e.g., because callers might not > restore the function pointer in the reverse order of how they > originally set it.) > > Properly "fixing" this for thread-safety would probably require some > redesigning, which at the time might not be warranted. I'm just posting > this for completeness. Maybe related read (also error handling in threads): https://public-inbox.org/git/20170619220036.22656-1-avarab@xxxxxxxxx/ > > Martin > > WARNING: ThreadSanitizer: data race (pid=21382) > Read of size 8 at 0x000000979970 by thread T1: > #0 set_try_to_free_routine wrapper.c:35 (git+0x0000006cde1c) > #1 prepare_trace_line trace.c:105 (git+0x0000006a3bf0) > #2 trace_strbuf_fl trace.c:185 (git+0x0000006a3bf0) > #3 packet_trace pkt-line.c:80 (git+0x0000005f9f43) > #4 packet_read pkt-line.c:309 (git+0x0000005fbe10) > #5 recv_sideband sideband.c:37 (git+0x000000684c5e) > #6 sideband_demux send-pack.c:216 (git+0x00000065a38c) > #7 run_thread run-command.c:933 (git+0x000000655a93) > #8 <null> <null> (libtsan.so.0+0x0000000230d9) > > Previous write of size 8 at 0x000000979970 by main thread: > #0 set_try_to_free_routine wrapper.c:38 (git+0x0000006cde39) > #1 prepare_trace_line trace.c:105 (git+0x0000006a3bf0) > #2 trace_strbuf_fl trace.c:185 (git+0x0000006a3bf0) > #3 packet_trace pkt-line.c:80 (git+0x0000005f9f43) > #4 packet_read pkt-line.c:324 (git+0x0000005fc0bb) > #5 packet_read_line_generic pkt-line.c:332 (git+0x0000005fc0bb) > #6 packet_read_line pkt-line.c:342 (git+0x0000005fc0bb) > #7 receive_unpack_status send-pack.c:149 (git+0x00000065c1e4) > #8 send_pack send-pack.c:581 (git+0x00000065c1e4) > #9 git_transport_push transport.c:574 (git+0x0000006ab2c1) > #10 transport_push transport.c:1068 (git+0x0000006ae5d5) > #11 push_with_options builtin/push.c:336 (git+0x00000049d580) > #12 do_push builtin/push.c:419 (git+0x00000049f57d) > #13 cmd_push builtin/push.c:591 (git+0x00000049f57d) > #14 run_builtin git.c:330 (git+0x00000040949e) > #15 handle_builtin git.c:538 (git+0x00000040949e) > #16 run_argv git.c:590 (git+0x000000409a0e) > #17 cmd_main git.c:667 (git+0x000000409a0e) > #18 main common-main.c:43 (git+0x0000004079d1) > > Location is global 'try_to_free_routine' of size 8 at 0x000000979970 (git+0x000000979970) > > Thread T1 (tid=21405, running) created by main thread at: > #0 pthread_create <null> (libtsan.so.0+0x000000027577) > #1 start_async run-command.c:1130 (git+0x0000006582e7) > #2 send_pack send-pack.c:562 (git+0x00000065b7c8) > #3 git_transport_push transport.c:574 (git+0x0000006ab2c1) > #4 transport_push transport.c:1068 (git+0x0000006ae5d5) > #5 push_with_options builtin/push.c:336 (git+0x00000049d580) > #6 do_push builtin/push.c:419 (git+0x00000049f57d) > #7 cmd_push builtin/push.c:591 (git+0x00000049f57d) > #8 run_builtin git.c:330 (git+0x00000040949e) > #9 handle_builtin git.c:538 (git+0x00000040949e) > #10 run_argv git.c:590 (git+0x000000409a0e) > #11 cmd_main git.c:667 (git+0x000000409a0e) > #12 main common-main.c:43 (git+0x0000004079d1) > > SUMMARY: ThreadSanitizer: data race wrapper.c:35 set_try_to_free_routine >
