SpanImpl::name() has to grab a lock and copy the name. (#369)

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.

Author: g-easy

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++;

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.

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_; }

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();
   }
 

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"