[vm, isolates] Set Isolate::mutator_thread_ under the thread registry lock.

Fixes data race with read in Isolate::ScheduleInterrupts.

TEST=tsan
Bug: #59649
Change-Id: I89dc54160f9c63ef5bdd9704a24e6eaeee1b3f4a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398543
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/vm/thread.cc

@@ -474,7 +474,8 @@ void Thread::ExitIsolate(bool isolate_shutdown) {
     thread->ResetMutatorState();
     thread->ResetState();
     SuspendDartMutatorThreadInternal(thread, VMTag::kInvalidTagId);
-    FreeActiveThread(thread, /*bypass_safepoint=*/false);
+    FreeActiveThread(thread, isolate, /*is_dart_mutator=*/true,
+                     /*bypass_safepoint=*/false);
   }
 
   // To let VM's thread pool (if we run on it) know that this thread is
@@ -488,7 +489,7 @@ void Thread::ExitIsolate(bool isolate_shutdown) {
 bool Thread::EnterIsolateGroupAsHelper(IsolateGroup* isolate_group,
                                        TaskKind kind,
                                        bool bypass_safepoint) {
-  Thread* thread = AddActiveThread(isolate_group, nullptr,
+  Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr,
                                    /*is_dart_mutator=*/false, bypass_safepoint);
   if (thread != nullptr) {
     thread->SetupState(kind);
@@ -512,13 +513,14 @@ void Thread::ExitIsolateGroupAsHelper(bool bypass_safepoint) {
   thread->ResetMutatorState();
   thread->ResetState();
   SuspendThreadInternal(thread, VMTag::kInvalidTagId);
-  FreeActiveThread(thread, bypass_safepoint);
+  FreeActiveThread(thread, /*isolate=*/nullptr, /*is_dart_mutator=*/false,
+                   bypass_safepoint);
 }
 
 bool Thread::EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group,
                                            TaskKind kind) {
   Thread* thread =
-      AddActiveThread(isolate_group, nullptr,
+      AddActiveThread(isolate_group, /*isolate=*/nullptr,
                       /*is_dart_mutator=*/false, /*bypass_safepoint=*/true);
   if (thread != nullptr) {
     thread->SetupState(kind);
@@ -537,7 +539,8 @@ void Thread::ExitIsolateGroupAsNonMutator() {
 
   thread->ResetState();
   SuspendThreadInternal(thread, VMTag::kInvalidTagId);
-  FreeActiveThread(thread, /*bypass_safepoint=*/true);
+  FreeActiveThread(thread, /*isolate=*/nullptr, /*is_dart_mutator=*/false,
+                   /*bypass_safepoint=*/true);
 }
 
 void Thread::ResumeDartMutatorThreadInternal(Thread* thread) {
@@ -621,6 +624,10 @@ Thread* Thread::AddActiveThread(IsolateGroup* group,
   thread->isolate_ = isolate;  // May be nullptr.
   thread->isolate_group_ = group;
   thread->scheduled_dart_mutator_isolate_ = isolate;
+  if (is_dart_mutator) {
+    ASSERT(thread_registry->threads_lock()->IsOwnedByCurrentThread());
+    isolate->mutator_thread_ = thread;
+  }
 
   // We start at being at-safepoint (in case any safepoint operation is
   // in-progress, we'll check into it once leaving the safepoint)
@@ -632,7 +639,10 @@ Thread* Thread::AddActiveThread(IsolateGroup* group,
   return thread;
 }
 
-void Thread::FreeActiveThread(Thread* thread, bool bypass_safepoint) {
+void Thread::FreeActiveThread(Thread* thread,
+                              Isolate* isolate,
+                              bool is_dart_mutator,
+                              bool bypass_safepoint) {
   ASSERT(!thread->HasActiveState());
   ASSERT(!thread->IsAtSafepoint());
 
@@ -672,6 +682,10 @@ void Thread::FreeActiveThread(Thread* thread, bool bypass_safepoint) {
   thread->isolate_ = nullptr;
   thread->isolate_group_ = nullptr;
   thread->scheduled_dart_mutator_isolate_ = nullptr;
+  if (is_dart_mutator) {
+    ASSERT(thread_registry->threads_lock()->IsOwnedByCurrentThread());
+    isolate->mutator_thread_ = nullptr;
+  }
   thread->set_execution_state(Thread::kThreadInNative);
   thread->stack_limit_.store(0);
   thread->safepoint_state_ = 0;
@@ -1454,7 +1468,6 @@ void Thread::ResetMutatorState() {
 
 void Thread::SetupDartMutatorState(Isolate* isolate) {
   field_table_values_ = isolate->field_table_->table();
-  isolate->mutator_thread_ = this;
 
   SetupDartMutatorStateDependingOnSnapshot(isolate->group());
 }
@@ -1490,7 +1503,6 @@ void Thread::SetupDartMutatorStateDependingOnSnapshot(IsolateGroup* group) {
 void Thread::ResetDartMutatorState(Isolate* isolate) {
   ASSERT(execution_state() == Thread::kThreadInVM);
 
-  isolate->mutator_thread_ = nullptr;
   is_unwind_in_progress_ = false;
 
   field_table_values_ = nullptr;

runtime/vm/thread.h

@@ -1481,7 +1481,10 @@ class Thread : public ThreadState {
   // Releases a active mutator threads from the thread registry.
   //
   // Thread needs to be at-safepoint.
-  static void FreeActiveThread(Thread* thread, bool bypass_safepoint);
+  static void FreeActiveThread(Thread* thread,
+                               Isolate* isolate,
+                               bool is_dart_mutator,
+                               bool bypass_safepoint);
 
   static void SetCurrent(Thread* current) { OSThread::SetCurrentTLS(current); }