From cd1c9083122ffa92a2d597527aaf9605d808d5a2 Mon Sep 17 00:00:00 2001
From: easy <g-easy@users.noreply.github.com>
Date: Thu, 1 Aug 2019 15:25:15 +1000
Subject: [PATCH] SpanImpl::name() has to grab a lock and copy the name.

Because SetName could change the name at any time.

This isn't a bug yet because only running_span_store calls into name(),
in a code path we're not using yet.

This error can be caught by -Wthread-safety-reference with clang-8 and
later.
---
 opencensus/trace/internal/running_span_store_impl.cc | 2 +-
 opencensus/trace/internal/span_impl.cc               | 5 +++++
 opencensus/trace/internal/span_impl.h                | 7 +++----
 opencensus/trace/internal/span_options_test.cc       | 2 +-
 tools/travis/build_bazel.sh                          | 2 +-
 5 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/opencensus/trace/internal/running_span_store_impl.cc b/opencensus/trace/internal/running_span_store_impl.cc
index cdcc7f6e..0ec2101f 100644
--- a/opencensus/trace/internal/running_span_store_impl.cc
+++ b/opencensus/trace/internal/running_span_store_impl.cc
@@ -67,7 +67,7 @@ RunningSpanStore::Summary RunningSpanStoreImpl::GetSummary() const {
   RunningSpanStore::Summary summary;
   absl::MutexLock l(&mu_);
   for (const auto& addr_span : spans_) {
-    const std::string& name = addr_span.second->name_constref();
+    const std::string name = addr_span.second->name();
     auto it = summary.per_span_name_summary.find(name);
     if (it != summary.per_span_name_summary.end()) {
       it->second.num_running_spans++;
diff --git a/opencensus/trace/internal/span_impl.cc b/opencensus/trace/internal/span_impl.cc
index d32b3c8b..66448a3b 100644
--- a/opencensus/trace/internal/span_impl.cc
+++ b/opencensus/trace/internal/span_impl.cc
@@ -161,6 +161,11 @@ bool SpanImpl::HasEnded() const {
   return has_ended_;
 }
 
+std::string SpanImpl::name() const {
+  absl::MutexLock l(&mu_);
+  return name_;
+}
+
 exporter::SpanData SpanImpl::ToSpanData() const {
   absl::MutexLock l(&mu_);
   // Make a deep copy of attributes.
diff --git a/opencensus/trace/internal/span_impl.h b/opencensus/trace/internal/span_impl.h
index 5ff62493..3e94c15b 100644
--- a/opencensus/trace/internal/span_impl.h
+++ b/opencensus/trace/internal/span_impl.h
@@ -94,10 +94,9 @@ class SpanImpl final {
   // Returns true if the span has ended.
   bool HasEnded() const LOCKS_EXCLUDED(mu_);
 
-  absl::string_view name() const { return name_; }
-
-  // Returns the name of the span as a constref string.
-  const std::string& name_constref() const { return name_; }
+  // Returns a copy of the current name of the Span, since SetName can be used
+  // to change it.
+  std::string name() const LOCKS_EXCLUDED(mu_);
 
   // Returns the SpanContext associated with this Span.
   SpanContext context() const { return context_; }
diff --git a/opencensus/trace/internal/span_options_test.cc b/opencensus/trace/internal/span_options_test.cc
index 1cdde891..1efe5d28 100644
--- a/opencensus/trace/internal/span_options_test.cc
+++ b/opencensus/trace/internal/span_options_test.cc
@@ -41,7 +41,7 @@ class SpanTestPeer {
     return span->span_impl_for_test()->status_;
   }
 
-  static absl::string_view GetName(Span* span) {
+  static std::string GetName(Span* span) {
     return span->span_impl_for_test()->name();
   }
 
diff --git a/tools/travis/build_bazel.sh b/tools/travis/build_bazel.sh
index 2a5827a8..790f73b1 100755
--- a/tools/travis/build_bazel.sh
+++ b/tools/travis/build_bazel.sh
@@ -24,7 +24,7 @@ export BAZEL_OPTIONS="$BAZEL_OPTIONS --experimental_ui_actions_shown=1"
 
 # Enable thread safety analysis (only works with clang).
 if [[ "$TRAVIS_COMPILER" = "clang" ]]; then
-  export BAZEL_OPTIONS="$BAZEL_OPTIONS --copt=-Werror=thread-safety"
+  export BAZEL_OPTIONS="$BAZEL_OPTIONS --copt=-Werror=thread-safety --copt=-Werror=thread-safety-reference"
 fi
 
 export BAZEL_VERSION="0.24.1"