commit 6a60078d96831f9a54e3e28bce3dfdb8ea73584c
author Alexander Aprelev <aam@google.com> Fri Mar 04 03:28:51 2022 +0000
committer Commit Bot <commit-bot@chromium.org> Fri Mar 04 03:28:51 2022 +0000
tree e7bcd77ff20bceab6833bca63ab1de4039d82a81
parent d3e36fab5eb55706a36144fe41abc2546ffa0b55

[vm/finalize] Ensure weak finalizers are invoked with isolate group still available.

Fixes https://github.com/dart-lang/sdk/issues/48321

Tested: DartAPI_WeakPersistentHandleCleanupFinalizerAtShutdown
Change-Id: If437dff4e524cb74ac8d0b3c80ec6bb56dd24e84
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235280
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>

runtime/vm/dart_api_impl_test.cc

diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc
index 73c4f52..4d7f959 100644
--- a/runtime/vm/dart_api_impl_test.cc
+++ b/runtime/vm/dart_api_impl_test.cc
@@ -3633,6 +3633,23 @@
 static Dart_WeakPersistentHandle weak_persistent_handle2;
 static Dart_WeakPersistentHandle weak_persistent_handle3;
 
+static void WeakPersistentHandlePeerCleanupEnsuresIGFinalizer(
+    void* isolate_callback_data,
+    void* peer) {
+  ASSERT(IsolateGroup::Current() != nullptr);
+}
+
+TEST_CASE(DartAPI_WeakPersistentHandleCleanupFinalizerAtShutdown) {
+  const char* kTestString1 = "Test String1";
+  int peer3 = 0;
+  Dart_EnterScope();
+  CHECK_API_SCOPE(thread);
+  Dart_Handle ref3 = Dart_NewStringFromCString(kTestString1);
+  weak_persistent_handle3 = Dart_NewWeakPersistentHandle(
+      ref3, &peer3, 0, WeakPersistentHandlePeerCleanupEnsuresIGFinalizer);
+  Dart_ExitScope();
+}
+
 static void WeakPersistentHandlePeerCleanupFinalizer(
     void* isolate_callback_data,
     void* peer) {

runtime/vm/isolate.cc

diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index c6e70aa..260c293 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -412,10 +412,6 @@
 }
 
 IsolateGroup::~IsolateGroup() {
-  // Finalize any weak persistent handles with a non-null referent.
-  FinalizeWeakPersistentHandlesVisitor visitor(this);
-  api_state()->VisitWeakHandlesUnlocked(&visitor);
-
   // Ensure we destroy the heap before the other members.
   heap_ = nullptr;
   ASSERT(marking_stack_ == nullptr);
@@ -2658,14 +2654,20 @@
   if (shutdown_group) {
     KernelIsolate::NotifyAboutIsolateGroupShutdown(isolate_group);
 
-#if !defined(DART_PRECOMPILED_RUNTIME)
     if (!is_vm_isolate) {
       Thread::EnterIsolateGroupAsHelper(isolate_group, Thread::kUnknownTask,
                                         /*bypass_safepoint=*/false);
+#if !defined(DART_PRECOMPILED_RUNTIME)
       BackgroundCompiler::Stop(isolate_group);
+#endif  // !defined(DART_PRECOMPILED_RUNTIME)
+
+      // Finalize any weak persistent handles with a non-null referent with
+      // isolate group still being available.
+      FinalizeWeakPersistentHandlesVisitor visitor(isolate_group);
+      isolate_group->api_state()->VisitWeakHandlesUnlocked(&visitor);
+
       Thread::ExitIsolateGroupAsHelper(/*bypass_safepoint=*/false);
     }
-#endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
     // The "vm-isolate" does not have a thread pool.
     ASSERT(is_vm_isolate == (isolate_group->thread_pool() == nullptr));