Safer interface for heap iteration.

runtime/vm/heap.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/heap.h"
 
 #include "platform/assert.h"
 #include "platform/utils.h"
 #include "vm/flags.h"
 #include "vm/isolate.h"
@@ skipping 201 matching lines @@
   return old_space_->Contains(addr, HeapPage::kExecutable);
 }
 
 
 void Heap::VisitObjects(ObjectVisitor* visitor) const {
   new_space_->VisitObjects(visitor);
   old_space_->VisitObjects(visitor);
 }
 
 
+HeapIterationScope::HeapIterationScope()
+    : StackResource(Thread::Current()->isolate()),
+      old_space_(isolate()->heap()->old_space()) {
+  // It's not yet safe to iterate over a paged space while it's concurrently
+  // sweeping, so wait for any such task to complete first.
+  MonitorLocker ml(old_space_->tasks_lock());
+  while (old_space_->tasks() > 0) {
+    ml.Wait();
+  }
+  old_space_->set_tasks(1);

[siva, 2015/07/07 18:10:24]

How do we ensure that people don't end up with nested HeapIterationScopes? Maybe
we need an assertion to prevent that.

[koda, 2015/07/07 22:46:39]

Done.

+}
+
+
+HeapIterationScope::~HeapIterationScope() {
+  old_space_->set_tasks(0);

[siva, 2015/07/07 18:10:24]

Doesn't this need to be set under a lock as well?

MonitorLocker ml(old_space_->tasks_lock());
ASSERT(old_space_->tasks() == 1);
old_space_->set_tasks(0);
ml.notify();

[koda, 2015/07/07 22:46:39]

Done.

+}
+
+
+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 Heap::IterateOldObjects(ObjectVisitor* visitor) const {
+  HeapIterationScope heap_iteration_scope;
+  old_space_->VisitObjects(visitor);
+}
+
+
 void Heap::VisitObjectPointers(ObjectPointerVisitor* visitor) const {
   new_space_->VisitObjectPointers(visitor);
   old_space_->VisitObjectPointers(visitor);
 }
 
 
 RawInstructions* Heap::FindObjectInCodeSpace(FindObjectVisitor* visitor) const {
   // Only executable pages can have RawInstructions objects.
   RawObject* raw_obj = old_space_->FindObject(visitor, HeapPage::kExecutable);
   ASSERT((raw_obj == Object::null()) ||
          (raw_obj->GetClassId() == kInstructionsCid));
   return reinterpret_cast<RawInstructions*>(raw_obj);
 }
 
 
 RawObject* Heap::FindOldObject(FindObjectVisitor* visitor) const {
-  // Wait for any concurrent GC tasks to finish before walking.
-  MonitorLocker ml(old_space_->tasks_lock());
-  while (old_space_->tasks() > 0) {
-    ml.Wait();
-  }
+  HeapIterationScope heap_iteration_scope;
   return old_space_->FindObject(visitor, HeapPage::kData);
 }
 
 
 RawObject* Heap::FindNewObject(FindObjectVisitor* visitor) const {
   return new_space_->FindObject(visitor);
 }
 
 
 RawObject* Heap::FindObject(FindObjectVisitor* visitor) const {
-  ASSERT(isolate()->no_safepoint_scope_depth() != 0);
+  // The visitor must not allocate from the heap.
+  NoSafepointScope no_safepoint_scope;
   RawObject* raw_obj = FindNewObject(visitor);
   if (raw_obj != Object::null()) {
     return raw_obj;
   }
   raw_obj = FindOldObject(visitor);
   if (raw_obj != Object::null()) {
     return raw_obj;
   }
   raw_obj = FindObjectInCodeSpace(visitor);
   return raw_obj;
@@ skipping 218 matching lines @@
     // VM isolate heap is premarked.
     VerifyObjectVisitor vm_object_visitor(
         isolate(), allocated_set, kRequireMarked);
     vm_isolate->heap()->VisitObjects(&vm_object_visitor);
   }
   return allocated_set;
 }
 
 
 bool Heap::Verify(MarkExpectation mark_expectation) const {
+  HeapIterationScope heap_iteration_scope;
+  return VerifyGC(mark_expectation);
+}
+
+
+bool Heap::VerifyGC(MarkExpectation mark_expectation) const {
   ObjectSet* allocated_set = CreateAllocatedObjectSet(mark_expectation);
   VerifyPointersVisitor visitor(isolate(), allocated_set);
   VisitObjectPointers(&visitor);
   delete allocated_set;
   // Only returning a value so that Heap::Validate can be called from an ASSERT.
   return true;
 }
 
 
 void Heap::PrintSizes() const {
@@ skipping 212 matching lines @@
     heap->DisableGrowthControl();
 }
 
 
 NoHeapGrowthControlScope::~NoHeapGrowthControlScope() {
     Heap* heap = reinterpret_cast<Isolate*>(isolate())->heap();
     heap->SetGrowthControlState(current_growth_controller_state_);
 }
 
 }  // namespace dart