Skip to content

Commit ce600d0

Browse files
alexmarkovCommit Queue
authored and
Commit Queue
committed
[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 <rmacnak@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
1 parent 0c41286 commit ce600d0

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

runtime/vm/thread.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ void Thread::AssertEmptyStackInvariants() {
304304
void Thread::AssertEmptyThreadInvariants() {
305305
AssertEmptyStackInvariants();
306306

307-
ASSERT(top_ == 0);
307+
ASSERT(top() == 0);
308308
ASSERT(end_ == 0);
309309
ASSERT(true_end_ == 0);
310310
ASSERT(isolate_ == nullptr);

runtime/vm/thread.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -720,10 +720,10 @@ class Thread : public ThreadState {
720720
// TLAB and end() is the chosen sampling boundary for the thread.
721721
//
722722
// When the heap sampling profiler is disabled, true_end() == end().
723-
uword top() const { return top_; }
723+
uword top() const { return top_.load(std::memory_order_relaxed); }
724724
uword end() const { return end_; }
725725
uword true_end() const { return true_end_; }
726-
void set_top(uword top) { top_ = top; }
726+
void set_top(uword top) { top_.store(top, std::memory_order_relaxed); }
727727
void set_end(uword end) { end_ = end; }
728728
void set_true_end(uword true_end) { true_end_ = true_end; }
729729
static intptr_t top_offset() { return OFFSET_OF(Thread, top_); }
@@ -1267,7 +1267,7 @@ class Thread : public ThreadState {
12671267
#if defined(DART_COMPRESSED_POINTERS)
12681268
uword heap_base_ = 0;
12691269
#endif
1270-
uword top_ = 0;
1270+
std::atomic<uword> top_ = 0;
12711271
uword end_ = 0;
12721272
const uword* dispatch_table_array_ = nullptr;
12731273
ObjectPtr* field_table_values_ = nullptr;

0 commit comments

Comments
 (0)