Protect TraceLog::enabled_state_observer_list_ with a dedicated lock

base/trace_event/trace_log.cc

Index: base/trace_event/trace_log.cc
diff --git a/base/trace_event/trace_log.cc b/base/trace_event/trace_log.cc
index 1a375d146a9eb20582e7948677029c93c6a1842f..aca5d0ff5e9f2adec6be896057acefc8a3aa3499 100644
--- a/base/trace_event/trace_log.cc
+++ b/base/trace_event/trace_log.cc
@@ -569,78 +569,76 @@ void TraceLog::GetKnownCategoryGroups(
 }
 
 void TraceLog::SetEnabled(const TraceConfig& trace_config, Mode mode) {
-  std::vector<EnabledStateObserver*> observer_list;
-  {
-    AutoLock lock(lock_);
-
-    // Can't enable tracing when Flush() is in progress.
-    DCHECK(!flush_task_runner_);
+  AutoLock lock(lock_);
 
-    InternalTraceOptions new_options =
-        GetInternalOptionsFromTraceConfig(trace_config);
+  // Can't enable tracing when Flush() is in progress.
+  DCHECK(!flush_task_runner_);
 
-    InternalTraceOptions old_options = trace_options();
+  InternalTraceOptions new_options =
+      GetInternalOptionsFromTraceConfig(trace_config);
 
-    if (IsEnabled()) {
-      if (new_options != old_options) {
-        DLOG(ERROR) << "Attempting to re-enable tracing with a different "
-                    << "set of options.";
-      }
+  InternalTraceOptions old_options = trace_options();
 
-      if (mode != mode_) {
-        DLOG(ERROR) << "Attempting to re-enable tracing with a different mode.";
-      }
-
-      trace_config_.Merge(trace_config);
-      UpdateCategoryGroupEnabledFlags();
-      return;
+  if (IsEnabled()) {
+    if (new_options != old_options) {
+      DLOG(ERROR) << "Attempting to re-enable tracing with a different "
+                  << "set of options.";
     }
 
-    if (dispatching_to_observer_list_) {
-      DLOG(ERROR)
-          << "Cannot manipulate TraceLog::Enabled state from an observer.";
-      return;
+    if (mode != mode_) {
+      DLOG(ERROR) << "Attempting to re-enable tracing with a different mode.";
     }
 
-    mode_ = mode;
+    trace_config_.Merge(trace_config);
+    UpdateCategoryGroupEnabledFlags();
+    return;
+  }
 
-    if (new_options != old_options) {
-      subtle::NoBarrier_Store(&trace_options_, new_options);
-      UseNextTraceBuffer();
-    }
+  if (dispatching_to_observer_list_) {
+    DLOG(ERROR)
+        << "Cannot manipulate TraceLog::Enabled state from an observer.";
+    return;
+  }
 
-    num_traces_recorded_++;
+  mode_ = mode;
 
-    trace_config_ = TraceConfig(trace_config);
-    UpdateCategoryGroupEnabledFlags();
-    UpdateSyntheticDelaysFromTraceConfig();
-
-    if (new_options & kInternalEnableSampling) {
-      sampling_thread_.reset(new TraceSamplingThread);
-      sampling_thread_->RegisterSampleBucket(
-          &g_trace_state[0], "bucket0",
-          Bind(&TraceSamplingThread::DefaultSamplingCallback));
-      sampling_thread_->RegisterSampleBucket(
-          &g_trace_state[1], "bucket1",
-          Bind(&TraceSamplingThread::DefaultSamplingCallback));
-      sampling_thread_->RegisterSampleBucket(
-          &g_trace_state[2], "bucket2",
-          Bind(&TraceSamplingThread::DefaultSamplingCallback));
-      if (!PlatformThread::Create(0, sampling_thread_.get(),
-                                  &sampling_thread_handle_)) {
-        DCHECK(false) << "failed to create thread";
-      }
-    }
+  if (new_options != old_options) {
+    subtle::NoBarrier_Store(&trace_options_, new_options);
+    UseNextTraceBuffer();
+  }
 
-    dispatching_to_observer_list_ = true;
-    observer_list = enabled_state_observer_list_;
+  num_traces_recorded_++;
+
+  trace_config_ = TraceConfig(trace_config);
+  UpdateCategoryGroupEnabledFlags();
+  UpdateSyntheticDelaysFromTraceConfig();
+
+  if (new_options & kInternalEnableSampling) {
+    sampling_thread_.reset(new TraceSamplingThread);
+    sampling_thread_->RegisterSampleBucket(
+        &g_trace_state[0], "bucket0",
+        Bind(&TraceSamplingThread::DefaultSamplingCallback));
+    sampling_thread_->RegisterSampleBucket(
+        &g_trace_state[1], "bucket1",
+        Bind(&TraceSamplingThread::DefaultSamplingCallback));
+    sampling_thread_->RegisterSampleBucket(
+        &g_trace_state[2], "bucket2",
+        Bind(&TraceSamplingThread::DefaultSamplingCallback));
+    if (!PlatformThread::Create(0, sampling_thread_.get(),
+                                &sampling_thread_handle_)) {
+      DCHECK(false) << "failed to create thread";
+    }
   }
-  // Notify observers outside the lock in case they trigger trace events.
-  for (size_t i = 0; i < observer_list.size(); ++i)
-    observer_list[i]->OnTraceLogEnabled();
 
   {
-    AutoLock lock(lock_);
+    AutoLock observer_list_lock(observer_list_lock_);
+    dispatching_to_observer_list_ = true;
+    {
+      // Notify observers outside the lock in case they trigger trace events.
+      AutoUnlock unlock(lock_);
+      for (size_t i = 0; i < enabled_state_observer_list_.size(); ++i)
+        enabled_state_observer_list_[i]->OnTraceLogEnabled();
+    }
     dispatching_to_observer_list_ = false;
   }
 }
@@ -716,18 +714,18 @@ void TraceLog::SetDisabledWhileLocked() {
   // Remove metadata events so they will not get added to a subsequent trace.
   metadata_events_.clear();
 
-  dispatching_to_observer_list_ = true;
-  std::vector<EnabledStateObserver*> observer_list =
-      enabled_state_observer_list_;
-
   {
-    // Dispatch to observers outside the lock in case the observer triggers a
-    // trace event.
-    AutoUnlock unlock(lock_);
-    for (size_t i = 0; i < observer_list.size(); ++i)
-      observer_list[i]->OnTraceLogDisabled();
+    AutoLock observer_list_lock(observer_list_lock_);
+    dispatching_to_observer_list_ = true;
+    {
+      // Dispatch to observers outside the lock in case the observer triggers a
+      // trace event.
+      AutoUnlock unlock(lock_);
+      for (size_t i = 0; i < enabled_state_observer_list_.size(); ++i)
+        enabled_state_observer_list_[i]->OnTraceLogDisabled();
+    }
+    dispatching_to_observer_list_ = false;
   }
-  dispatching_to_observer_list_ = false;
 }
 
 int TraceLog::GetNumTracesRecorded() {
@@ -738,12 +736,12 @@ int TraceLog::GetNumTracesRecorded() {
 }
 
 void TraceLog::AddEnabledStateObserver(EnabledStateObserver* listener) {
-  AutoLock lock(lock_);
+  AutoLock observer_list_lock(observer_list_lock_);
   enabled_state_observer_list_.push_back(listener);
 }
 
 void TraceLog::RemoveEnabledStateObserver(EnabledStateObserver* listener) {
-  AutoLock lock(lock_);
+  AutoLock observer_list_lock(observer_list_lock_);
   std::vector<EnabledStateObserver*>::iterator it =
       std::find(enabled_state_observer_list_.begin(),
                 enabled_state_observer_list_.end(), listener);
@@ -752,7 +750,7 @@ void TraceLog::RemoveEnabledStateObserver(EnabledStateObserver* listener) {
 }
 
 bool TraceLog::HasEnabledStateObserver(EnabledStateObserver* listener) const {
-  AutoLock lock(lock_);
+  AutoLock observer_list_lock(observer_list_lock_);
   return ContainsValue(enabled_state_observer_list_, listener);
 }