Protect TraceLog::enabled_state_observer_list_ with a dedicated lock

base/trace_event/trace_log.h

 // Copyright 2015 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.
 
 #ifndef BASE_TRACE_EVENT_TRACE_LOG_H_
 #define BASE_TRACE_EVENT_TRACE_LOG_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 89 matching lines @@
   void AddClockSyncMetadataEvent();
 #endif
 
   // Enabled state listeners give a callback when tracing is enabled or
   // disabled. This can be used to tie into other library's tracing systems
   // on-demand.
   class BASE_EXPORT EnabledStateObserver {
    public:
     virtual ~EnabledStateObserver() = default;
 
-    // Called just after the tracing system becomes enabled, outside of the
-    // |lock_|. TraceLog::IsEnabled() is true at this point.
+    // Called just after the tracing system becomes enabled, outside of |lock_|
+    // but inside of |observer_list_lock_|. TraceLog::IsEnabled() is true at
+    // this point. Any subclass MUST NOT access |enabled_state_observer_list_|
+    // in the callback (e.g., via any |TraceLog::FooEnabledStateObserver()|),
+    // which would otherwise cause a deadlock.
     virtual void OnTraceLogEnabled() = 0;
 
-    // Called just after the tracing system disables, outside of the |lock_|.
-    // TraceLog::IsEnabled() is false at this point.
+    // Called just after the tracing system disables, outside of |lock_| but
+    // inside of |observer_list_lock_|. TraceLog::IsEnabled() is false at this
+    // point. Any subclass MUST NOT access |enabled_state_observer_list_| in the
+    // callback (e.g., via any |TraceLog::FooEnabledStateObserver()|), which
+    // would otherwise cause a deadlock.
     virtual void OnTraceLogDisabled() = 0;
   };
   void AddEnabledStateObserver(EnabledStateObserver* listener);
   void RemoveEnabledStateObserver(EnabledStateObserver* listener);
   bool HasEnabledStateObserver(EnabledStateObserver* listener) const;
 
   TraceLogStatus GetStatus() const;
   bool BufferIsFull() const;
 
   // Computes an estimate of the size of the TraceLog including all the retained
@@ skipping 297 matching lines @@
   // trace option as an AtomicWord.
   static const InternalTraceOptions kInternalNone;
   static const InternalTraceOptions kInternalRecordUntilFull;
   static const InternalTraceOptions kInternalRecordContinuously;
   static const InternalTraceOptions kInternalEchoToConsole;
   static const InternalTraceOptions kInternalEnableSampling;
   static const InternalTraceOptions kInternalRecordAsMuchAsPossible;
   static const InternalTraceOptions kInternalEnableArgumentFilter;
 
   // This lock protects TraceLog member accesses (except for members protected
-  // by thread_info_lock_) from arbitrary threads.
+  // by thread_info_lock_ or observer_list_lock_) from arbitrary threads.
   mutable Lock lock_;
   // This lock protects accesses to thread_names_, thread_event_start_times_
   // and thread_colors_.
   Lock thread_info_lock_;
   Mode mode_;
   int num_traces_recorded_;
   std::unique_ptr<TraceBuffer> logged_events_;
   std::vector<std::unique_ptr<TraceEvent>> metadata_events_;
   subtle::AtomicWord /* EventCallback */ event_callback_;
+
+  // This lock protects accesses to enabled_state_observer_list_.
+  mutable Lock observer_list_lock_;
   bool dispatching_to_observer_list_;
+  // TODO(xiaochengh): Replace these raw pointers by weak pointers and direct
+  // callbacks by task posting, so that we can get rid of the current nasty
+  // and complicated handling of threading issues.
   std::vector<EnabledStateObserver*> enabled_state_observer_list_;
 
   std::string process_name_;
   base::hash_map<int, std::string> process_labels_;
   int process_sort_index_;
   base::hash_map<int, int> thread_sort_indices_;
   base::hash_map<int, std::string> thread_names_;
 
   // The following two maps are used only when ECHO_TO_CONSOLE.
   base::hash_map<int, std::stack<TimeTicks>> thread_event_start_times_;
@@ skipping 43 matching lines @@
   subtle::AtomicWord generation_;
   bool use_worker_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(TraceLog);
 };
 
 }  // namespace trace_event
 }  // namespace base
 
 #endif  // BASE_TRACE_EVENT_TRACE_LOG_H_