Fix some TSAN problems in Activity Tracker/Analyzer.

base/debug/activity_tracker.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/debug/activity_tracker.h"
 
 #include "base/debug/stack_trace.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/pending_task.h"
 #include "base/process/process.h"
 #include "base/process/process_handle.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/threading/platform_thread.h"
 
 namespace base {
 namespace debug {
 
 namespace {
 
 // A number that identifies the memory as having been initialized. It's
-// arbitrary but happens to be the first 8 bytes of SHA1(ThreadActivityTracker).
+// arbitrary but happens to be the first 4 bytes of SHA1(ThreadActivityTracker).
 // A version number is added on so that major structure changes won't try to
 // read an older version (since the cookie won't match).
-const uint64_t kHeaderCookie = 0xC0029B240D4A3092ULL + 1;  // v1
+const uint32_t kHeaderCookie = 0xC0029B24UL + 2;  // v2
 
 // The minimum depth a stack should support.
 const int kMinStackDepth = 2;
 
 union ThreadRef {
   int64_t as_id;
 #if defined(OS_WIN)
   // On Windows, the handle itself is often a pseudo-handle with a common
   // value meaning "this thread" and so the thread-id is used. The former
   // can be converted to a thread-id with a system call.
@@ skipping 52 matching lines @@
 
 ActivitySnapshot::ActivitySnapshot() {}
 ActivitySnapshot::~ActivitySnapshot() {}
 
 
 // This information is kept for every thread that is tracked. It is filled
 // the very first time the thread is seen. All fields must be of exact sizes
 // so there is no issue moving between 32 and 64-bit builds.
 struct ThreadActivityTracker::Header {
   // This unique number indicates a valid initialization of the memory.
-  uint64_t cookie;
+  std::atomic<uint32_t> cookie;
+  uint32_t reserved;  // pad out to 64 bits
 
   // The process-id and thread-id (thread_ref.as_id) to which this data belongs.
   // These identifiers are not guaranteed to mean anything but are unique, in
   // combination, among all active trackers. It would be nice to always have
   // the process_id be a 64-bit value but the necessity of having it atomic
   // (for the memory barriers it provides) limits it to the natural word size
   // of the machine.
 #ifdef ARCH_CPU_64_BITS
   std::atomic<int64_t> process_id;
 #else
@@ skipping 60 matching lines @@
       sizeof(header_->thread_ref) == sizeof(header_->thread_ref.as_id),
       "PlatformThreadHandle::Handle is too big to hold in 64-bit ID");
 
   // Ensure that the alignment of Activity.data is properly aligned to a
   // 64-bit boundary so there are no interoperability-issues across cpu
   // architectures.
   static_assert(offsetof(Activity, data) % sizeof(uint64_t) == 0,
                 "ActivityData.data is not 64-bit aligned");
 
   // Provided memory should either be completely initialized or all zeros.
-  if (header_->cookie == 0) {
+  if (header_->cookie.load(std::memory_order_relaxed) == 0) {
     // This is a new file. Double-check other fields and then initialize.
     DCHECK_EQ(0, header_->process_id.load(std::memory_order_relaxed));
     DCHECK_EQ(0, header_->thread_ref.as_id);
     DCHECK_EQ(0, header_->start_time);
     DCHECK_EQ(0, header_->start_ticks);
     DCHECK_EQ(0U, header_->stack_slots);
     DCHECK_EQ(0U, header_->current_depth.load(std::memory_order_relaxed));
     DCHECK_EQ(0U, header_->stack_unchanged.load(std::memory_order_relaxed));
     DCHECK_EQ(0, stack_[0].time_internal);
     DCHECK_EQ(0U, stack_[0].origin_address);
     DCHECK_EQ(0U, stack_[0].call_stack[0]);
     DCHECK_EQ(0U, stack_[0].data.task.sequence_id);
 
 #if defined(OS_WIN)
     header_->thread_ref.as_tid = PlatformThread::CurrentId();
 #elif defined(OS_POSIX)
     header_->thread_ref.as_handle =
         PlatformThread::CurrentHandle().platform_handle();
 #endif
+    header_->process_id.store(GetCurrentProcId(), std::memory_order_relaxed);
+
     header_->start_time = base::Time::Now().ToInternalValue();
     header_->start_ticks = base::TimeTicks::Now().ToInternalValue();
     header_->stack_slots = stack_slots_;
     strlcpy(header_->thread_name, PlatformThread::GetName(),
             sizeof(header_->thread_name));
-    header_->cookie = kHeaderCookie;
 
     // This is done last so as to guarantee that everything above is "released"
     // by the time this value gets written.
-    header_->process_id.store(GetCurrentProcId(), std::memory_order_release);
+    header_->cookie.store(kHeaderCookie, std::memory_order_release);
 
     valid_ = true;
     DCHECK(IsValid());
   } else {
     // This is a file with existing data. Perform basic consistency checks.
     valid_ = true;
     valid_ = IsValid();
   }
 }
 
@@ skipping 73 matching lines @@
   // The stack has shrunk meaning that some other thread trying to copy the
   // contents for reporting purposes could get bad data. That thread would
   // have written a non-zero value into |stack_unchanged|; clearing it here
   // will let that thread detect that something did change. This needs to
   // happen after the atomic |depth| operation above so a "release" store
   // is required.
   header_->stack_unchanged.store(0, std::memory_order_release);
 }
 
 bool ThreadActivityTracker::IsValid() const {
-  if (header_->cookie != kHeaderCookie ||
+  if (header_->cookie.load(std::memory_order_acquire) != kHeaderCookie ||
       header_->process_id.load(std::memory_order_relaxed) == 0 ||
       header_->thread_ref.as_id == 0 ||
       header_->start_time == 0 ||
       header_->start_ticks == 0 ||
       header_->stack_slots != stack_slots_ ||
       header_->thread_name[sizeof(header_->thread_name) - 1] != '\0') {
     return false;
   }
 
   return valid_;
@@ skipping 449 matching lines @@
     const base::Process* process)
     : GlobalActivityTracker::ScopedThreadActivity(
           nullptr,
           Activity::ACT_PROCESS_WAIT,
           ActivityData::ForProcess(process->Pid()),
           /*lock_allowed=*/true) {}
 #endif
 
 }  // namespace debug
 }  // namespace base