Modify tracked_objects::DeathData to use atomics.

This eliminates races on the individual members of DeathData.
However, snapshotting of DeathData can still be inconsistent
since fields can be written and read from different fields.
Based on the comments in tracked_objects.{cc,h}, this appears
to be intentional.

BUG=447468

Review URL: https://codereview.chromium.org/1263773008

Cr-Commit-Position: refs/heads/master@{#369075}

2 files changed, 54 insertions, 36 deletions

diff --git a/base/tracked_objects.cc b/base/tracked_objects.cc
index 6ae7d46..0632edf 100644
--- a/base/tracked_objects.cc
+++ b/base/tracked_objects.cc
@@ -139,20 +139,24 @@ void DeathData::RecordDeath(const int32_t queue_duration,
                             const uint32_t random_number) {
   // We'll just clamp at INT_MAX, but we should note this in the UI as such.
   if (count_ < INT_MAX)
-    ++count_;
+    base::subtle::NoBarrier_Store(&count_, count_ + 1);
 
-  int sample_probability_count = sample_probability_count_;
+  int sample_probability_count =
+      base::subtle::NoBarrier_Load(&sample_probability_count_);
   if (sample_probability_count < INT_MAX)
     ++sample_probability_count;
-  sample_probability_count_ = sample_probability_count;
+  base::subtle::NoBarrier_Store(&sample_probability_count_,
+                                sample_probability_count);
 
-  queue_duration_sum_ += queue_duration;
-  run_duration_sum_ += run_duration;
+  base::subtle::NoBarrier_Store(&queue_duration_sum_,
+                                queue_duration_sum_ + queue_duration);
+  base::subtle::NoBarrier_Store(&run_duration_sum_,
+                                run_duration_sum_ + run_duration);
 
-  if (queue_duration_max_ < queue_duration)
-    queue_duration_max_ = queue_duration;
-  if (run_duration_max_ < run_duration)
-    run_duration_max_ = run_duration;
+  if (queue_duration_max() < queue_duration)
+    base::subtle::NoBarrier_Store(&queue_duration_max_, queue_duration);
+  if (run_duration_max() < run_duration)
+    base::subtle::NoBarrier_Store(&run_duration_max_, run_duration);
 
   // Take a uniformly distributed sample over all durations ever supplied during
   // the current profiling phase.
@@ -164,17 +168,17 @@ void DeathData::RecordDeath(const int32_t queue_duration,
   // used them to generate random_number).
   CHECK_GT(sample_probability_count, 0);
   if (0 == (random_number % sample_probability_count)) {
-    queue_duration_sample_ = queue_duration;
-    run_duration_sample_ = run_duration;
+    base::subtle::NoBarrier_Store(&queue_duration_sample_, queue_duration);
+    base::subtle::NoBarrier_Store(&run_duration_sample_, run_duration);
   }
 }
 
 void DeathData::OnProfilingPhaseCompleted(int profiling_phase) {
   // Snapshotting and storing current state.
   last_phase_snapshot_ = new DeathDataPhaseSnapshot(
-      profiling_phase, count_, run_duration_sum_, run_duration_max_,
-      run_duration_sample_, queue_duration_sum_, queue_duration_max_,
-      queue_duration_sample_, last_phase_snapshot_);
+      profiling_phase, count(), run_duration_sum(), run_duration_max(),
+      run_duration_sample(), queue_duration_sum(), queue_duration_max(),
+      queue_duration_sample(), last_phase_snapshot_);
 
   // Not touching fields for which a delta can be computed by comparing with a
   // snapshot from the previous phase. Resetting other fields.  Sample values
@@ -193,15 +197,17 @@ void DeathData::OnProfilingPhaseCompleted(int profiling_phase) {
   // resets.
   // sample_probability_count_ is incrementable, but must be reset to 0 at the
   // phase end, so that we start a new uniformly randomized sample selection
-  // after the reset.  Corruptions due to race conditions are possible, but the
-  // damage is limited to selecting a wrong sample, which is not something that
-  // can cause accumulating or cascading effects.
-  // If there were no corruptions caused by race conditions, we never send a
+  // after the reset. These fields are updated using atomics. However, race
+  // conditions are possible since these are updated individually and not
+  // together atomically, resulting in the values being mutually inconsistent.
+  // The damage is limited to selecting a wrong sample, which is not something
+  // that can cause accumulating or cascading effects.
+  // If there were no inconsistencies caused by race conditions, we never send a
   // sample for the previous phase in the next phase's snapshot because
   // ThreadData::SnapshotExecutedTasks doesn't send deltas with 0 count.
-  sample_probability_count_ = 0;
-  run_duration_max_ = 0;
-  queue_duration_max_ = 0;
+  base::subtle::NoBarrier_Store(&sample_probability_count_, 0);
+  base::subtle::NoBarrier_Store(&run_duration_max_, 0);
+  base::subtle::NoBarrier_Store(&queue_duration_max_, 0);
 }
 
 //------------------------------------------------------------------------------
diff --git a/base/tracked_objects.h b/base/tracked_objects.h
index ef43fe7..1a00ec0 100644
--- a/base/tracked_objects.h
+++ b/base/tracked_objects.h
@@ -333,13 +333,25 @@ class BASE_EXPORT DeathData {
 
   // Metrics and past snapshots accessors, used only for serialization and in
   // tests.
-  int count() const { return count_; }
-  int32_t run_duration_sum() const { return run_duration_sum_; }
-  int32_t run_duration_max() const { return run_duration_max_; }
-  int32_t run_duration_sample() const { return run_duration_sample_; }
-  int32_t queue_duration_sum() const { return queue_duration_sum_; }
-  int32_t queue_duration_max() const { return queue_duration_max_; }
-  int32_t queue_duration_sample() const { return queue_duration_sample_; }
+  int count() const { return base::subtle::NoBarrier_Load(&count_); }
+  int32_t run_duration_sum() const {
+    return base::subtle::NoBarrier_Load(&run_duration_sum_);
+  }
+  int32_t run_duration_max() const {
+    return base::subtle::NoBarrier_Load(&run_duration_max_);
+  }
+  int32_t run_duration_sample() const {
+    return base::subtle::NoBarrier_Load(&run_duration_sample_);
+  }
+  int32_t queue_duration_sum() const {
+    return base::subtle::NoBarrier_Load(&queue_duration_sum_);
+  }
+  int32_t queue_duration_max() const {
+    return base::subtle::NoBarrier_Load(&queue_duration_max_);
+  }
+  int32_t queue_duration_sample() const {
+    return base::subtle::NoBarrier_Load(&queue_duration_sample_);
+  }
   const DeathDataPhaseSnapshot* last_phase_snapshot() const {
     return last_phase_snapshot_;
   }
@@ -354,28 +366,28 @@ class BASE_EXPORT DeathData {
   // frequently used.  This might help a bit with cache lines.
   // Number of runs seen (divisor for calculating averages).
   // Can be incremented only on the death thread.
-  int count_;
+  base::subtle::Atomic32 count_;
 
   // Count used in determining probability of selecting exec/queue times from a
   // recorded death as samples.
   // Gets incremented only on the death thread, but can be set to 0 by
   // OnProfilingPhaseCompleted() on the snapshot thread.
-  int sample_probability_count_;
+  base::subtle::Atomic32 sample_probability_count_;
 
   // Basic tallies, used to compute averages.  Can be incremented only on the
   // death thread.
-  int32_t run_duration_sum_;
-  int32_t queue_duration_sum_;
+  base::subtle::Atomic32 run_duration_sum_;
+  base::subtle::Atomic32 queue_duration_sum_;
   // Max values, used by local visualization routines.  These are often read,
   // but rarely updated.  The max values get assigned only on the death thread,
   // but these fields can be set to 0 by OnProfilingPhaseCompleted() on the
   // snapshot thread.
-  int32_t run_duration_max_;
-  int32_t queue_duration_max_;
+  base::subtle::Atomic32 run_duration_max_;
+  base::subtle::Atomic32 queue_duration_max_;
   // Samples, used by crowd sourcing gatherers.  These are almost never read,
   // and rarely updated.  They can be modified only on the death thread.
-  int32_t run_duration_sample_;
-  int32_t queue_duration_sample_;
+  base::subtle::Atomic32 run_duration_sample_;
+  base::subtle::Atomic32 queue_duration_sample_;
 
   // Snapshot of this death data made at the last profiling phase completion, if
   // any.  DeathData owns the whole list starting with this pointer.