Repository: kudu Updated Branches: refs/heads/master e14bb60cc -> dba7cddf7
tsan-suppressions: suppress various glog/gflags data races In an upcoming patch we'll begin building more of our thirdparty dependencies with -fsanitize=thread. Having tested that, I saw that TSAN now flags some new data races. All of them are "benign" in that they should be safe on x86. Nonetheless, we either need to patch glog/gflags to fix them, or suppress them. I chose the latter for expediency. Change-Id: I3906070cb0e202e5b841f5b163e4adc023fe3882 Reviewed-on: http://gerrit.cloudera.org:8080/4509 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/bafbc4a2 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/bafbc4a2 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/bafbc4a2 Branch: refs/heads/master Commit: bafbc4a2ca56080c8426e5ca7bcde87feb516e4b Parents: e14bb60 Author: Adar Dembo <[email protected]> Authored: Tue Sep 20 16:18:53 2016 -0700 Committer: Adar Dembo <[email protected]> Committed: Mon Sep 26 19:20:20 2016 +0000 ---------------------------------------------------------------------- build-support/tsan-suppressions.txt | 30 ++++++++++++++++----- src/kudu/client/client-test.cc | 1 - src/kudu/integration-tests/alter_table-test.cc | 4 --- src/kudu/util/stack_watchdog-test.cc | 3 --- 4 files changed, 24 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/bafbc4a2/build-support/tsan-suppressions.txt ---------------------------------------------------------------------- diff --git a/build-support/tsan-suppressions.txt b/build-support/tsan-suppressions.txt index e492b65..65f03bf 100644 --- a/build-support/tsan-suppressions.txt +++ b/build-support/tsan-suppressions.txt @@ -38,15 +38,33 @@ race:epoll_ctl # a lot of stress testing. race:concurrent_btree.h -# GLog's fatal signal handler isn't signal-safe -- it allocates memory. -# This isn't great, but nothing we can do about it. See -# https://code.google.com/p/google-glog/issues/detail?id=191 -signal:logging_fail - -# LOG(FATAL) from multiple threads can also end up triggering a TSAN error. # See https://github.com/google/glog/issues/80 for a general list of TSAN # issues in glog. +# 1. glog's fatal signal handler isn't signal-safe -- it allocates memory. +# This isn't great, but nothing we can do about it. See +# https://code.google.com/p/google-glog/issues/detail?id=191 +# 2. LOG(FATAL) from multiple threads can also end up triggering a TSAN error. +# 3. g_now_entering in stacktrace_libunwind-inl.h is reset to false without +# a Release_Store. +# 4. glog's ANNOTATE_BENIGN_RACE macro doesn't do anything. +# 5. Mutex::is_safe_ is accessed in an unsafe way. +# 6. vlocal__ is access in an unsafe way at every VLOG() or VLOG_IS_ON() +# call-site. +signal:logging_fail race:google::LogMessage::Init +race:google::GetStackTrace +race:google::InitVLOG3__ +race:glog_internal_namespace_::Mutex +race:vlocal__ + +# gflags variables are accessed without synchronization, but FlagSaver and other +# APIs acquire locks when accessing them. This should be safe on x86 for +# primitive flag types, but not for string flags, which is why fLS is omitted. +race:fLB:: +race:fLD:: +race:fLI:: +race:fLI64:: +race:fLU64:: # This method in Boost's UUID library operates on static state with impunity, # triggering (harmless) data races in TSAN when boost::uuids::random_generator http://git-wip-us.apache.org/repos/asf/kudu/blob/bafbc4a2/src/kudu/client/client-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc index 25b141a..8b8456b 100644 --- a/src/kudu/client/client-test.cc +++ b/src/kudu/client/client-test.cc @@ -1606,7 +1606,6 @@ int64_t SumResults(const KuduScanBatch& batch) { TEST_F(ClientTest, TestScannerKeepAlive) { ASSERT_NO_FATAL_FAILURE(InsertTestRows(client_table_.get(), 1000)); // Set the scanner ttl really low - ANNOTATE_BENIGN_RACE(&FLAGS_scanner_ttl_ms, "Set at runtime, for tests."); FLAGS_scanner_ttl_ms = 100; // 100 milliseconds // Start a scan but don't get the whole data back KuduScanner scanner(client_table_.get()); http://git-wip-us.apache.org/repos/asf/kudu/blob/bafbc4a2/src/kudu/integration-tests/alter_table-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/alter_table-test.cc b/src/kudu/integration-tests/alter_table-test.cc index 3f36269..20a48d7 100644 --- a/src/kudu/integration-tests/alter_table-test.cc +++ b/src/kudu/integration-tests/alter_table-test.cc @@ -96,10 +96,6 @@ class AlterTableTest : public KuduTest { FLAGS_enable_data_block_fsync = false; // Keep unit tests fast. FLAGS_use_hybrid_clock = false; - ANNOTATE_BENIGN_RACE(&FLAGS_flush_threshold_mb, - "safe to change at runtime"); - ANNOTATE_BENIGN_RACE(&FLAGS_enable_maintenance_manager, - "safe to change at runtime"); } void SetUp() override { http://git-wip-us.apache.org/repos/asf/kudu/blob/bafbc4a2/src/kudu/util/stack_watchdog-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/stack_watchdog-test.cc b/src/kudu/util/stack_watchdog-test.cc index a7cb06d..9bbb097 100644 --- a/src/kudu/util/stack_watchdog-test.cc +++ b/src/kudu/util/stack_watchdog-test.cc @@ -21,7 +21,6 @@ #include <string> #include <vector> -#include "kudu/gutil/dynamic_annotations.h" #include "kudu/gutil/strings/join.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/util/stopwatch.h" @@ -40,8 +39,6 @@ class StackWatchdogTest : public KuduTest { virtual void SetUp() OVERRIDE { KuduTest::SetUp(); KernelStackWatchdog::GetInstance()->SaveLogsForTests(true); - ANNOTATE_BENIGN_RACE(&FLAGS_hung_task_check_interval_ms, - "Integer flag change should be safe"); FLAGS_hung_task_check_interval_ms = 10; } };
