commit a9daa3a26900cd0f0f9ee7dab72980fc990adda6
author Mathieu Chartier <mathieuc@google.com> Thu Oct 13 01:31:19 2016 +0000
committer Gerrit Code Review <noreply-gerritcodereview@google.com> Thu Oct 13 01:31:19 2016 +0000
tree a3e268848716edcf92d34cd1f19470fe7bb499f3
parent 834a448aa91a44c9076d3f7dc92b45f550d96285
parent 51eaa7f5547f9ddea92ce57c6c2df9534d5017b8

Merge "Fix moving GC bug in cloning finalizable objects"

runtime/mirror/class.cc

diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 7606915..689dd22 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1002,7 +1002,7 @@
       REQUIRES_SHARED(Locks::mutator_lock_) {
     StackHandleScope<1> hs(self_);
     Handle<mirror::Class> h_new_class_obj(hs.NewHandle(obj->AsClass()));
-    mirror::Object::CopyObject(self_, h_new_class_obj.Get(), orig_->Get(), copy_bytes_);
+    mirror::Object::CopyObject(h_new_class_obj.Get(), orig_->Get(), copy_bytes_);
     mirror::Class::SetStatus(h_new_class_obj, Class::kStatusResolving, self_);
     h_new_class_obj->PopulateEmbeddedVTable(pointer_size_);
     h_new_class_obj->SetImt(imt_, pointer_size_);

runtime/mirror/object.cc

diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index dbfe1d9..daee727 100644
--- a/runtime/mirror/object.cc
+++ b/runtime/mirror/object.cc
@@ -72,8 +72,7 @@
   ObjPtr<Object> const dest_obj_;
 };
 
-Object* Object::CopyObject(Thread* self,
-                           ObjPtr<mirror::Object> dest,
+Object* Object::CopyObject(ObjPtr<mirror::Object> dest,
                            ObjPtr<mirror::Object> src,
                            size_t num_bytes) {
   // Copy instance data.  Don't assume memcpy copies by words (b/32012820).
@@ -128,26 +127,21 @@
   } else {
     heap->WriteBarrierEveryFieldOf(dest);
   }
-  if (c->IsFinalizable()) {
-    heap->AddFinalizerReference(self, &dest);
-  }
   return dest.Ptr();
 }
 
 // An allocation pre-fence visitor that copies the object.
 class CopyObjectVisitor {
  public:
-  CopyObjectVisitor(Thread* self, Handle<Object>* orig, size_t num_bytes)
-      : self_(self), orig_(orig), num_bytes_(num_bytes) {
-  }
+  CopyObjectVisitor(Handle<Object>* orig, size_t num_bytes)
+      : orig_(orig), num_bytes_(num_bytes) {}
 
   void operator()(ObjPtr<Object> obj, size_t usable_size ATTRIBUTE_UNUSED) const
       REQUIRES_SHARED(Locks::mutator_lock_) {
-    Object::CopyObject(self_, obj, orig_->Get(), num_bytes_);
+    Object::CopyObject(obj, orig_->Get(), num_bytes_);
   }
 
  private:
-  Thread* const self_;
   Handle<Object>* const orig_;
   const size_t num_bytes_;
   DISALLOW_COPY_AND_ASSIGN(CopyObjectVisitor);
@@ -162,12 +156,15 @@
   StackHandleScope<1> hs(self);
   Handle<Object> this_object(hs.NewHandle(this));
   ObjPtr<Object> copy;
-  CopyObjectVisitor visitor(self, &this_object, num_bytes);
+  CopyObjectVisitor visitor(&this_object, num_bytes);
   if (heap->IsMovableObject(this)) {
     copy = heap->AllocObject<true>(self, GetClass(), num_bytes, visitor);
   } else {
     copy = heap->AllocNonMovableObject<true>(self, GetClass(), num_bytes, visitor);
   }
+  if (this_object->GetClass()->IsFinalizable()) {
+    heap->AddFinalizerReference(self, &copy);
+  }
   return copy.Ptr();
 }
 

runtime/mirror/object.h

diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index 175b0c3..84aa96c 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -609,11 +609,10 @@
     }
   }
 
-  // A utility function that copies an object in a read barrier and
-  // write barrier-aware way. This is internally used by Clone() and
-  // Class::CopyOf().
-  static Object* CopyObject(Thread* self,
-                            ObjPtr<mirror::Object> dest,
+  // A utility function that copies an object in a read barrier and write barrier-aware way.
+  // This is internally used by Clone() and Class::CopyOf(). If the object is finalizable,
+  // it is the callers job to call Heap::AddFinalizerReference.
+  static Object* CopyObject(ObjPtr<mirror::Object> dest,
                             ObjPtr<mirror::Object> src,
                             size_t num_bytes)
       REQUIRES_SHARED(Locks::mutator_lock_);