[vm, gc] Require a safepoint for heap iteration.

runtime/vm/heap.cc

Index: runtime/vm/heap.cc
diff --git a/runtime/vm/heap.cc b/runtime/vm/heap.cc
index ad1f24deadd339df802c63bc1e38e44f711af771..56a71f6bb1eafba32591f68fe43fa550f45f7435 100644
--- a/runtime/vm/heap.cc
+++ b/runtime/vm/heap.cc
@@ -14,6 +14,7 @@
 #include "vm/os.h"
 #include "vm/pages.h"
 #include "vm/raw_object.h"
+#include "vm/safepoint.h"
 #include "vm/scavenger.h"
 #include "vm/service.h"
 #include "vm/service_event.h"
@@ -192,7 +193,8 @@ void Heap::VisitObjectsImagePages(ObjectVisitor* visitor) const {
 
 HeapIterationScope::HeapIterationScope(bool writable)
     : StackResource(Thread::Current()),
-      old_space_(isolate()->heap()->old_space()),
+      heap_(isolate()->heap()),
+      old_space_(heap_->old_space()),
       writable_(writable) {
   {
     // It's not yet safe to iterate over a paged space while it's concurrently
@@ -212,16 +214,20 @@ HeapIterationScope::HeapIterationScope(bool writable)
     old_space_->set_tasks(1);
   }
 
+  isolate()->safepoint_handler()->SafepointThreads(thread());
+
   if (writable_) {
-    thread()->heap()->WriteProtectCode(false);
+    heap_->WriteProtectCode(false);
   }
 }
 
 HeapIterationScope::~HeapIterationScope() {
   if (writable_) {
-    thread()->heap()->WriteProtectCode(true);
+    heap_->WriteProtectCode(true);
   }
 
+  isolate()->safepoint_handler()->ResumeThreads(thread());
+
   MonitorLocker ml(old_space_->tasks_lock());
 #if defined(DEBUG)
   ASSERT(old_space_->iterating_thread_ == thread());
@@ -232,21 +238,37 @@ HeapIterationScope::~HeapIterationScope() {
   ml.NotifyAll();
 }
 
-void Heap::IterateObjects(ObjectVisitor* visitor) const {
-  // The visitor must not allocate from the heap.
-  NoSafepointScope no_safepoint_scope_;
-  new_space_.VisitObjects(visitor);
-  IterateOldObjects(visitor);
+void HeapIterationScope::IterateObjects(ObjectVisitor* visitor) const {
+  heap_->VisitObjects(visitor);
 }
 
-void Heap::IterateOldObjects(ObjectVisitor* visitor) const {
-  HeapIterationScope heap_iteration_scope;
-  old_space_.VisitObjects(visitor);
+void HeapIterationScope::IterateObjectsNoImagePages(
+    ObjectVisitor* visitor) const {
+  heap_->new_space()->VisitObjects(visitor);
+  heap_->old_space()->VisitObjectsNoImagePages(visitor);

[danunez, 2017/08/05 17:24:11]

I would just use old_space_ since we have initialized it in the constructor.

 }
 
-void Heap::IterateOldObjectsNoImagePages(ObjectVisitor* visitor) const {
-  HeapIterationScope heap_iteration_scope;
-  old_space_.VisitObjectsNoImagePages(visitor);
+void HeapIterationScope::IterateOldObjects(ObjectVisitor* visitor) const {
+  old_space_->VisitObjects(visitor);
+}
+
+void HeapIterationScope::IterateOldObjectsNoImagePages(
+    ObjectVisitor* visitor) const {
+  old_space_->VisitObjectsNoImagePages(visitor);
+}
+
+void HeapIterationScope::IterateVMIsolateObjects(ObjectVisitor* visitor) const {
+  Dart::vm_isolate()->heap()->VisitObjects(visitor);
+}
+
+void HeapIterationScope::IterateObjectPointers(ObjectPointerVisitor* visitor,
+                                               bool validate_frames) {
+  isolate()->VisitObjectPointers(visitor, validate_frames);
+}
+
+void HeapIterationScope::IterateStackPointers(ObjectPointerVisitor* visitor,
+                                              bool validate_frames) {
+  isolate()->VisitStackPointers(visitor, validate_frames);
 }
 
 void Heap::VisitObjectPointers(ObjectPointerVisitor* visitor) const {
@@ -263,7 +285,6 @@ RawInstructions* Heap::FindObjectInCodeSpace(FindObjectVisitor* visitor) const {
 }
 
 RawObject* Heap::FindOldObject(FindObjectVisitor* visitor) const {
-  HeapIterationScope heap_iteration_scope;
   return old_space_.FindObject(visitor, HeapPage::kData);
 }