commit 2d8559517759dcfac749a28d6713d36809ec556f
author Mathieu Chartier <mathieuc@google.com> Wed Oct 12 19:37:59 2016 -0700
committer Mathieu Chartier <mathieuc@google.com> Wed Oct 12 20:34:22 2016 -0700
tree 5fb2530b8c7f181dfc073430926bc02c4217db9a
parent 000432d9a4f828ed6736d2003a5d78de5484a384

Move Heap::GetInstances to use Handles

Prevent moving GC bugs from VisitObjects. Fix JDWP tests.

Test: art/tools/run-jdwp-tests.sh '--mode=host' '--variant=X32' --debug

Bug: 31113334
Change-Id: Ie7f51f1980ec0c1eddc3b59c3e49564eacb3be85

runtime/debugger.cc

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 073750e..ada1a23 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -918,11 +918,11 @@
   if (c == nullptr) {
     return error;
   }
-  std::vector<ObjPtr<mirror::Object>> raw_instances;
-  StackHandleScope<1> hs(Thread::Current());
-  Runtime::Current()->GetHeap()->GetInstances(hs.NewHandle(c), max_count, raw_instances);
+  VariableSizedHandleScope hs(Thread::Current());
+  std::vector<Handle<mirror::Object>> raw_instances;
+  Runtime::Current()->GetHeap()->GetInstances(hs, hs.NewHandle(c), max_count, raw_instances);
   for (size_t i = 0; i < raw_instances.size(); ++i) {
-    instances->push_back(gRegistry->Add(raw_instances[i]));
+    instances->push_back(gRegistry->Add(raw_instances[i].Get()));
   }
   return JDWP::ERR_NONE;
 }

runtime/gc/heap.cc

diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index ddb5be1..db90a2a 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1923,11 +1923,15 @@
 
 class InstanceCollector {
  public:
-  InstanceCollector(Handle<mirror::Class> c,
+  InstanceCollector(VariableSizedHandleScope& scope,
+                    Handle<mirror::Class> c,
                     int32_t max_count,
-                    std::vector<ObjPtr<mirror::Object>>& instances)
+                    std::vector<Handle<mirror::Object>>& instances)
       REQUIRES_SHARED(Locks::mutator_lock_)
-      : class_(c), max_count_(max_count), instances_(instances) {}
+      : scope_(scope),
+        class_(c),
+        max_count_(max_count),
+        instances_(instances) {}
 
   static void Callback(mirror::Object* obj, void* arg)
       REQUIRES_SHARED(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
@@ -1936,22 +1940,24 @@
     if (obj->GetClass() == instance_collector->class_.Get()) {
       if (instance_collector->max_count_ == 0 ||
           instance_collector->instances_.size() < instance_collector->max_count_) {
-        instance_collector->instances_.push_back(obj);
+        instance_collector->instances_.push_back(instance_collector->scope_.NewHandle(obj));
       }
     }
   }
 
  private:
+  VariableSizedHandleScope& scope_;
   Handle<mirror::Class> const class_;
   const uint32_t max_count_;
-  std::vector<ObjPtr<mirror::Object>>& instances_;
+  std::vector<Handle<mirror::Object>>& instances_;
   DISALLOW_COPY_AND_ASSIGN(InstanceCollector);
 };
 
-void Heap::GetInstances(Handle<mirror::Class> c,
+void Heap::GetInstances(VariableSizedHandleScope& scope,
+                        Handle<mirror::Class> c,
                         int32_t max_count,
-                        std::vector<ObjPtr<mirror::Object>>& instances) {
-  InstanceCollector collector(c, max_count, instances);
+                        std::vector<Handle<mirror::Object>>& instances) {
+  InstanceCollector collector(scope, c, max_count, instances);
   VisitObjects(&InstanceCollector::Callback, &collector);
 }
 

runtime/gc/heap.h

diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h
index 796b51d..6d37140 100644
--- a/runtime/gc/heap.h
+++ b/runtime/gc/heap.h
@@ -342,9 +342,10 @@
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Implements JDWP RT_Instances.
-  void GetInstances(Handle<mirror::Class> c,
+  void GetInstances(VariableSizedHandleScope& scope,
+                    Handle<mirror::Class> c,
                     int32_t max_count,
-                    std::vector<ObjPtr<mirror::Object>>& instances)
+                    std::vector<Handle<mirror::Object>>& instances)
       REQUIRES(!Locks::heap_bitmap_lock_, !*gc_complete_lock_)
       REQUIRES_SHARED(Locks::mutator_lock_);