The problem here is that ignorelist did not work by some reason, if I
will look at the ignored functions it should not contain any TSan
interseption code, while it does:
$ lldb-13 clickhouse
(lldb) target create "clickhouse"
disas -n rd_avg_rollover
Current executable set to '/home/azat/ch/tmp/tsan-test/clickhouse' (x86_64).
(lldb) disas -n rd_avg_rollover
clickhouse`rd_kafka_stats_emit_avg:
clickhouse[0x1cbf84a7] <+39>: leaq 0x30(%r15), %r12
clickhouse[0x1cbf84ab] <+43>: movq %r12, %rdi
clickhouse[0x1cbf84ae] <+46>: callq 0x1ccdad40 ; rdk_thread_mutex_lock at tinycthread.c:111
clickhouse[0x1cbf84b3] <+51>: leaq 0x58(%r15), %rdi
clickhouse[0x1cbf84b7] <+55>: callq 0x71b5390 ; __tsan_read4
clickhouse[0x1cbf84bc] <+60>: cmpl $0x0, 0x58(%r15)
clickhouse[0x1cbf84c1] <+65>: je 0x1cbf8595 ; <+277> [inlined] rd_avg_rollover + 238 at rdavg.h
clickhouse[0x1cbf84c7] <+71>: leaq -0xc8(%rbp), %rdi
clickhouse[0x1cbf84ce] <+78>: xorl %esi, %esi
clickhouse[0x1cbf84d0] <+80>: callq 0x1ccdac80 ; rdk_thread_mutex_init at tinycthread.c:62
clickhouse[0x1cbf84d5] <+85>: leaq 0x5c(%r15), %rdi
clickhouse[0x1cbf84d9] <+89>: callq 0x71b5390 ; __tsan_read4
(lldb) disas -n rd_avg_calc
clickhouse`rd_kafka_broker_ops_io_serve:
clickhouse[0x1cbdf086] <+1990>: leaq 0x5a4(%rbx), %rdi
clickhouse[0x1cbdf08d] <+1997>: callq 0x71b5390 ; __tsan_read4
clickhouse[0x1cbdf092] <+2002>: cmpl $0x0, 0x5a4(%rbx)
clickhouse[0x1cbdf099] <+2009>: je 0x1cbdf12b ; <+2155> [inlined] rd_kafka_broker_timeout_scan + 719 at rdkafka_broker.c
I guess the reason is that they had been inlined
So now rd_avg_calc() guarded with a mutex.
Refs: https://github.com/ClickHouse/librdkafka/pull/11
Fixes: https://github.com/ClickHouse/ClickHouse/issues/60939
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
The problem is that TSan still fails [1] is that ignorelist does not
work for static functions without asterisk:
// test.cpp
#include <thread>
bool flag = false;
// avoid mangling
extern "C" {
static void set_flag_impl()
{
flag = true;
}
void set_flag()
{
set_flag_impl();
}
void set_flag_if()
{
if (flag)
flag = false;
}
}
int main()
{
std::thread t1([]{ set_flag(); });
std::thread t2([]{ set_flag_if(); });
t1.join();
t2.join();
return 0;
}
// ignorelist
[thread]
fun:set_flag_impl
$ clang++ -g -fno-omit-frame-pointer -fsanitize=thread -fsanitize-ignorelist=ignorelist -o test test.cpp && ./test
SUMMARY: ThreadSanitizer: data race /tmp/test-tsan-ignorelist/test.cpp:19:9 in set_flag_if
$ sed -i 's/set_flag_impl/*set_flag_impl*/' ignorelist
$ clang++ -g -fno-omit-frame-pointer -fsanitize=thread -fsanitize-ignorelist=ignorelist -o test test.cpp && ./test
OK
But, note that ignorelist is tricky, and will not work for
functions with __always_inline__ attribute for example.
P.S. set_flag_impl also has brackets in the output (i.e.
set_flag_impl()), while ther eis brackets for rd_avg_calc on CI [1].
[1]: https://s3.amazonaws.com/clickhouse-test-reports/63039/84bebc534ba7cf6e9dbfc1d91e8350939a84f87c/integration_tests__tsan__[6_6]//home/ubuntu/actions-runner/_work/_temp/test/output_dir/integration_run_parallel4_0.log
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
