From f37db8487fbd5d6aa4e84a5b4a9936928041b67e Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Wed, 12 Feb 2025 14:48:28 -0800 Subject: [PATCH] [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 https://github.com/dart-lang/sdk/issues/44089. TEST=ci Change-Id: I8ad87c340580325cbd6e22b5f068e1e33b0a7d46 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409460 Reviewed-by: Ryan Macnak Commit-Queue: Alexander Markov --- runtime/vm/profiler.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc index 12785ab02c1..c9467c62d52 100644 --- a/runtime/vm/profiler.cc +++ b/runtime/vm/profiler.cc @@ -1361,6 +1361,12 @@ void Profiler::SampleThread(Thread* thread, ASSERT(os_thread != nullptr); Isolate* isolate = thread->isolate(); + // Double check if interrupts are disabled + // after the thread interrupter decided to send a signal. + if (!os_thread->ThreadInterruptsEnabled()) { + return; + } + // Thread is not doing VM work. if (thread->task_kind() == Thread::kUnknownTask) { counters_.bail_out_unknown_task.fetch_add(1);