[vm] Ensure reload safepoint operation sends OOBs to all non-parked mutators

The reload safepoint operation mechanism will send out-of-band message
to all isolates that are not already at-reload-safepoint.

It identified all such threads by iterating the active threads list,
filtering those that are not at-reload-safepoint and those with an
active isolate (i.e. `thread->isolate() != nullptr`).

This `thread->isolate() != nullptr` condition isn't quite
correct. A mutator may temporarily run under a [NoActiveIsolateScope] to
make the `Isolate::Current()` unavailable to code that shouldn't depend
on isolates (e.g. GC and Compiler do that) or may even have the
incorrect isolate on it (e.g. Debugger Service notifications).

So if one thread triggers a [ReloadSafepointOperation] and another
thread is under a [NoActiveIsolateScope] it will not get the OOB message
and therefore not get interrupted to check-in.

This has caused flaky timeouts of `vm/dart/isolates/reload_active_stack_test`
- as this test runs isolates with active stack without going back
to event loop (which would check into reload operations).

Closes https://github.com/dart-lang/sdk/issues/52135

TEST=Updated vm/cc/Reload_NotAtSafepoint for regression test
TEST=Fixes flaky timeouts of vm/dart/isolates/reload_active_stack_test

Change-Id: Ib407c42aa97798ac994aff3bce263da79b83666a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302320
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

runtime/vm/heap/safepoint.cc

@@ -178,7 +178,7 @@ void SafepointHandler::LevelHandler::NotifyThreadsToGetToSafepointLevel(
           // via `Thread::EnterIsolateGroupAsHelper()`. In that case we cannot
           // send an OOB message. Instead we'll have to wait until that thread
           // de-schedules itself.
-          auto isolate = current->isolate();
+          auto isolate = current->scheduled_dart_mutator_isolate();
           if (isolate != nullptr) {
             oob_isolates->Add(isolate->main_port());
           }

runtime/vm/heap/safepoint_test.cc

@@ -757,15 +757,23 @@ ISOLATE_UNIT_TEST_CASE(Reload_NotAtSafepoint) {
 
   std::shared_ptr<ReloadTask::Data> task(
       new ReloadTask::Data(isolate->group()));
-  pool.Run<ReloadTask>(task);
-  task->WaitUntil(ReloadTask::kEntered);
 
-  // We are not at a safepoint. The [ReloadTask] will trigger a reload
-  // safepoint operation, sees that we are not at reload safepoint and instead
-  // sends us an OOB.
-  ASSERT(!thread->IsAtSafepoint());
-  while (!messages->HasOOBMessages()) {
-    OS::Sleep(1000);
+  {
+    // Even if we are not running with an active isolate (e.g. due to being in
+    // GC / Compiler) the reload safepoint operation should still send us an OOB
+    // message (it should know this thread belongs to an isolate).
+    NoActiveIsolateScope no_active_isolate(thread);
+
+    pool.Run<ReloadTask>(task);
+    task->WaitUntil(ReloadTask::kEntered);
+
+    // We are not at a safepoint. The [ReloadTask] will trigger a reload
+    // safepoint operation, sees that we are not at reload safepoint and instead
+    // sends us an OOB.
+    ASSERT(!thread->IsAtSafepoint());
+    while (!messages->HasOOBMessages()) {
+      OS::Sleep(1000);
+    }
   }
 
   // Examine the OOB message for it's content.

runtime/vm/thread.cc

@@ -330,7 +330,7 @@ void Thread::AssertEmptyThreadInvariants() {
   ASSERT(vm_tag_ == VMTag::kInvalidTagId);
   ASSERT(task_kind_ == kUnknownTask);
   ASSERT(execution_state_ == Thread::kThreadInNative);
-  ASSERT(!is_dart_mutator_);
+  ASSERT(scheduled_dart_mutator_isolate_ == nullptr);
 
   ASSERT(write_barrier_mask_ == UntaggedObject::kGenerationalBarrierMask);
   ASSERT(store_buffer_block_ == nullptr);
@@ -390,7 +390,7 @@ bool Thread::EnterIsolate(Isolate* isolate) {
   Thread* thread = nullptr;
   if (is_resumable) {
     thread = isolate->mutator_thread();
-    ASSERT(thread->is_dart_mutator_);
+    ASSERT(thread->scheduled_dart_mutator_isolate_ == isolate);
     ASSERT(thread->isolate() == isolate);
     ASSERT(thread->isolate_group() == isolate->group());
     {
@@ -597,7 +597,7 @@ Thread* Thread::AddActiveThread(IsolateGroup* group,
 
   thread->isolate_ = isolate;  // May be nullptr.
   thread->isolate_group_ = group;
-  thread->is_dart_mutator_ = is_dart_mutator;
+  thread->scheduled_dart_mutator_isolate_ = isolate;
 
   // We start at being at-safepoint (in case any safepoint operation is
   // in-progress, we'll check into it once leaving the safepoint)
@@ -648,7 +648,7 @@ void Thread::FreeActiveThread(Thread* thread, bool bypass_safepoint) {
 
   thread->isolate_ = nullptr;
   thread->isolate_group_ = nullptr;
-  thread->is_dart_mutator_ = false;
+  thread->scheduled_dart_mutator_isolate_ = nullptr;
   thread->set_execution_state(Thread::kThreadInNative);
   thread->stack_limit_.store(0);
   thread->safepoint_state_ = 0;

runtime/vm/thread.h

@@ -549,7 +549,20 @@ class Thread : public ThreadState {
     return OFFSET_OF(Thread, field_table_values_);
   }
 
-  bool IsDartMutatorThread() const { return is_dart_mutator_; }
+  bool IsDartMutatorThread() const {
+    return scheduled_dart_mutator_isolate_ != nullptr;
+  }
+
+  // Returns the dart mutator [Isolate] this thread belongs to or nullptr.
+  //
+  // `isolate()` in comparison can return
+  //   - `nullptr` for dart mutators (e.g. if the mutator runs under
+  //     [NoActiveIsolateScope])
+  //   - an incorrect isolate (e.g. if [ActiveIsolateScope] is used to seemingly
+  //     enter another isolate)
+  Isolate* scheduled_dart_mutator_isolate() const {
+    return scheduled_dart_mutator_isolate_;
+  }
 
 #if defined(DEBUG)
   bool IsInsideCompiler() const { return inside_compiler_; }
@@ -1382,7 +1395,7 @@ class Thread : public ThreadState {
 #endif
 
   Thread* next_;  // Used to chain the thread structures in an isolate.
-  bool is_dart_mutator_ = false;
+  Isolate* scheduled_dart_mutator_isolate_ = nullptr;
 
   bool is_unwind_in_progress_ = false;