[VM/Timeline] Add test that makes TSAN check the track metadata map

TEST=CI

Change-Id: I9e4fa4bbc08e77a1365e84f2d048384d7f0b1350
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281720
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>

runtime/vm/os_thread.cc

@@ -27,7 +27,9 @@ inline void UpdateTimelineTrackMetadata(const OSThread& thread) {
   RecorderLockScope rl;
   TimelineEventRecorder* recorder = Timeline::recorder();
   if (recorder != nullptr && !rl.IsShuttingDown()) {
-    recorder->AddTrackMetadataBasedOnThread(thread);
+    recorder->AddTrackMetadataBasedOnThread(
+        OS::ProcessId(), OSThread::ThreadIdToIntPtr(thread.trace_id()),
+        thread.name());
   }
 }
 #endif  // defined(SUPPORT_TIMELINE)

runtime/vm/timeline.cc

@@ -1072,8 +1072,10 @@ TimelineEventRecorder::TimelineEventRecorder()
   // were initialized before this point.
   OSThreadIterator it;
   while (it.HasNext()) {
-    OSThread* thread = it.Next();
-    AddTrackMetadataBasedOnThread(*thread);
+    OSThread& thread = *it.Next();
+    AddTrackMetadataBasedOnThread(OS::ProcessId(),
+                                  OSThread::ThreadIdToIntPtr(thread.trace_id()),
+                                  thread.name());
   }
 }
 
@@ -1246,7 +1248,9 @@ TimelineEventBlock* TimelineEventRecorder::GetNewBlock() {
 }
 
 void TimelineEventRecorder::AddTrackMetadataBasedOnThread(
-    const OSThread& thread) {
+    const intptr_t process_id,
+    const intptr_t trace_id,
+    const char* thread_name) {
   if (FLAG_timeline_recorder != nullptr &&
       // There is no way to retrieve track metadata when a callback or systrace
       // recorder is in use, so we don't need to update the map in these cases.
@@ -1254,23 +1258,19 @@ void TimelineEventRecorder::AddTrackMetadataBasedOnThread(
       strcmp("systrace", FLAG_timeline_recorder) != 0) {
     MutexLocker ml(&track_uuid_to_track_metadata_lock_);
 
-    intptr_t pid = OS::ProcessId();
-    intptr_t tid = OSThread::ThreadIdToIntPtr(thread.trace_id());
-    const char* thread_name = thread.name();
-
-    void* key = reinterpret_cast<void*>(tid);
-    const intptr_t hash = Utils::WordHash(tid);
+    void* key = reinterpret_cast<void*>(trace_id);
+    const intptr_t hash = Utils::WordHash(trace_id);
     SimpleHashMap::Entry* entry =
         track_uuid_to_track_metadata_.Lookup(key, hash, true);
     if (entry->value == nullptr) {
       entry->value = new TimelineTrackMetadata(
-          pid, tid,
+          process_id, trace_id,
           Utils::CreateCStringUniquePtr(
               Utils::StrDup(thread_name == nullptr ? "" : thread_name)));
     } else {
       TimelineTrackMetadata* value =
           static_cast<TimelineTrackMetadata*>(entry->value);
-      ASSERT(pid == value->pid());
+      ASSERT(process_id == value->pid());
       value->set_track_name(Utils::CreateCStringUniquePtr(
           Utils::StrDup(thread_name == nullptr ? "" : thread_name)));
     }

runtime/vm/timeline.h

@@ -848,7 +848,9 @@ class TimelineEventRecorder : public MallocAllocated {
   void FinishBlock(TimelineEventBlock* block);
   // This function must be called at least once for each thread that corresponds
   // to a track in the trace.
-  void AddTrackMetadataBasedOnThread(const OSThread& thread);
+  void AddTrackMetadataBasedOnThread(const intptr_t process_id,
+                                     const intptr_t trace_id,
+                                     const char* thread_name);
 
  protected:
 #ifndef PRODUCT

runtime/vm/timeline_test.cc

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 #include <cstring>
+#include <memory>
 
 #include "platform/assert.h"
 
@@ -373,6 +374,55 @@ TEST_CASE(TimelineRingRecorderJSONOrder) {
   EXPECT(alpha < beta);
 }
 
+TEST_CASE(TimelineTrackMetadataRace) {
+  struct ReportMetadataArguments {
+    TimelineEventRingRecorder* recorder;
+    ThreadJoinId join_id = OSThread::kInvalidThreadJoinId;
+  };
+
+  TimelineEventRingRecorder recorder;
+  const intptr_t fake_process_id = 1;
+  const intptr_t fake_trace_id = 1;
+
+  // Try concurrently reading from / writing to the metadata map. I don't think
+  // it's possible to assert anything about the outcome, because of scheduling
+  // uncertainty. This test is just used to ensure that TSAN checks the metadata
+  // map code.
+  JSONStream js;
+  TimelineEventFilter filter;
+  const ReportMetadataArguments report_metadata_1_arguments{&recorder};
+  const ReportMetadataArguments report_metadata_2_arguments{&recorder};
+  OSThread::Start(
+      "ReportMetadata1",
+      [](uword arguments_ptr) {
+        ReportMetadataArguments& arguments =
+            *reinterpret_cast<ReportMetadataArguments*>(arguments_ptr);
+        arguments.recorder->AddTrackMetadataBasedOnThread(
+            fake_process_id, fake_trace_id, "Thread 1");
+        arguments.join_id =
+            OSThread::GetCurrentThreadJoinId(OSThread::Current());
+      },
+      reinterpret_cast<uword>(&report_metadata_1_arguments));
+  OSThread::Start(
+      "ReportMetadata2",
+      [](uword arguments_ptr) {
+        ReportMetadataArguments& arguments =
+            *reinterpret_cast<ReportMetadataArguments*>(arguments_ptr);
+        arguments.recorder->AddTrackMetadataBasedOnThread(
+            fake_process_id, fake_trace_id, "Incorrect Name");
+        arguments.join_id =
+            OSThread::GetCurrentThreadJoinId(OSThread::Current());
+      },
+      reinterpret_cast<uword>(&report_metadata_2_arguments));
+  recorder.PrintJSON(&js, &filter);
+  while (report_metadata_1_arguments.join_id == OSThread::kInvalidThreadId ||
+         report_metadata_2_arguments.join_id == OSThread::kInvalidThreadId) {
+    // Spin until the join IDs have been set.
+  }
+  OSThread::Join(report_metadata_1_arguments.join_id);
+  OSThread::Join(report_metadata_2_arguments.join_id);
+}
+
 #endif  // !PRODUCT
 
 #if defined(SUPPORT_TIMELINE)