diff --git a/runtime/vm/os_thread.cc b/runtime/vm/os_thread.cc index 35bfdb7535b..1bb18a168c5 100644 --- a/runtime/vm/os_thread.cc +++ b/runtime/vm/os_thread.cc @@ -25,6 +25,7 @@ thread_local ThreadState* OSThread::current_vm_thread_ = NULL; #if defined(SUPPORT_TIMELINE) inline void UpdateTimelineTrackMetadata(const OSThread& thread) { RecorderShutdownSynchronizationLockScope ls; + MutexLocker ml(Timeline::recorder_lock()); TimelineEventRecorder* recorder = Timeline::recorder(); if (recorder != nullptr && !ls.IsShuttingDown()) { recorder->AddTrackMetadataBasedOnThread( diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc index 533bd825231..beb295c1bae 100644 --- a/runtime/vm/timeline.cc +++ b/runtime/vm/timeline.cc @@ -235,7 +235,19 @@ static bool HasStream(MallocGrowableArray* streams, const char* stream) { void Timeline::Init() { RecorderShutdownSynchronizationLock::Init(); ASSERT(recorder_ == NULL); - recorder_ = CreateTimelineRecorder(); + { + MutexLocker ml(Timeline::recorder_lock()); + recorder_ = CreateTimelineRecorder(); + } + // The following is needed to backfill information about any |OSThread|s that + // were initialized before this point. + OSThreadIterator it; + while (it.HasNext()) { + OSThread& thread = *it.Next(); + recorder_->AddTrackMetadataBasedOnThread( + OS::ProcessId(), OSThread::ThreadIdToIntPtr(thread.trace_id()), + thread.name()); + } if (FLAG_trace_timeline) { OS::PrintErr("Using the %s timeline recorder.\n", recorder_->name()); } @@ -438,6 +450,7 @@ void TimelineEventArguments::Free() { length_ = 0; } +Mutex* Timeline::recorder_lock_ = new Mutex(); TimelineEventRecorder* Timeline::recorder_ = NULL; Dart_TimelineRecorderCallback Timeline::callback_ = NULL; MallocGrowableArray* Timeline::enabled_streams_ = NULL; @@ -1071,17 +1084,7 @@ TimelineEventRecorder::TimelineEventRecorder() track_uuid_to_track_metadata_( &SimpleHashMap::SamePointerValue, TimelineEventRecorder::kTrackUuidToTrackMetadataInitialCapacity), - track_uuid_to_track_metadata_lock_() { - // The following is needed to backfill information about any |OSThread|s that - // were initialized before this point. - OSThreadIterator it; - while (it.HasNext()) { - OSThread& thread = *it.Next(); - AddTrackMetadataBasedOnThread(OS::ProcessId(), - OSThread::ThreadIdToIntPtr(thread.trace_id()), - thread.name()); - } -} + track_uuid_to_track_metadata_lock_() {} TimelineEventRecorder::~TimelineEventRecorder() { for (SimpleHashMap::Entry* entry = track_uuid_to_track_metadata_.Start(); diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h index 58555a20050..a1708623f05 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -193,6 +193,10 @@ class Timeline : public AllStatic { // Cleanup timeline system. Not thread safe. static void Cleanup(); + // Used to prevent races between reading / modifying the value of + // |Timeline::recorder_|. + static Mutex* recorder_lock() { return recorder_lock_; } + // Access the global recorder. Not thread safe. static TimelineEventRecorder* recorder() { return recorder_; } @@ -237,6 +241,12 @@ class Timeline : public AllStatic { static void ClearUnsafe(); static void ReclaimCachedBlocksFromThreadsUnsafe(); + // |recorder_lock_| is used in |OSThread|'s constructor, so it must not be + // destroyed until |OSThread|s can no longer be created. Therefore, there is + // currently no opportunity to delete this lock. If a thread only begins to + // run after we have started to run TLS destructors for a call to |exit()|, + // there will be a race involving this lock's deletion. + static Mutex* recorder_lock_; static TimelineEventRecorder* recorder_; static Dart_TimelineRecorderCallback callback_; static MallocGrowableArray* enabled_streams_;