From f36c1094710bd51f643fb4bc84d5de4bfc5d11f3 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Mon, 2 Oct 2023 17:57:33 +0000 Subject: [PATCH] [vm, profiler] Don't assume the isolate has a mutator during a profile sample. A sample can be taken when Thread::Current() and Thread::Current()->isolate() are non-NULL. When a thread is entering or exiting an isolate, the is a brief window between the TLS being set/cleared and Isolate::mutator_thread_ being set/cleared. TEST=vm/cc/Profiler_EnterExitIsolate Bug: https://github.com/flutter/flutter/issues/134548 Change-Id: I7a6b25dabf930e8480d1a85d7f24c4765fc36b15 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328380 Commit-Queue: Ryan Macnak Reviewed-by: Ben Konyi --- runtime/vm/profiler.cc | 10 +++++++--- runtime/vm/profiler_test.cc | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc index 42d3ea69427..08c0367b5ff 100644 --- a/runtime/vm/profiler.cc +++ b/runtime/vm/profiler.cc @@ -820,9 +820,13 @@ Sample* SampleBlockBuffer::ReserveSampleImpl(Isolate* isolate, } if (block != nullptr) { block->MarkCompleted(); - if (!Isolate::IsSystemIsolate(isolate) && - isolate->TrySetHasCompletedBlocks()) { - isolate->mutator_thread()->ScheduleInterrupts(Thread::kVMInterrupt); + if (!Isolate::IsSystemIsolate(isolate)) { + Thread* mutator = isolate->mutator_thread(); + // The mutator thread might be NULL if we sample in the middle of + // Thread::Enter/ExitIsolate. + if ((mutator != nullptr) && isolate->TrySetHasCompletedBlocks()) { + mutator->ScheduleInterrupts(Thread::kVMInterrupt); + } } } return next->ReserveSample(); diff --git a/runtime/vm/profiler_test.cc b/runtime/vm/profiler_test.cc index 77d9b62eeb1..74fbfee00f0 100644 --- a/runtime/vm/profiler_test.cc +++ b/runtime/vm/profiler_test.cc @@ -2430,6 +2430,23 @@ ISOLATE_UNIT_TEST_CASE(Profiler_ProfileCodeTableTest) { EXPECT_EQ(table->FindCodeForPC(50), code1); } +// Try to hit any races in related to setting TLS and Isolate::mutator_thread_. +// https://github.com/flutter/flutter/issues/134548 +ISOLATE_UNIT_TEST_CASE(Profiler_EnterExitIsolate) { + EnableProfiler(); + Profiler::SetSamplePeriod(50); // Microseconds. + + const char* kScript = "main() => null;\n"; + const Library& root_library = Library::Handle(LoadTestScript(kScript)); + + Isolate* isolate = Isolate::Current(); + for (intptr_t i = 0; i < 100000; i++) { + Thread::ExitIsolate(); + Thread::EnterIsolate(isolate); + Invoke(root_library, "main"); + } +} + #endif // !PRODUCT } // namespace dart