From 8649806076e72f1afb5afe4b267ec0561314118f Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Mon, 6 Apr 2026 10:35:42 -0700 Subject: [PATCH] [vm/profiler/gardening] Fix TSAN failures in profiler. Use relaxed atomics for sample fields. Fix lock grabbing ordering. BUG=https://github.com/dart-lang/sdk/issues/62873 TEST=ci Change-Id: I9c4ae0c78d5b81a72dc8a9e4802f08fd57beb27a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493161 Reviewed-by: Ryan Macnak Commit-Queue: Alexander Aprelev --- runtime/vm/profiler.cc | 19 +++++++++++-------- runtime/vm/profiler.h | 28 ++++++++++++++-------------- runtime/vm/timeline.cc | 19 +++++++++++-------- runtime/vm/timeline.h | 2 +- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc index 3b9e26d02e7..38ed932111c 100644 --- a/runtime/vm/profiler.cc +++ b/runtime/vm/profiler.cc @@ -1003,7 +1003,9 @@ class ReturnAddressLocator : public ValueObject { ASSERT(code_.ContainsInstructionAt(pc())); } - ReturnAddressLocator(uword pc, uword* stack_buffer, const Code& code) + ReturnAddressLocator(uword pc, + RelaxedAtomic* stack_buffer, + const Code& code) : stack_buffer_(stack_buffer), pc_(pc), code_(Code::ZoneHandle(code.ptr())) { @@ -1037,7 +1039,7 @@ class ReturnAddressLocator : public ValueObject { } private: - uword* stack_buffer_; + RelaxedAtomic* stack_buffer_; uword pc_; const Code& code_; }; @@ -1282,7 +1284,7 @@ class ProfilerDartStackWalker : public ProfilerStackWalker { static void CopyStackBuffer(Sample* sample, uword sp_addr) { ASSERT(sample != nullptr); uword* sp = reinterpret_cast(sp_addr); - uword* buffer = sample->GetStackBuffer(); + RelaxedAtomic* buffer = sample->GetStackBuffer(); if (sp != nullptr) { for (intptr_t i = 0; i < Sample::kStackBufferSizeInWords; i++) { buffer[i] = reinterpret_cast(LoadStackSlot(sp)); @@ -2200,7 +2202,7 @@ ProcessedSample::ProcessedSample() void ProcessedSample::FixupCaller(const CodeLookupTable& clt, uword pc_marker, - uword* stack_buffer) { + RelaxedAtomic* stack_buffer) { const CodeDescriptor* cd = clt.FindCode(At(0)); if (cd == nullptr) { // No Dart code. @@ -2213,10 +2215,11 @@ void ProcessedSample::FixupCaller(const CodeLookupTable& clt, CheckForMissingDartFrame(clt, cd, pc_marker, stack_buffer); } -void ProcessedSample::CheckForMissingDartFrame(const CodeLookupTable& clt, - const CodeDescriptor* cd, - uword pc_marker, - uword* stack_buffer) { +void ProcessedSample::CheckForMissingDartFrame( + const CodeLookupTable& clt, + const CodeDescriptor* cd, + uword pc_marker, + RelaxedAtomic* stack_buffer) { ASSERT(cd != nullptr); if (cd->code().IsBytecode()) { // Bytecode frame build is atomic from the profiler's perspective, diff --git a/runtime/vm/profiler.h b/runtime/vm/profiler.h index 96c8bff742f..89411504d84 100644 --- a/runtime/vm/profiler.h +++ b/runtime/vm/profiler.h @@ -428,22 +428,22 @@ class Sample { } static constexpr int kPCArraySizeInWords = 32; - uword* GetPCArray() { return &pc_array_[0]; } + RelaxedAtomic* GetPCArray() { return &pc_array_[0]; } static constexpr int kStackBufferSizeInWords = 2; - uword* GetStackBuffer() { return &stack_buffer_[0]; } + RelaxedAtomic* GetStackBuffer() { return &stack_buffer_[0]; } private: - int64_t timestamp_; - Dart_Port port_; - ThreadId tid_; - uword stack_buffer_[kStackBufferSizeInWords]; - uword pc_array_[kPCArraySizeInWords]; - uword vm_tag_; - uword user_tag_; - Sample* next_; - uint32_t state_; - uint32_t allocation_identity_hash_; + RelaxedAtomic timestamp_; + RelaxedAtomic port_; + RelaxedAtomic tid_; + RelaxedAtomic stack_buffer_[kStackBufferSizeInWords]; + RelaxedAtomic pc_array_[kPCArraySizeInWords]; + RelaxedAtomic vm_tag_; + RelaxedAtomic user_tag_; + RelaxedAtomic next_; + RelaxedAtomic state_; + RelaxedAtomic allocation_identity_hash_; using HeadSampleBit = BitField; using LeafFrameIsDart = @@ -944,12 +944,12 @@ class ProcessedSample : public ZoneObject { private: void FixupCaller(const CodeLookupTable& clt, uword pc_marker, - uword* stack_buffer); + RelaxedAtomic* stack_buffer); void CheckForMissingDartFrame(const CodeLookupTable& clt, const CodeDescriptor* code, uword pc_marker, - uword* stack_buffer); + RelaxedAtomic* stack_buffer); ZoneGrowableArray pcs_; int64_t timestamp_; diff --git a/runtime/vm/timeline.cc b/runtime/vm/timeline.cc index 35cd2fbfe9b..20c5757ac77 100644 --- a/runtime/vm/timeline.cc +++ b/runtime/vm/timeline.cc @@ -377,7 +377,7 @@ void Timeline::Cleanup() { } } -void Timeline::ReclaimCachedBlocksFromThreads() { +void Timeline::ReclaimCachedBlocksFromThreads(OSThreadIterator* it) { RecorderSynchronizationLockScope ls; TimelineEventRecorder* recorder = Timeline::recorder(); if (recorder == nullptr || ls.IsUninitialized()) { @@ -385,9 +385,8 @@ void Timeline::ReclaimCachedBlocksFromThreads() { } ASSERT(recorder != nullptr); // Iterate over threads. - OSThreadIterator it; - while (it.HasNext()) { - OSThread* thread = it.Next(); + while (it->HasNext()) { + OSThread* thread = it->Next(); MutexLocker ml(thread->timeline_block_lock()); // Grab block and clear it. TimelineEventBlock* block = thread->TimelineBlockLocked(); @@ -443,8 +442,9 @@ void Timeline::Clear() { ASSERT(recorder != nullptr); // Acquire the recorder's lock to prevent the reclaimed blocks from being // handed out again until they have been cleared. + OSThreadIterator it; MutexLocker ml(&recorder->lock_); - ReclaimCachedBlocksFromThreads(); + ReclaimCachedBlocksFromThreads(&it); recorder->ClearLocked(); } @@ -1546,8 +1546,9 @@ void TimelineEventRecorder::WriteTo(const char* directory) { // Acquire the recorder's lock to prevent the reclaimed blocks from being // handed out again until the trace has been serialized. + OSThreadIterator it; MutexLocker ml(&lock_); - Timeline::ReclaimCachedBlocksFromThreads(); + Timeline::ReclaimCachedBlocksFromThreads(&it); intptr_t pid = OS::ProcessId(); char* filename = @@ -1732,8 +1733,9 @@ void TimelineEventFixedBufferRecorder::ForEachNonEmptyBlock( std::function&& handle_block) { // Acquire the recorder's lock to prevent the reclaimed blocks from being // handed out again until the trace has been serialized. + OSThreadIterator it; MutexLocker ml(&lock_); - Timeline::ReclaimCachedBlocksFromThreads(); + Timeline::ReclaimCachedBlocksFromThreads(&it); ResetTimeTracking(); intptr_t block_offset = FindOldestBlockIndexLocked(); if (block_offset == -1) { @@ -2590,8 +2592,9 @@ void TimelineEventEndlessRecorder::ForEachNonEmptyBlock( std::function&& handle_block) { // Acquire the recorder's lock to prevent the reclaimed blocks from being // handed out again until the trace has been serialized. + OSThreadIterator it; MutexLocker ml(&lock_); - Timeline::ReclaimCachedBlocksFromThreads(); + Timeline::ReclaimCachedBlocksFromThreads(&it); ResetTimeTracking(); for (TimelineEventBlock* current = head_; current != nullptr; current = current->next()) { diff --git a/runtime/vm/timeline.h b/runtime/vm/timeline.h index 5f23416d177..757a3530cbc 100644 --- a/runtime/vm/timeline.h +++ b/runtime/vm/timeline.h @@ -281,7 +281,7 @@ class Timeline : public AllStatic { } // Reclaim all |TimelineEventBlocks|s that are cached by threads. - static void ReclaimCachedBlocksFromThreads(); + static void ReclaimCachedBlocksFromThreads(OSThreadIterator* it); static void Clear();