commit f36c1094710bd51f643fb4bc84d5de4bfc5d11f3
author Ryan Macnak <rmacnak@google.com> Mon Oct 02 17:57:33 2023 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Oct 02 17:57:33 2023 +0000
tree ec4e2b2920c1e2589a705990fdba93ab163db605
parent ebd669a0d4d98fd4695771fdad4aedcb34709b36

[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 <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>

runtime/vm/profiler.cc

diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc
index 42d3ea6..08c0367 100644
--- a/runtime/vm/profiler.cc
+++ b/runtime/vm/profiler.cc
@@ -820,9 +820,13 @@
   }
   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();

runtime/vm/profiler_test.cc

diff --git a/runtime/vm/profiler_test.cc b/runtime/vm/profiler_test.cc
index 77d9b62..74fbfee 100644
--- a/runtime/vm/profiler_test.cc
+++ b/runtime/vm/profiler_test.cc
@@ -2430,6 +2430,23 @@
   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