Use OOB messages to schedule compaction for Dart_NotifyLowMemory.

This ensures any finalizers that run during compaction happen on a thread the embedder knows about.

Bug: https://github.com/dart-lang/sdk/issues/31662
Change-Id: If1ca39fe72937ad2bc4607f7cacc4dc82e49be01
Reviewed-on: https://dart-review.googlesource.com/30040
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>

runtime/include/dart_api.h

@@ -486,11 +486,12 @@ DART_EXPORT void Dart_DeletePersistentHandle(Dart_PersistentHandle object);
  * calling Dart_DeleteWeakPersistentHandle.
  *
  * If the object becomes unreachable the callback is invoked with the weak
- * persistent handle and the peer as arguments. This gives the native code the
+ * persistent handle and the peer as arguments. The callback is invoked on the
- * ability to cleanup data associated with the object and clear out any cached
+ * thread that has entered the isolate at the time of garbage collection. This
- * references to the handle. All references to this handle after the callback
+ * gives the embedder the ability to cleanup data associated with the object and
- * will be invalid. It is illegal to call into the VM from the callback.
+ * clear out any cached references to the handle. All references to this handle
- * If the handle is deleted before the object becomes unreachable,
+ * after the callback will be invalid. It is illegal to call into the VM from
+ * the callback. If the handle is deleted before the object becomes unreachable,
  * the callback is never invoked.
  *
  * Requires there to be a current isolate.

runtime/vm/dart_api_impl_test.cc

@@ -1240,7 +1240,7 @@ class GCTestHelper : public AllStatic {
     Thread* thread = Thread::Current();
     PageSpace* old_space = thread->isolate()->heap()->old_space();
     MonitorLocker ml(old_space->tasks_lock());
-    while (old_space->sweeper_tasks() > 0) {
+    while (old_space->tasks() > 0) {
       ml.WaitWithSafepointCheck(thread);
     }
   }

runtime/vm/gc_sweeper.cc

@@ -115,7 +115,7 @@ class SweeperTask : public ThreadPool::Task {
     ASSERT(last_ != NULL);
     ASSERT(freelist_ != NULL);
     MonitorLocker ml(old_space_->tasks_lock());
-    old_space_->set_sweeper_tasks(old_space_->sweeper_tasks() + 1);
+    old_space_->set_tasks(old_space_->tasks() + 1);
   }
 
   virtual void Run() {
@@ -155,7 +155,7 @@ class SweeperTask : public ThreadPool::Task {
     // This sweeper task is done. Notify the original isolate.
     {
       MonitorLocker ml(old_space_->tasks_lock());
-      old_space_->set_sweeper_tasks(old_space_->sweeper_tasks() - 1);
+      old_space_->set_tasks(old_space_->tasks() - 1);
       ml.NotifyAll();
     }
   }

runtime/vm/heap.cc

@@ -205,14 +205,14 @@ HeapIterationScope::HeapIterationScope(Thread* thread, bool writable)
     // We currently don't support nesting of HeapIterationScopes.
     ASSERT(old_space_->iterating_thread_ != thread);
 #endif
-    while (old_space_->sweeper_tasks() > 0) {
+    while (old_space_->tasks() > 0) {
       ml.WaitWithSafepointCheck(thread);
     }
 #if defined(DEBUG)
     ASSERT(old_space_->iterating_thread_ == NULL);
     old_space_->iterating_thread_ = thread;
 #endif
-    old_space_->set_sweeper_tasks(1);
+    old_space_->set_tasks(1);
   }
 
   isolate()->safepoint_handler()->SafepointThreads(thread);
@@ -234,8 +234,8 @@ HeapIterationScope::~HeapIterationScope() {
   ASSERT(old_space_->iterating_thread_ == thread());
   old_space_->iterating_thread_ = NULL;
 #endif
-  ASSERT(old_space_->sweeper_tasks() == 1);
+  ASSERT(old_space_->tasks() == 1);
-  old_space_->set_sweeper_tasks(0);
+  old_space_->set_tasks(0);
   ml.NotifyAll();
 }
 
@@ -365,49 +365,8 @@ void Heap::NotifyIdle(int64_t deadline) {
   }
 }
 
-class LowMemoryTask : public ThreadPool::Task {
- public:
-  explicit LowMemoryTask(Isolate* isolate) : task_isolate_(isolate) {}
-
-  virtual void Run() {
-    bool result =
-        Thread::EnterIsolateAsHelper(task_isolate_, Thread::kLowMemoryTask);
-    ASSERT(result);
-    Heap* heap = task_isolate_->heap();
-    {
-      TIMELINE_FUNCTION_GC_DURATION(Thread::Current(), "LowMemoryTask");
-      heap->CollectAllGarbage(Heap::kLowMemory);
-    }
-    // Exit isolate cleanly *before* notifying it, to avoid shutdown race.
-    Thread::ExitIsolateAsHelper();
-    // This compactor task is done. Notify the original isolate.
-    {
-      MonitorLocker ml(heap->old_space()->tasks_lock());
-      heap->old_space()->set_low_memory_tasks(
-          heap->old_space()->low_memory_tasks() - 1);
-      ml.NotifyAll();
-    }
-  }
-
- private:
-  Isolate* task_isolate_;
-};
-
 void Heap::NotifyLowMemory() {
-  {
+  CollectAllGarbage(kLowMemory);
-    MonitorLocker ml(old_space_.tasks_lock());
-    if (old_space_.low_memory_tasks() > 0) {
-      return;
-    }
-    old_space_.set_low_memory_tasks(old_space_.low_memory_tasks() + 1);
-  }
-
-  bool success = Dart::thread_pool()->Run(new LowMemoryTask(isolate()));
-  if (!success) {
-    MonitorLocker ml(old_space_.tasks_lock());
-    old_space_.set_low_memory_tasks(old_space_.low_memory_tasks() - 1);
-    ml.NotifyAll();
-  }
 }
 
 void Heap::EvacuateNewSpace(Thread* thread, GCReason reason) {
@@ -503,7 +462,7 @@ void Heap::CollectAllGarbage(GCReason reason) {
 
 void Heap::WaitForSweeperTasks(Thread* thread) {
   MonitorLocker ml(old_space_.tasks_lock());
-  while (old_space_.sweeper_tasks() > 0) {
+  while (old_space_.tasks() > 0) {
     ml.WaitWithSafepointCheck(thread);
   }
 }

runtime/vm/isolate.cc

@@ -418,6 +418,10 @@ RawError* IsolateMessageHandler::HandleLibMessage(const Array& message) {
 #endif
       break;
     }
+    case Isolate::kLowMemoryMsg: {
+      I->heap()->NotifyLowMemory();
+      break;
+    }
 
     case Isolate::kAddExitMsg:
     case Isolate::kDelExitMsg:
@@ -1700,21 +1704,7 @@ class FinalizeWeakPersistentHandlesVisitor : public HandleVisitor {
 
 // static
 void Isolate::NotifyLowMemory() {
-  MonitorLocker ml(isolates_list_monitor_);
+  Isolate::KillAllIsolates(Isolate::kLowMemoryMsg);
-  if (!creation_enabled_) {
-    return;  // VM is shutting down
-  }
-  for (Isolate* isolate = isolates_list_head_; isolate != NULL;
-       isolate = isolate->next_) {
-    if (isolate == Dart::vm_isolate()) {
-      // Nothing to compact / isolate structure not completely initialized.
-    } else if (isolate == Isolate::Current()) {
-      isolate->heap()->NotifyLowMemory();
-    } else {
-      MutexLocker ml(isolate->mutex_);
-      isolate->heap()->NotifyLowMemory();
-    }
-  }
 }
 
 void Isolate::LowLevelShutdown() {
@@ -1846,9 +1836,8 @@ void Isolate::Shutdown() {
     // TODO(koda): Support faster sweeper shutdown (e.g., after current page).
     PageSpace* old_space = heap_->old_space();
     MonitorLocker ml(old_space->tasks_lock());
-    while (old_space->sweeper_tasks() > 0 ||
+    while (old_space->tasks() > 0) {
-           old_space->low_memory_tasks() > 0) {
+      ml.Wait();
-      ml.WaitWithSafepointCheck(thread);
     }
   }
 
@@ -1871,8 +1860,7 @@ void Isolate::Shutdown() {
   if (heap_ != NULL) {
     PageSpace* old_space = heap_->old_space();
     MonitorLocker ml(old_space->tasks_lock());
-    ASSERT(old_space->sweeper_tasks() == 0);
+    ASSERT(old_space->tasks() == 0);
-    ASSERT(old_space->low_memory_tasks() == 0);
   }
 #endif
 

runtime/vm/isolate.h

@@ -166,6 +166,7 @@ class Isolate : public BaseIsolate {
     // Internal message ids.
     kInterruptMsg = 10,     // Break in the debugger.
     kInternalKillMsg = 11,  // Like kill, but does not run exit listeners, etc.
+    kLowMemoryMsg = 12,     // Run compactor, etc.
   };
   // The different Isolate API message priorities for ping and kill messages.
   enum LibMsgPriority {

runtime/vm/pages.cc

@@ -183,8 +183,7 @@ PageSpace::PageSpace(Heap* heap,
       max_capacity_in_words_(max_capacity_in_words),
       max_external_in_words_(max_external_in_words),
       tasks_lock_(new Monitor()),
-      sweeper_tasks_(0),
+      tasks_(0),
-      low_memory_tasks_(0),
 #if defined(DEBUG)
       iterating_thread_(NULL),
 #endif
@@ -203,7 +202,7 @@ PageSpace::PageSpace(Heap* heap,
 PageSpace::~PageSpace() {
   {
     MonitorLocker ml(tasks_lock());
-    while (sweeper_tasks() > 0) {
+    while (tasks() > 0) {
       ml.Wait();
     }
   }
@@ -851,7 +850,7 @@ bool PageSpace::ShouldPerformIdleMarkSweep(int64_t deadline) {
 
   {
     MonitorLocker locker(tasks_lock());
-    if (sweeper_tasks() > 0) {
+    if (tasks() > 0) {
       // A concurrent sweeper is running. If we start a mark sweep now
       // we'll have to wait for it, and this wait time is not included in
       // mark_sweep_words_per_micro_.
@@ -875,10 +874,10 @@ void PageSpace::CollectGarbage(bool compact) {
   // Wait for pending tasks to complete and then account for the driver task.
   {
     MonitorLocker locker(tasks_lock());
-    while (sweeper_tasks() > 0) {
+    while (tasks() > 0) {
       locker.WaitWithSafepointCheck(thread);
     }
-    set_sweeper_tasks(1);
+    set_tasks(1);
   }
 
   const int64_t pre_safe_point = OS::GetCurrentMonotonicMicros();
@@ -1036,7 +1035,7 @@ void PageSpace::CollectGarbage(bool compact) {
   // Done, reset the task count.
   {
     MonitorLocker ml(tasks_lock());
-    set_sweeper_tasks(sweeper_tasks() - 1);
+    set_tasks(tasks() - 1);
     ml.NotifyAll();
   }
 

runtime/vm/pages.h

@@ -330,15 +330,10 @@ class PageSpace {
   }
 
   Monitor* tasks_lock() const { return tasks_lock_; }
-  intptr_t sweeper_tasks() const { return sweeper_tasks_; }
+  intptr_t tasks() const { return tasks_; }
-  void set_sweeper_tasks(intptr_t val) {
+  void set_tasks(intptr_t val) {
     ASSERT(val >= 0);
-    sweeper_tasks_ = val;
+    tasks_ = val;
-  }
-  intptr_t low_memory_tasks() const { return low_memory_tasks_; }
-  void set_low_memory_tasks(intptr_t val) {
-    ASSERT(val >= 0);
-    low_memory_tasks_ = val;
   }
 
   // Attempt to allocate from bump block rather than normal freelist.
@@ -439,8 +434,7 @@ class PageSpace {
 
   // Keep track of running MarkSweep tasks.
   Monitor* tasks_lock_;
-  int32_t sweeper_tasks_;
+  intptr_t tasks_;
-  int32_t low_memory_tasks_;
 #if defined(DEBUG)
   Thread* iterating_thread_;
 #endif

runtime/vm/scavenger.cc

@@ -498,7 +498,7 @@ void Scavenger::Epilogue(Isolate* isolate, SemiSpace* from) {
   {
     PageSpace* page_space = heap_->old_space();
     MonitorLocker ml(page_space->tasks_lock());
-    if (page_space->sweeper_tasks() == 0) {
+    if (page_space->tasks() == 0) {
       VerifyStoreBufferPointerVisitor verify_store_buffer_visitor(isolate, to_);
       heap_->old_space()->VisitObjectPointers(&verify_store_buffer_visitor);
     }

runtime/vm/thread.h

@@ -169,7 +169,6 @@ class Thread : public BaseThread {
     kMarkerTask = 0x4,
     kSweeperTask = 0x8,
     kCompactorTask = 0x10,
-    kLowMemoryTask = 0x20,
   };
   // Converts a TaskKind to its corresponding C-String name.
   static const char* TaskKindToCString(TaskKind kind);