[vm, gc] Set the current isolate to the group's only member before running finalizers.

Some embedder code relies on the presence of a current isolate equal to the isolate that created the handle. This code will be need to be updated to be isolate-group aware before we can run finalizers only in the context of the isolate group instead of an isolate.

Bug: https://github.com/dart-lang/sdk/issues/40836
Change-Id: I38956c556df551e48ee7e4d0822aa5def8dc4fc5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138013
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>

runtime/vm/dart_api_impl_test.cc

@@ -3468,6 +3468,50 @@ TEST_CASE(DartAPI_WeakPersistentHandleExternalAllocationSizeOddReferents) {
   }
 }
 
+static void AssertingFinalizer(void* isolate_callback_data,
+                               Dart_WeakPersistentHandle handle,
+                               void* peer) {
+  Dart_Isolate expected_isolate = reinterpret_cast<Dart_Isolate>(peer);
+  EXPECT_EQ(expected_isolate, Dart_CurrentIsolate());
+}
+
+// TODO(https://github.com/dart-lang/sdk/issues/40836): Remove.
+TEST_CASE(DartAPI_WeakPersistentHandleFinalizerCurrentIsolate) {
+  {
+    Dart_EnterScope();
+
+    Dart_Handle obj = AllocateNewString("ABC");
+    void* peer = Dart_CurrentIsolate();
+    EXPECT_NOTNULL(peer);
+    intptr_t external_size = 0;
+    Dart_NewWeakPersistentHandle(obj, peer, external_size, AssertingFinalizer);
+
+    Dart_ExitScope();
+  }
+
+  {
+    TransitionNativeToVM transition(thread);
+    GCTestHelper::CollectAllGarbage();
+  }
+
+  {
+    Dart_EnterScope();
+
+    Dart_Handle obj = AllocateOldString("ABC");
+    void* peer = Dart_CurrentIsolate();
+    EXPECT_NOTNULL(peer);
+    intptr_t external_size = 0;
+    Dart_NewWeakPersistentHandle(obj, peer, external_size, AssertingFinalizer);
+
+    Dart_ExitScope();
+  }
+
+  {
+    TransitionNativeToVM transition(thread);
+    GCTestHelper::CollectAllGarbage();
+  }
+}
+
 static Dart_WeakPersistentHandle weak1 = NULL;
 static Dart_WeakPersistentHandle weak2 = NULL;
 static Dart_WeakPersistentHandle weak3 = NULL;

runtime/vm/heap/heap.cc

@@ -52,6 +52,24 @@ class NoActiveIsolateScope {
   Isolate* saved_isolate_;
 };
 
+RunFinalizersScope::RunFinalizersScope(Thread* thread) : thread_(thread) {
+  if (!FLAG_enable_isolate_groups) {
+    ASSERT(thread->IsAtSafepoint() ||
+           (thread->task_kind() == Thread::kMarkerTask));
+    IsolateGroup* isolate_group = thread->isolate_group();
+    Isolate* isolate = isolate_group->isolates_.First();
+    ASSERT(isolate == isolate_group->isolates_.Last());
+    saved_isolate_ = thread->isolate_;
+    thread->isolate_ = isolate;
+  }
+}
+
+RunFinalizersScope::~RunFinalizersScope() {
+  if (!FLAG_enable_isolate_groups) {
+    thread_->isolate_ = saved_isolate_;
+  }
+}
+
 Heap::Heap(IsolateGroup* isolate_group,
            intptr_t max_new_gen_semi_words,
            intptr_t max_old_gen_words)
@@ -460,8 +478,7 @@ void Heap::EvacuateNewSpace(Thread* thread, GCReason reason) {
 }
 
 void Heap::CollectNewSpaceGarbage(Thread* thread, GCReason reason) {
-  // TODO(40836) : Uncomment this line once the issue is fixed.
-  // NoActiveIsolateScope no_active_isolate_scope;
+  NoActiveIsolateScope no_active_isolate_scope;
   ASSERT((reason != kOldSpace) && (reason != kPromotion));
   if (thread->isolate_group() == Dart::vm_isolate()->group()) {
     // The vm isolate cannot safely collect garbage due to unvisited read-only
@@ -495,8 +512,7 @@ void Heap::CollectNewSpaceGarbage(Thread* thread, GCReason reason) {
 void Heap::CollectOldSpaceGarbage(Thread* thread,
                                   GCType type,
                                   GCReason reason) {
-  // TODO(40836) : Uncomment this line once the issue is fixed.
-  // NoActiveIsolateScope no_active_isolate_scope;
+  NoActiveIsolateScope no_active_isolate_scope;
 
   ASSERT(reason != kNewSpace);
   ASSERT(type != kScavenge);

runtime/vm/heap/heap.h

@@ -30,6 +30,20 @@ class ServiceEvent;
 class TimelineEventScope;
 class VirtualMemory;
 
+// Temporarily enter the only isolate in the group before running weak handle
+// finalizers to keep existing embedder code working. Remove once embedders
+// are updated to think in terms of isolate groups.
+// TODO(https://github.com/dart-lang/sdk/issues/40836): Remove.
+class RunFinalizersScope {
+ public:
+  explicit RunFinalizersScope(Thread* thread);
+  ~RunFinalizersScope();
+
+ private:
+  Thread* thread_;
+  Isolate* saved_isolate_;
+};
+
 class Heap {
  public:
   enum Space {

runtime/vm/heap/marker.cc

@@ -465,9 +465,12 @@ void GCMarker::IterateWeakRoots(Thread* thread) {
 
 void GCMarker::ProcessWeakHandles(Thread* thread) {
   TIMELINE_FUNCTION_GC_DURATION(thread, "ProcessWeakHandles");
+
   MarkingWeakVisitor visitor(thread);
   ApiState* state = isolate_group_->api_state();
   ASSERT(state != NULL);
+
+  RunFinalizersScope tcis(thread);
   isolate_group_->VisitWeakPersistentHandles(&visitor);
 }
 

runtime/vm/heap/scavenger.cc

@@ -772,6 +772,7 @@ bool Scavenger::IsUnreachable(RawObject** p) {
 
 void Scavenger::IterateWeakRoots(IsolateGroup* isolate_group,
                                  HandleVisitor* visitor) {
+  RunFinalizersScope tcis(Thread::Current());
   isolate_group->VisitWeakPersistentHandles(visitor);
 }
 

runtime/vm/isolate.h

@@ -476,6 +476,7 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
  private:
   friend class Heap;
   friend class StackFrame;  // For `[isolates_].First()`.
+  friend class RunFinalizersScope;  // For `[isolates_].First()`.
 
 #define ISOLATE_GROUP_FLAG_BITS(V) V(CompactionInProgress)
 

runtime/vm/thread.h

@@ -1034,6 +1034,7 @@ class Thread : public ThreadState {
   friend Isolate* CreateWithinExistingIsolateGroup(IsolateGroup*,
                                                    const char*,
                                                    char**);
+  friend class RunFinalizersScope;
   DISALLOW_COPY_AND_ASSIGN(Thread);
 };