From ce600d0f50432b2452dedd6674e878d670da24ab Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Tue, 11 Feb 2025 09:39:20 -0800 Subject: [PATCH] [vm] Fix TSAN data race when accessing Thread::top_ This is a possible fix for the following benign TSAN error: WARNING: ThreadSanitizer: data race (pid=40436) Read of size 8 at 0x7b7c000bfa48 by thread T6 (mutexes: write M0): #0 dart::Page::object_end() const out/ReleaseTSANX64/../../runtime/vm/heap/page.h (dart+0x25ade83) #1 dart::Page::used() const out/ReleaseTSANX64/../../runtime/vm/heap/page.h:107:34 (dart+0x25ade83) #2 dart::SemiSpace::used_in_words() const out/ReleaseTSANX64/../../runtime/vm/heap/scavenger.h:46:18 (dart+0x25ade83) #3 dart::Scavenger::UsedInWords() const out/ReleaseTSANX64/../../runtime/vm/heap/scavenger.h:157:17 (dart+0x25ade83) #4 dart::GetProcessMemoryUsageHelper(dart::JSONStream*)::$_0::operator()(dart::IsolateGroup*) const out/ReleaseTSANX64/../../runtime/vm/service.cc:4729:50 (dart+0x25ade83) Previous write of size 8 at 0x7b7c000bfa48 by thread T9: #0 dart::Thread::set_top(unsigned long) out/ReleaseTSANX64/../../runtime/vm/thread.h:698:34 (dart+0x2622071) #1 dart::Scavenger::TryAllocateFromTLAB(dart::Thread*, long) out/ReleaseTSANX64/../../runtime/vm/heap/scavenger.h:256:13 (dart+0x2622071) #2 dart::Scavenger::TryAllocate(dart::Thread*, long) out/ReleaseTSANX64/../../runtime/vm/heap/scavenger.h:139:12 (dart+0x2622071) #3 dart::Heap::AllocateNew(dart::Thread*, long) out/ReleaseTSANX64/../../runtime/vm/heap/heap.cc:84:27 (dart+0x2622071) #4 dart::Heap::Allocate(dart::Thread*, long, dart::Heap::Space) out/ReleaseTSANX64/../../runtime/vm/heap/heap.h:87:16 (dart+0x2416cab) TEST=ci Change-Id: Id9077cff2c1143adf999b2b26c941701d63cd844 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409180 Reviewed-by: Ryan Macnak Commit-Queue: Alexander Markov --- runtime/vm/thread.cc | 2 +- runtime/vm/thread.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc index a0d88117ddb..ee99e7d4b96 100644 --- a/runtime/vm/thread.cc +++ b/runtime/vm/thread.cc @@ -304,7 +304,7 @@ void Thread::AssertEmptyStackInvariants() { void Thread::AssertEmptyThreadInvariants() { AssertEmptyStackInvariants(); - ASSERT(top_ == 0); + ASSERT(top() == 0); ASSERT(end_ == 0); ASSERT(true_end_ == 0); ASSERT(isolate_ == nullptr); diff --git a/runtime/vm/thread.h b/runtime/vm/thread.h index 86c0cc48c4a..45e86fff909 100644 --- a/runtime/vm/thread.h +++ b/runtime/vm/thread.h @@ -720,10 +720,10 @@ class Thread : public ThreadState { // TLAB and end() is the chosen sampling boundary for the thread. // // When the heap sampling profiler is disabled, true_end() == end(). - uword top() const { return top_; } + uword top() const { return top_.load(std::memory_order_relaxed); } uword end() const { return end_; } uword true_end() const { return true_end_; } - void set_top(uword top) { top_ = top; } + void set_top(uword top) { top_.store(top, std::memory_order_relaxed); } void set_end(uword end) { end_ = end; } void set_true_end(uword true_end) { true_end_ = true_end; } static intptr_t top_offset() { return OFFSET_OF(Thread, top_); } @@ -1267,7 +1267,7 @@ class Thread : public ThreadState { #if defined(DART_COMPRESSED_POINTERS) uword heap_base_ = 0; #endif - uword top_ = 0; + std::atomic top_ = 0; uword end_ = 0; const uword* dispatch_table_array_ = nullptr; ObjectPtr* field_table_values_ = nullptr;