Reapply "[vm/shared] Ensure to iterate over mutators, rather than only isolates during deopt."

This reverts commit ce9c2d3 and as patchset 2 applies the fix to crash revealed on mac bots.

TEST=isolate_group_bound_lazy_deopt_test

Change-Id: I8aa05a482954dfbd4599d68637287f8b93716ce2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448641
Commit-Queue: Alexander Aprelev <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>

Author: aam

runtime/vm/compiler/runtime_offsets_extracted.h

@@ -11,6 +11,7 @@
 #include "vm/object.h"
 #include "vm/runtime_entry.h"
 #include "vm/stack_frame.h"
+#include "vm/thread_registry.h"
 
 namespace dart {
 
@@ -55,28 +56,22 @@ void WeakCodeReferences::DisableCode(bool are_mutators_stopped) {
   auto isolate_group = IsolateGroup::Current();
   auto disable_code_fun = [&]() {
     Code& code = Code::Handle();
-    isolate_group->ForEachIsolate(
-        [&](Isolate* isolate) {
-          auto mutator_thread = isolate->mutator_thread();
-          if (mutator_thread == nullptr) {
-            return;
+    isolate_group->ForEachMutatorAtASafepoint([&](Thread* thread) {
+      DartFrameIterator iterator(
+          thread, StackFrameIterator::kAllowCrossThreadIteration);
+      StackFrame* frame = iterator.NextFrame();
+      while (frame != nullptr) {
+        if (!frame->is_interpreted()) {
+          code = frame->LookupDartCode();
+
+          if (set.ContainsKey(code)) {
+            ReportDeoptimization(code);
+            DeoptimizeAt(thread, code, frame);
           }
-          DartFrameIterator iterator(
-              mutator_thread, StackFrameIterator::kAllowCrossThreadIteration);
-          StackFrame* frame = iterator.NextFrame();
-          while (frame != nullptr) {
-            if (!frame->is_interpreted()) {
-              code = frame->LookupDartCode();
-
-              if (set.ContainsKey(code)) {
-                ReportDeoptimization(code);
-                DeoptimizeAt(mutator_thread, code, frame);
-              }
-            }
-            frame = iterator.NextFrame();
-          }
-        },
-        /*at_safepoint=*/true);
+        }
+        frame = iterator.NextFrame();
+      }
+    });
 
     // Switch functions that use dependent code to unoptimized code.
     Object& owner = Object::Handle();

runtime/vm/heap/weak_code.cc

@@ -320,6 +320,7 @@ IsolateGroup::IsolateGroup(std::shared_ptr<IsolateGroupSource> source,
       thread_pool_(),
       isolates_lock_(new SafepointRwLock()),
       isolates_(),
+      mutators_(),
       start_time_micros_(OS::GetCurrentMonotonicMicros()),
       is_system_isolate_group_(source->flags.is_system_isolate),
       random_(),
@@ -473,13 +474,15 @@ bool IsolateGroup::UnregisterIsolateDecrementCount() {
   return isolate_count_ == 0;
 }
 
-void IsolateGroup::RegisterIsolateGroupMutator() {
+void IsolateGroup::RegisterIsolateGroupMutator(Thread* mutator) {
   SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
+  mutators_.Append(mutator);
   group_mutator_count_++;
 }
 
-void IsolateGroup::UnregisterIsolateGroupMutator() {
+void IsolateGroup::UnregisterIsolateGroupMutator(Thread* mutator) {
   SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
+  mutators_.Remove(mutator);
   group_mutator_count_--;
 }
 
@@ -2914,6 +2917,27 @@ Isolate* IsolateGroup::FirstIsolateLocked() const {
   return isolates_.IsEmpty() ? nullptr : isolates_.First();
 }
 
+void IsolateGroup::ForEachMutatorAtASafepoint(
+    std::function<void(Thread* thread)> function) {
+  auto thread = Thread::Current();
+  ASSERT(thread->OwnsSafepoint() ||
+         (thread->task_kind() == Thread::kMutatorTask) ||
+         (thread->task_kind() == Thread::kMarkerTask) ||
+         (thread->task_kind() == Thread::kCompactorTask) ||
+         (thread->task_kind() == Thread::kScavengerTask) ||
+         (thread->task_kind() == Thread::kIncrementalCompactorTask));
+  for (Isolate* isolate : isolates_) {
+    auto thread = isolate->mutator_thread();
+    if (thread != nullptr) {
+      function(thread);
+    }
+  }
+  for (Thread* mutator : mutators_) {
+    ASSERT(mutator != nullptr);
+    function(mutator);
+  }
+}
+
 void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable) {
   auto thread = Thread::Current();
   StoppedMutatorsScope stopped_mutators_scope(thread);

runtime/vm/isolate.cc

@@ -374,8 +374,8 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   // Returns `true` if this was the last isolate and the caller is responsible
   // for deleting the isolate group.
   bool UnregisterIsolateDecrementCount();
-  void RegisterIsolateGroupMutator();
-  void UnregisterIsolateGroupMutator();
+  void RegisterIsolateGroupMutator(Thread* mutator);
+  void UnregisterIsolateGroupMutator(Thread* mutator);
 
   bool ContainsOnlyOneIsolate();
 
@@ -650,6 +650,8 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   Isolate* FirstIsolate() const;
   Isolate* FirstIsolateLocked() const;
 
+  void ForEachMutatorAtASafepoint(std::function<void(Thread* thread)> function);
+
   // Ensures mutators are stopped during execution of the provided function.
   //
   // If the current thread is the only mutator in the isolate group,
@@ -907,6 +909,7 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   IntrusiveDList<Isolate> isolates_;
   RelaxedAtomic<Dart_Port> interrupt_port_ = ILLEGAL_PORT;
   intptr_t isolate_count_ = 0;
+  IntrusiveDList<Thread> mutators_;
   intptr_t group_mutator_count_ = 0;
   bool initial_spawn_successful_ = false;
   Dart_LibraryTagHandler library_tag_handler_ = nullptr;

runtime/vm/isolate.h

@@ -537,10 +537,10 @@ void Thread::EnterIsolateGroupAsMutator(IsolateGroup* isolate_group,
   isolate_group->IncreaseMutatorCount(/*thread=*/nullptr,
                                       /*is_nested_reenter=*/true,
                                       /*was_stolen=*/false);
-  isolate_group->RegisterIsolateGroupMutator();
-
   Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr,
                                    kMutatorTask, bypass_safepoint);
+  isolate_group->RegisterIsolateGroupMutator(thread);
+
   RELEASE_ASSERT(thread != nullptr);
   // Even if [bypass_safepoint] is true, a thread may need mutator state (e.g.
   // parallel scavenger threads write to the [Thread]s storebuffer)
@@ -583,8 +583,8 @@ void Thread::ExitIsolateGroupAsMutator(bool bypass_safepoint) {
   thread->ClearStackLimit();
   SuspendThreadInternal(thread, VMTag::kInvalidTagId);
   auto group = thread->isolate_group();
+  group->UnregisterIsolateGroupMutator(thread);
   FreeActiveThread(thread, /*isolate=*/nullptr, bypass_safepoint);
-  group->UnregisterIsolateGroupMutator();
   group->DecreaseMutatorCount(/*is_nested_exit=*/true);
 }
 

runtime/vm/thread.cc

@@ -23,6 +23,7 @@
 #include "vm/handles.h"
 #include "vm/heap/pointer_block.h"
 #include "vm/heap/sampler.h"
+#include "vm/intrusive_dlist.h"
 #include "vm/os_thread.h"
 #include "vm/pending_deopts.h"
 #include "vm/random.h"
@@ -364,7 +365,7 @@ class MutatorThreadVisitor {
 // a thread is allocated by ThreadRegistry::GetFromFreelistLocked either
 // before entering an isolate or entering an isolate group, and destroyed
 // automatically when the underlying OS thread exits.
-class Thread : public ThreadState {
+class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
  public:
   // The kind of task this thread is performing. Sampled by the profiler.
   enum TaskKind {

runtime/vm/thread.h

@@ -0,0 +1,65 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+//
+// Tests deoptimization in presence of isolategroup-bound code.
+//
+// VMOptions=--experimental-shared-data --optimization-counter-threshold=10  --no-background-compilation
+
+// Test that lazy deoptimization works if the program returns to a function
+// that is scheduled for lazy deoptimization via an exception.
+
+import 'package:dart_internal/isolate_group.dart' show IsolateGroup;
+import 'package:expect/expect.dart';
+
+class C {
+  dynamic x = 42;
+}
+
+@pragma('vm:never-inline')
+AA(C c, bool b) {
+  if (b) {
+    c.x = 2.5;
+    throw 123;
+  }
+}
+
+@pragma('vm:never-inline')
+T1(C c, bool b) {
+  try {
+    AA(c, b);
+  } on dynamic {}
+  return c.x + 1;
+}
+
+@pragma('vm:never-inline')
+T2(C c, bool b) {
+  try {
+    AA(c, b);
+  } on String {
+    Expect.isTrue(false);
+  } on int catch (e) {
+    Expect.equals(e, 123);
+    Expect.equals(b, true);
+    Expect.equals(c.x, 2.5);
+  }
+  return c.x + 1;
+}
+
+mainBody() {
+  var c = new C();
+  for (var i = 0; i < 10000; ++i) {
+    T1(c, false);
+    T2(c, false);
+  }
+  Expect.equals(43, T1(c, false));
+  Expect.equals(43, T2(c, false));
+  Expect.equals(3.5, T1(c, true));
+  Expect.equals(3.5, T2(c, true));
+}
+
+main() {
+  IsolateGroup.runSync(() {
+    mainBody();
+  });
+}