Skip to content

Commit f37db84

Browse files
alexmarkovCommit Queue
authored and
Commit Queue
committed
[vm] Double check if thread interrupts are enabled after thread is interrupted
Thread interrupts can be disabled after thread interrupter decided to interrupt a thread but before thread actually receives and handles a signal. This may cause a data race accessing a Sample between profiler which initializes and adds a Sample and the main thread using Profiler::sample_block_buffer() in the profiler unit tests. This is a possible fix for the following benign TSAN error: WARNING: ThreadSanitizer: data race (pid=5824) Write of size 4 at 0x7f53e8e53a98 by thread T6: #0 dart::Sample::set_thread_task(dart::Thread::TaskKind) out/ReleaseTSANX64/../../runtime/vm/profiler.h:325:12 (run_vm_tests+0x292cab2) #1 dart::SetupSample(dart::Thread*, bool, unsigned long) out/ReleaseTSANX64/../../runtime/vm/profiler.cc:1226:11 (run_vm_tests+0x292cab2) #2 dart::Profiler::SampleThread(dart::Thread*, dart::InterruptedThreadState const&) out/ReleaseTSANX64/../../runtime/vm/profiler.cc:1381:7 (run_vm_tests+0x292cab2) #3 dart::ThreadInterrupterLinux::ThreadInterruptSignalHandler(int, siginfo_t*, void*) out/ReleaseTSANX64/../../runtime/vm/thread_interrupter_linux.cc:44:5 (run_vm_tests+0x2a064b0) #4 __tsan::CallUserSignalHandler(__tsan::ThreadState*, bool, bool, int, __sanitizer::__sanitizer_siginfo*, void*) ../staging/llvm_build/tools/clang/stage2-bins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:2122:5 (run_vm_tests+0x1fc057f) #5 dart::Mutex::Unlock() out/ReleaseTSANX64/../../runtime/vm/os_thread_linux.cc:367:16 (run_vm_tests+0x2924b89) Previous read of size 4 at 0x7f53e8e53a98 by main thread: #0 dart::Sample::head_sample() const out/ReleaseTSANX64/../../runtime/vm/profiler.h:350:59 (run_vm_tests+0x22c70e7) #1 dart::SampleBuffer::VisitSamples(dart::SampleVisitor*) out/ReleaseTSANX64/../../runtime/vm/profiler.h:589:20 (run_vm_tests+0x22c70e7) #2 dart::SampleBlockBuffer::VisitSamples(dart::SampleVisitor*) out/ReleaseTSANX64/../../runtime/vm/profiler.h:752:18 (run_vm_tests+0x22c70e7) #3 dart::Dart_TestHelperProfiler_GetSourceReport(dart::Thread*) out/ReleaseTSANX64/../../runtime/vm/profiler_test.cc:2241:26 (run_vm_tests+0x22c70e7) #4 dart::Dart_TestProfiler_GetSourceReport() out/ReleaseTSANX64/../../runtime/vm/profiler_test.cc:2206:1 (run_vm_tests+0x22c70e7) #5 dart::TestCase::Run() out/ReleaseTSANX64/../../runtime/bin/run_vm_tests.cc:53:3 (run_vm_tests+0x203fe6c) --- Re-run this test: python3 tools/test.py -n vm-tsan-linux-release-x64 vm/cc/Profiler_GetSourceReport Also, this change is a possible fix for #44089. TEST=ci Change-Id: I8ad87c340580325cbd6e22b5f068e1e33b0a7d46 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409460 Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
1 parent 6d37abb commit f37db84

File tree

1 file changed

+6
-0
lines changed

1 file changed

+6
-0
lines changed

runtime/vm/profiler.cc

+6
Original file line numberDiff line numberDiff line change
@@ -1361,6 +1361,12 @@ void Profiler::SampleThread(Thread* thread,
13611361
ASSERT(os_thread != nullptr);
13621362
Isolate* isolate = thread->isolate();
13631363

1364+
// Double check if interrupts are disabled
1365+
// after the thread interrupter decided to send a signal.
1366+
if (!os_thread->ThreadInterruptsEnabled()) {
1367+
return;
1368+
}
1369+
13641370
// Thread is not doing VM work.
13651371
if (thread->task_kind() == Thread::kUnknownTask) {
13661372
counters_.bail_out_unknown_task.fetch_add(1);

0 commit comments

Comments
 (0)