[VM/Timeline] Change locking scheme used by TimelineEventRingRecorder to prevent races
TEST=Loaded the timeline in DevTools on a debug build with the recorder
set to ring, endless, and startup. Recorded traces on a debug build with
the recorder set to systrace, file, and perfettofile. Ran the
vm/cc/TimelineRingRecorderRace test on ASAN and TSAN.
Issue: https://github.com/flutter/flutter/issues/129258
Change-Id: I39920ac3e4a70e5714915fcd554ae171ed82504a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312721
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 6ee3aee..3536a12 100644
--- a/runtime/vm/os_thread.cc
+++ b/runtime/vm/os_thread.cc
@@ -96,6 +96,11 @@
log_ = nullptr;
#if defined(SUPPORT_TIMELINE)
if (Timeline::recorder() != nullptr) {
+ // Acquire the recorder's lock so that |timeline_block_| cannot be given to
+ // another thread until the call to |TimelineEventRecorder::FinishBlock| is
+ // complete.
+ MutexLocker recorder_lock_locker(&Timeline::recorder()->lock_);
+ MutexLocker timeline_block_lock_locker(timeline_block_lock());
Timeline::recorder()->FinishBlock(timeline_block_);
}
#endif
diff --git a/runtime/vm/os_thread.h b/runtime/vm/os_thread.h
index 2eea84b..07297ae 100644
--- a/runtime/vm/os_thread.h
+++ b/runtime/vm/os_thread.h
@@ -60,6 +60,7 @@
friend class SafepointMutexLocker;
friend class OSThreadIterator;
friend class TimelineEventRecorder;
+ friend class TimelineEventRingRecorder;
friend class PageSpace;
friend void Dart_TestMutex();
DISALLOW_COPY_AND_ASSIGN(Mutex);
@@ -111,10 +112,14 @@
Mutex* timeline_block_lock() const { return &timeline_block_lock_; }
// Only safe to access when holding |timeline_block_lock_|.
- TimelineEventBlock* timeline_block() const { return timeline_block_; }
+ TimelineEventBlock* TimelineBlockLocked() const {
+ ASSERT(timeline_block_lock()->IsOwnedByCurrentThread());
+ return timeline_block_;
+ }
// Only safe to access when holding |timeline_block_lock_|.
- void set_timeline_block(TimelineEventBlock* block) {
+ void SetTimelineBlockLocked(TimelineEventBlock* block) {
+ ASSERT(timeline_block_lock()->IsOwnedByCurrentThread());
timeline_block_ = block;
}
@@ -284,6 +289,8 @@
char* name_; // A name for this thread.
mutable Mutex timeline_block_lock_;
+ // The block that the timeline recorder has permitted this thread to write
+ // events to.
TimelineEventBlock* timeline_block_;
// All |Thread|s are registered in the thread list.
diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc
index a9c212a..aeefc65 100644
--- a/runtime/vm/timeline.cc
+++ b/runtime/vm/timeline.cc
@@ -94,12 +94,11 @@
// busy if blocks are being reclaimed by the reporting system.
//
// Reporting:
-// When requested, the timeline is serialized in the trace-event format
-// (https://goo.gl/hDZw5M). The request can be for a VM-wide timeline or an
-// isolate specific timeline. In both cases it may be that a thread has
-// a |TimelineEventBlock| cached in TLS partially filled with events. In order
-// to report a complete timeline the cached |TimelineEventBlock|s need to be
-// reclaimed.
+// When requested, the timeline is serialized in either Chrome's JSON trace
+// format (https://goo.gl/hDZw5M) or Perfetto's proto trace format. In both
+// cases, it may be that a thread has a |TimelineEventBlock| cached in TLS
+// partially filled with events. In order to report a complete timeline, the
+// cached |TimelineEventBlock|s need to be reclaimed.
//
// Reclaiming open |TimelineEventBlock|s from threads:
//
@@ -108,6 +107,9 @@
// To reclaim blocks, we iterate over all threads and remove the cached
// |TimelineEventBlock| from each thread. This is safe because we hold the
// |Thread|'s |timeline_block_lock_| meaning the block can't be being modified.
+// When clearing the reclaimed blocks, or serializing the events in them, we
+// hold |TimelineEventRecorder::lock_| before reclaiming the blocks, to prevent
+// reclaimed blocks from being handed out again until we release it.
//
// Locking notes:
// The following locks are used by the timeline system:
@@ -120,8 +122,8 @@
//
// Locks must always be taken in the following order:
// |Thread::thread_list_lock_|
-// |Thread::timeline_block_lock_|
-// |TimelineEventRecorder::lock_|
+// |TimelineEventRecorder::lock_|
+// |Thread::timeline_block_lock_|
//
std::atomic<RecorderSynchronizationLock::RecorderState>
@@ -338,11 +340,8 @@
OSThread* thread = it.Next();
MutexLocker ml(thread->timeline_block_lock());
// Grab block and clear it.
- TimelineEventBlock* block = thread->timeline_block();
- thread->set_timeline_block(nullptr);
- // TODO(johnmccutchan): Consider dropping the timeline_block_lock here
- // if we can do it everywhere. This would simplify the lock ordering
- // requirements.
+ TimelineEventBlock* block = thread->TimelineBlockLocked();
+ thread->SetTimelineBlockLocked(nullptr);
recorder->FinishBlock(block);
}
}
@@ -392,8 +391,11 @@
return;
}
ASSERT(recorder != nullptr);
+ // Acquire the recorder's lock to prevent the reclaimed blocks from being
+ // handed out again until they have been cleared.
+ MutexLocker ml(&recorder->lock_);
ReclaimCachedBlocksFromThreads();
- recorder->Clear();
+ recorder->ClearLocked();
}
void TimelineEventArguments::SetNumArguments(intptr_t length) {
@@ -1248,11 +1250,6 @@
return !InUseLocked() && !IsEmpty();
}
-ThreadId TimelineEventBlock::ThreadIdLocked() const {
- ASSERT(Timeline::recorder()->lock_.IsOwnedByCurrentThread());
- return thread_id_;
-}
-
TimelineEventFilter::TimelineEventFilter(int64_t time_origin_micros,
int64_t time_extent_micros)
: time_origin_micros_(time_origin_micros),
@@ -1360,6 +1357,11 @@
// Grab the current thread.
OSThread* thread = OSThread::Current();
ASSERT(thread != nullptr);
+ // Acquire the recorder lock in case we need to call |GetNewBlockLocked|. We
+ // acquire the lock here and not directly before calls to |GetNewBlockLocked|
+ // due to locking order restrictions.
+ Mutex& recorder_lock = lock_;
+ recorder_lock.Lock();
Mutex* thread_block_lock = thread->timeline_block_lock();
ASSERT(thread_block_lock != nullptr);
// We are accessing the thread's timeline block- so take the lock here.
@@ -1372,26 +1374,34 @@
}
#endif // defined(DEBUG)
- TimelineEventBlock* thread_block = thread->timeline_block();
+ TimelineEventBlock* thread_block = thread->TimelineBlockLocked();
if ((thread_block != nullptr) && thread_block->IsFull()) {
- MutexLocker ml(&lock_);
// Thread has a block and it is full:
// 1) Mark it as finished.
- thread_block->Finish();
+ thread->SetTimelineBlockLocked(nullptr);
+ FinishBlock(thread_block);
// 2) Allocate a new block.
+ // We release |thread_block_lock| before calling |GetNewBlockLocked| to
+ // avoid TSAN warnings about lock order inversion.
+ thread_block_lock->Unlock();
thread_block = GetNewBlockLocked();
- thread->set_timeline_block(thread_block);
+ thread_block_lock->Lock();
+ thread->SetTimelineBlockLocked(thread_block);
} else if (thread_block == nullptr) {
- MutexLocker ml(&lock_);
// Thread has no block. Attempt to allocate one.
+ // We release |thread_block_lock| before calling |GetNewBlockLocked| to
+ // avoid TSAN warnings about lock order inversion.
+ thread_block_lock->Unlock();
thread_block = GetNewBlockLocked();
- thread->set_timeline_block(thread_block);
+ thread_block_lock->Lock();
+ thread->SetTimelineBlockLocked(thread_block);
}
+ recorder_lock.Unlock();
if (thread_block != nullptr) {
// NOTE: We are exiting this function with the thread's block lock held.
ASSERT(!thread_block->IsFull());
- TimelineEvent* event = thread_block->StartEvent();
+ TimelineEvent* event = thread_block->StartEventLocked();
return event;
}
// Drop lock here as no event is being handed out.
@@ -1471,6 +1481,9 @@
return;
}
+ // Acquire the recorder's lock to prevent the reclaimed blocks from being
+ // handed out again until the trace has been serialized.
+ MutexLocker ml(&lock_);
Timeline::ReclaimCachedBlocksFromThreads();
intptr_t pid = OS::ProcessId();
@@ -1501,16 +1514,9 @@
#endif
void TimelineEventRecorder::FinishBlock(TimelineEventBlock* block) {
- if (block == nullptr) {
- return;
+ if (block != nullptr) {
+ block->Finish();
}
- MutexLocker ml(&lock_);
- block->Finish();
-}
-
-TimelineEventBlock* TimelineEventRecorder::GetNewBlock() {
- MutexLocker ml(&lock_);
- return GetNewBlockLocked();
}
void TimelineEventRecorder::AddTrackMetadataBasedOnThread(
@@ -1591,11 +1597,9 @@
}
TimelineEventFixedBufferRecorder::~TimelineEventFixedBufferRecorder() {
- MutexLocker ml(&lock_);
- // Delete all blocks.
- for (intptr_t i = 0; i < num_blocks_; i++) {
- blocks_[i].Reset();
- }
+ // We do not need to acquire any locks, because at this point we must have
+ // reclaimed all the blocks, and |RecorderSynchronizationLock| must have been
+ // put in a state that prevents blocks from being given out.
delete memory_;
}
@@ -1607,8 +1611,10 @@
void TimelineEventFixedBufferRecorder::PrintEventsCommon(
const TimelineEventFilter& filter,
std::function<void(const TimelineEvent&)>&& print_impl) {
- Timeline::ReclaimCachedBlocksFromThreads();
+ // Acquire the recorder's lock to prevent the reclaimed blocks from being
+ // handed out again until the trace has been serialized.
MutexLocker ml(&lock_);
+ Timeline::ReclaimCachedBlocksFromThreads();
ResetTimeTracking();
intptr_t block_offset = FindOldestBlockIndexLocked();
if (block_offset == -1) {
@@ -1759,11 +1765,12 @@
#endif // !defined(PRODUCT)
TimelineEventBlock* TimelineEventFixedBufferRecorder::GetHeadBlockLocked() {
+ ASSERT(lock_.IsOwnedByCurrentThread());
return &blocks_[0];
}
-void TimelineEventFixedBufferRecorder::Clear() {
- MutexLocker ml(&lock_);
+void TimelineEventFixedBufferRecorder::ClearLocked() {
+ ASSERT(lock_.IsOwnedByCurrentThread());
for (intptr_t i = 0; i < num_blocks_; i++) {
TimelineEventBlock* block = &blocks_[i];
block->Reset();
@@ -1800,18 +1807,25 @@
}
TimelineEventBlock* TimelineEventRingRecorder::GetNewBlockLocked() {
- // TODO(johnmccutchan): This function should only hand out blocks
- // which have been marked as finished.
+ ASSERT(lock_.IsOwnedByCurrentThread());
if (block_cursor_ == num_blocks_) {
block_cursor_ = 0;
}
TimelineEventBlock* block = &blocks_[block_cursor_++];
- block->Reset();
- block->Open();
+ if (block->current_owner_ != nullptr) {
+ MutexLocker ml(block->current_owner_->timeline_block_lock());
+ block->current_owner_->SetTimelineBlockLocked(nullptr);
+ block->Reset();
+ block->Open();
+ } else {
+ block->Reset();
+ block->Open();
+ }
return block;
}
TimelineEventBlock* TimelineEventStartupRecorder::GetNewBlockLocked() {
+ ASSERT(lock_.IsOwnedByCurrentThread());
if (block_cursor_ == num_blocks_) {
return nullptr;
}
@@ -2188,14 +2202,18 @@
TimelineEventEndlessRecorder::TimelineEventEndlessRecorder()
: head_(nullptr), tail_(nullptr), block_index_(0) {}
-TimelineEventEndlessRecorder::~TimelineEventEndlessRecorder() {}
+TimelineEventEndlessRecorder::~TimelineEventEndlessRecorder() {
+ ASSERT(head_ == nullptr);
+}
#ifndef PRODUCT
void TimelineEventEndlessRecorder::PrintEventsCommon(
const TimelineEventFilter& filter,
std::function<void(const TimelineEvent&)>&& print_impl) {
- Timeline::ReclaimCachedBlocksFromThreads();
+ // Acquire the recorder's lock to prevent the reclaimed blocks from being
+ // handed out again until the trace has been serialized.
MutexLocker ml(&lock_);
+ Timeline::ReclaimCachedBlocksFromThreads();
ResetTimeTracking();
for (TimelineEventBlock* current = head_; current != nullptr;
current = current->next()) {
@@ -2273,6 +2291,7 @@
#endif // !defined(PRODUCT)
TimelineEventBlock* TimelineEventEndlessRecorder::GetHeadBlockLocked() {
+ ASSERT(lock_.IsOwnedByCurrentThread());
return head_;
}
@@ -2288,6 +2307,7 @@
}
TimelineEventBlock* TimelineEventEndlessRecorder::GetNewBlockLocked() {
+ ASSERT(lock_.IsOwnedByCurrentThread());
TimelineEventBlock* block = new TimelineEventBlock(block_index_++);
block->Open();
if (head_ == nullptr) {
@@ -2302,8 +2322,8 @@
return block;
}
-void TimelineEventEndlessRecorder::Clear() {
- MutexLocker ml(&lock_);
+void TimelineEventEndlessRecorder::ClearLocked() {
+ ASSERT(lock_.IsOwnedByCurrentThread());
TimelineEventBlock* current = head_;
while (current != nullptr) {
TimelineEventBlock* next = current->next();
@@ -2319,7 +2339,7 @@
: next_(nullptr),
length_(0),
block_index_(block_index),
- thread_id_(OSThread::kInvalidThreadId),
+ current_owner_(nullptr),
in_use_(false) {}
TimelineEventBlock::~TimelineEventBlock() {
@@ -2339,11 +2359,13 @@
}
#endif
-TimelineEvent* TimelineEventBlock::StartEvent() {
+TimelineEvent* TimelineEventBlock::StartEventLocked() {
+ OSThread* os_thread = OSThread::Current();
+ ASSERT(os_thread != nullptr);
+ ASSERT(os_thread == current_owner_);
+ ASSERT(os_thread->timeline_block_lock()->IsOwnedByCurrentThread());
ASSERT(!IsFull());
if (FLAG_trace_timeline) {
- OSThread* os_thread = OSThread::Current();
- ASSERT(os_thread != nullptr);
intptr_t tid = OSThread::ThreadIdToIntPtr(os_thread->id());
OS::PrintErr("StartEvent in block %p for thread %" Pd "\n", this, tid);
}
@@ -2364,14 +2386,14 @@
events_[i].Reset();
}
length_ = 0;
- thread_id_ = OSThread::kInvalidThreadId;
+ current_owner_ = nullptr;
in_use_ = false;
}
void TimelineEventBlock::Open() {
OSThread* os_thread = OSThread::Current();
ASSERT(os_thread != nullptr);
- thread_id_ = os_thread->trace_id();
+ current_owner_ = os_thread;
in_use_ = true;
}
@@ -2379,6 +2401,7 @@
if (FLAG_trace_timeline) {
OS::PrintErr("Finish block %p\n", this);
}
+ current_owner_ = nullptr;
in_use_ = false;
#ifndef PRODUCT
if (Service::timeline_stream.enabled()) {
diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h
index 6c8711f..778d021 100644
--- a/runtime/vm/timeline.h
+++ b/runtime/vm/timeline.h
@@ -10,6 +10,7 @@
#include "include/dart_tools_api.h"
+#include "platform/assert.h"
#include "platform/atomic.h"
#include "platform/hashmap.h"
#include "vm/allocation.h"
@@ -815,27 +816,25 @@
// Only safe to access under the recorder's lock.
inline bool ContainsEventsThatCanBeSerializedLocked() const;
- // Only safe to access under the recorder's lock.
- inline ThreadId ThreadIdLocked() const;
-
protected:
#ifndef PRODUCT
void PrintJSON(JSONStream* stream) const;
#endif
- TimelineEvent* StartEvent();
+ // Only safe to call from the thread that owns this block, while the thread is
+ // holding its block lock.
+ TimelineEvent* StartEventLocked();
TimelineEvent events_[kBlockSize];
TimelineEventBlock* next_;
intptr_t length_;
intptr_t block_index_;
- // Only accessed under the recorder's lock.
- ThreadId thread_id_;
+ // Only safe to access under the recorder's lock.
+ OSThread* current_owner_;
bool in_use_;
void Open();
- void Finish();
friend class Thread;
friend class TimelineEventRecorder;
@@ -847,6 +846,8 @@
friend class JSONStream;
private:
+ void Finish();
+
DISALLOW_COPY_AND_ASSIGN(TimelineEventBlock);
};
@@ -907,7 +908,8 @@
#endif // !defined(PRODUCT)
virtual const char* name() const = 0;
virtual intptr_t Size() = 0;
- TimelineEventBlock* GetNewBlock();
+ // Only safe to call when holding |lock_|.
+ virtual TimelineEventBlock* GetNewBlockLocked() = 0;
void FinishBlock(TimelineEventBlock* block);
// This function must be called at least once for each thread that corresponds
// to a track in the trace.
@@ -936,9 +938,10 @@
// Interface method(s) which must be implemented.
virtual TimelineEvent* StartEvent() = 0;
virtual void CompleteEvent(TimelineEvent* event) = 0;
+ // Only safe to call when holding |lock_|.
virtual TimelineEventBlock* GetHeadBlockLocked() = 0;
- virtual TimelineEventBlock* GetNewBlockLocked() = 0;
- virtual void Clear() = 0;
+ // Only safe to call when holding |lock_|.
+ virtual void ClearLocked() = 0;
// Utility method(s).
#ifndef PRODUCT
@@ -968,6 +971,7 @@
friend class TimelineStream;
friend class TimelineTestHelper;
friend class Timeline;
+ friend class OSThread;
private:
static constexpr intptr_t kTrackUuidToTrackMetadataInitialCapacity = 1 << 4;
@@ -1010,7 +1014,7 @@
TimelineEventBlock* GetHeadBlockLocked();
// Only safe to call when holding |lock_|.
intptr_t FindOldestBlockIndexLocked() const;
- void Clear();
+ void ClearLocked();
#ifndef PRODUCT
void PrintJSONEvents(const JSONArray& array,
@@ -1089,9 +1093,9 @@
}
protected:
- TimelineEventBlock* GetNewBlockLocked() { return nullptr; }
- TimelineEventBlock* GetHeadBlockLocked() { return nullptr; }
- void Clear() {}
+ TimelineEventBlock* GetNewBlockLocked() { UNREACHABLE(); }
+ TimelineEventBlock* GetHeadBlockLocked() { UNREACHABLE(); }
+ void ClearLocked() { ASSERT(lock_.IsOwnedByCurrentThread()); }
TimelineEvent* StartEvent();
void CompleteEvent(TimelineEvent* event);
};
@@ -1141,7 +1145,7 @@
void CompleteEvent(TimelineEvent* event);
TimelineEventBlock* GetNewBlockLocked();
TimelineEventBlock* GetHeadBlockLocked();
- void Clear();
+ void ClearLocked();
#ifndef PRODUCT
void PrintJSONEvents(const JSONArray& array,
@@ -1190,9 +1194,9 @@
virtual const char* name() const = 0;
protected:
- TimelineEventBlock* GetNewBlockLocked() { return nullptr; }
- TimelineEventBlock* GetHeadBlockLocked() { return nullptr; }
- void Clear() {}
+ TimelineEventBlock* GetNewBlockLocked() { UNREACHABLE(); }
+ TimelineEventBlock* GetHeadBlockLocked() { UNREACHABLE(); }
+ void ClearLocked() { ASSERT(lock_.IsOwnedByCurrentThread()); }
TimelineEvent* StartEvent();
void CompleteEvent(TimelineEvent* event);
};
diff --git a/runtime/vm/timeline_test.cc b/runtime/vm/timeline_test.cc
index 8d768ff..cce7fbb 100644
--- a/runtime/vm/timeline_test.cc
+++ b/runtime/vm/timeline_test.cc
@@ -47,11 +47,17 @@
event->StreamInit(stream);
}
+ static Mutex& GetRecorderLock(TimelineEventRecorder& recorder) {
+ return recorder.lock_;
+ }
+
static void FakeThreadEvent(TimelineEventBlock* block,
intptr_t ftid,
const char* label = "fake",
TimelineStream* stream = nullptr) {
- TimelineEvent* event = block->StartEvent();
+ OSThread& current_thread = *OSThread::Current();
+ MutexLocker ml(current_thread.timeline_block_lock());
+ TimelineEvent* event = block->StartEventLocked();
ASSERT(event != nullptr);
event->DurationBegin(label);
event->thread_ = OSThread::ThreadIdFromIntPtr(ftid);
@@ -60,10 +66,6 @@
}
}
- static void SetBlockThread(TimelineEventBlock* block, intptr_t ftid) {
- block->thread_id_ = OSThread::ThreadIdFromIntPtr(ftid);
- }
-
static void FakeDuration(TimelineEventRecorder* recorder,
const char* label,
int64_t start,
@@ -74,7 +76,7 @@
TimelineEvent* event = recorder->StartEvent();
ASSERT(event != nullptr);
event->Duration(label, start, end);
- event->Complete();
+ recorder->CompleteEvent(event);
}
static void FakeBegin(TimelineEventRecorder* recorder,
@@ -86,7 +88,7 @@
TimelineEvent* event = recorder->StartEvent();
ASSERT(event != nullptr);
event->Begin(label, /*id=*/-1, start);
- event->Complete();
+ recorder->CompleteEvent(event);
}
static void FakeEnd(TimelineEventRecorder* recorder,
@@ -98,7 +100,7 @@
TimelineEvent* event = recorder->StartEvent();
ASSERT(event != nullptr);
event->End(label, end);
- event->Complete();
+ recorder->CompleteEvent(event);
}
static void FinishBlock(TimelineEventBlock* block) { block->Finish(); }
@@ -346,21 +348,25 @@
new TimelineEventRingRecorder(TimelineEventBlock::kBlockSize * 2);
TimelineRecorderOverride<TimelineEventRingRecorder> override(recorder);
- TimelineEventBlock* block_0 = Timeline::recorder()->GetNewBlock();
- EXPECT(block_0 != nullptr);
- TimelineEventBlock* block_1 = Timeline::recorder()->GetNewBlock();
- EXPECT(block_1 != nullptr);
- // Test that we wrapped.
- EXPECT(block_0 == Timeline::recorder()->GetNewBlock());
+ {
+ Mutex& recorder_lock = TimelineTestHelper::GetRecorderLock(*recorder);
+ MutexLocker ml(&recorder_lock);
+ TimelineEventBlock* block_0 = Timeline::recorder()->GetNewBlockLocked();
+ EXPECT(block_0 != nullptr);
+ TimelineEventBlock* block_1 = Timeline::recorder()->GetNewBlockLocked();
+ EXPECT(block_1 != nullptr);
+ // Test that we wrapped.
+ EXPECT(block_0 == Timeline::recorder()->GetNewBlockLocked());
- // Emit the earlier event into block_1.
- TimelineTestHelper::FakeThreadEvent(block_1, 2, "Alpha", &stream);
- OS::Sleep(32);
- // Emit the later event into block_0.
- TimelineTestHelper::FakeThreadEvent(block_0, 2, "Beta", &stream);
+ // Emit the earlier event into block_1.
+ TimelineTestHelper::FakeThreadEvent(block_1, 2, "Alpha", &stream);
+ OS::Sleep(32);
+ // Emit the later event into block_0.
+ TimelineTestHelper::FakeThreadEvent(block_0, 2, "Beta", &stream);
- TimelineTestHelper::FinishBlock(block_0);
- TimelineTestHelper::FinishBlock(block_1);
+ TimelineTestHelper::FinishBlock(block_0);
+ TimelineTestHelper::FinishBlock(block_1);
+ }
JSONStream js;
TimelineEventFilter filter;
@@ -374,6 +380,72 @@
EXPECT(alpha < beta);
}
+TEST_CASE(TimelineRingRecorderRace) {
+ struct ReportEventsArguments {
+ Monitor& synchronization_monitor;
+ TimelineEventRecorder& recorder;
+ ThreadJoinId join_id = OSThread::kInvalidThreadJoinId;
+ };
+
+ // Note that |recorder| will be freed by |TimelineRecorderOverride|'s
+ // destructor.
+ TimelineEventRingRecorder& recorder =
+ *(new TimelineEventRingRecorder(2 * TimelineEventBlock::kBlockSize));
+ TimelineRecorderOverride<TimelineEventRingRecorder> override(&recorder);
+ Monitor synchronization_monitor;
+ JSONStream js;
+ TimelineEventFilter filter;
+ ReportEventsArguments report_events_1_arguments{synchronization_monitor,
+ recorder};
+ ReportEventsArguments report_events_2_arguments{synchronization_monitor,
+ recorder};
+
+ // Try concurrently writing events, serializing them, and clearing the
+ // timeline. It is not possible to assert anything about the outcome, because
+ // of scheduling uncertainty. This test is just used to ensure that TSAN
+ // checks the ring recorder code.
+ OSThread::Start(
+ "ReportEvents1",
+ [](uword arguments_ptr) {
+ ReportEventsArguments& arguments =
+ *reinterpret_cast<ReportEventsArguments*>(arguments_ptr);
+ for (intptr_t i = 0; i < 2 * TimelineEventBlock::kBlockSize; ++i) {
+ TimelineTestHelper::FakeDuration(&arguments.recorder, "testEvent",
+ /*start=*/0, /*end=*/1);
+ }
+ MonitorLocker ml(&arguments.synchronization_monitor);
+ arguments.join_id =
+ OSThread::GetCurrentThreadJoinId(OSThread::Current());
+ ml.Notify();
+ },
+ reinterpret_cast<uword>(&report_events_1_arguments));
+ OSThread::Start(
+ "ReportEvents2",
+ [](uword arguments_ptr) {
+ ReportEventsArguments& arguments =
+ *reinterpret_cast<ReportEventsArguments*>(arguments_ptr);
+ for (intptr_t i = 0; i < 2 * TimelineEventBlock::kBlockSize; ++i) {
+ TimelineTestHelper::FakeDuration(&arguments.recorder, "testEvent",
+ /*start=*/0, /*end=*/1);
+ }
+ MonitorLocker ml(&arguments.synchronization_monitor);
+ arguments.join_id =
+ OSThread::GetCurrentThreadJoinId(OSThread::Current());
+ ml.Notify();
+ },
+ reinterpret_cast<uword>(&report_events_2_arguments));
+ Timeline::Clear();
+ recorder.PrintJSON(&js, &filter);
+
+ MonitorLocker ml(&synchronization_monitor);
+ while (report_events_1_arguments.join_id == OSThread::kInvalidThreadJoinId ||
+ report_events_2_arguments.join_id == OSThread::kInvalidThreadJoinId) {
+ ml.Wait();
+ }
+ OSThread::Join(report_events_1_arguments.join_id);
+ OSThread::Join(report_events_2_arguments.join_id);
+}
+
// |OSThread::Start()| takes in a function pointer, and only lambdas that don't
// capture can be converted to function pointers. So, we use these macros to
// avoid needing to capture.
