[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>
diff --git a/runtime/vm/os_thread.cc b/runtime/vm/os_thread.cc
index e101536..e04dc4a 100644
--- a/runtime/vm/os_thread.cc
+++ b/runtime/vm/os_thread.cc
@@ -27,7 +27,9 @@
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)
diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc
index f70b6b4..43a17b0 100644
--- a/runtime/vm/timeline.cc
+++ b/runtime/vm/timeline.cc
@@ -1072,8 +1072,10 @@
// 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 @@
}
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 @@
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)));
}
diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h
index 962f935..edbc125 100644
--- a/runtime/vm/timeline.h
+++ b/runtime/vm/timeline.h
@@ -848,7 +848,9 @@
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
diff --git a/runtime/vm/timeline_test.cc b/runtime/vm/timeline_test.cc
index ee3ff18..3ada2b2 100644
--- a/runtime/vm/timeline_test.cc
+++ b/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 @@
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)
