commit e2d628b5b0a1b9c29c173f3cbad3ef6cb6c24d2d
author Sebastien Hertz <shertz@google.com> Thu Oct 23 15:39:33 2014 +0200
committer Sebastien Hertz <shertz@google.com> Thu Oct 23 16:31:58 2014 +0200
tree b1fa6ef18e26a5bad38dde6735b673a54194a6e8
parent 8fc5acfd382bdc0d7920e8a13439b64344a8988a

Make ObjectRegistry::InternalAdd GC safe

Because a call to IdentityHashCode may cause GC, the object pointer
may become invalid (if the object has been moved) on next uses. We
now access the object through a Handle to be GC safe.

Also remove unused methods.

Bug: 18098424
Change-Id: I38fb55c3a6be62c4d98d4c94272a9cfeba327598

runtime/jdwp/object_registry.cc

diff --git a/runtime/jdwp/object_registry.cc b/runtime/jdwp/object_registry.cc
index 35aaf0a..4f34896 100644
--- a/runtime/jdwp/object_registry.cc
+++ b/runtime/jdwp/object_registry.cc
@@ -16,6 +16,7 @@
 
 #include "object_registry.h"
 
+#include "handle_scope-inl.h"
 #include "mirror/class.h"
 #include "scoped_thread_state_change.h"
 
@@ -46,12 +47,17 @@
     return 0;
   }
 
+  Thread* const self = Thread::Current();
+  StackHandleScope<1> hs(self);
+  Handle<mirror::Object> obj_h(hs.NewHandle(o));
+
   // Call IdentityHashCode here to avoid a lock level violation between lock_ and monitor_lock.
-  int32_t identity_hash_code = o->IdentityHashCode();
-  ScopedObjectAccessUnchecked soa(Thread::Current());
+  int32_t identity_hash_code = obj_h->IdentityHashCode();
+
+  ScopedObjectAccessUnchecked soa(self);
   MutexLock mu(soa.Self(), lock_);
   ObjectRegistryEntry* entry = nullptr;
-  if (ContainsLocked(soa.Self(), o, identity_hash_code, &entry)) {
+  if (ContainsLocked(soa.Self(), obj_h.Get(), identity_hash_code, &entry)) {
     // This object was already in our map.
     ++entry->reference_count;
   } else {
@@ -66,7 +72,7 @@
     // This object isn't in the registry yet, so add it.
     JNIEnv* env = soa.Env();
 
-    jobject local_reference = soa.AddLocalReference<jobject>(o);
+    jobject local_reference = soa.AddLocalReference<jobject>(obj_h.Get());
 
     entry->jni_reference_type = JNIWeakGlobalRefType;
     entry->jni_reference = env->NewWeakGlobalRef(local_reference);
@@ -80,17 +86,6 @@
   return entry->id;
 }
 
-bool ObjectRegistry::Contains(mirror::Object* o, ObjectRegistryEntry** out_entry) {
-  if (o == nullptr) {
-    return false;
-  }
-  // Call IdentityHashCode here to avoid a lock level violation between lock_ and monitor_lock.
-  int32_t identity_hash_code = o->IdentityHashCode();
-  Thread* self = Thread::Current();
-  MutexLock mu(self, lock_);
-  return ContainsLocked(self, o, identity_hash_code, out_entry);
-}
-
 bool ObjectRegistry::ContainsLocked(Thread* self, mirror::Object* o, int32_t identity_hash_code,
                                     ObjectRegistryEntry** out_entry) {
   DCHECK(o != nullptr);

runtime/jdwp/object_registry.h

diff --git a/runtime/jdwp/object_registry.h b/runtime/jdwp/object_registry.h
index faddff1..0693f33 100644
--- a/runtime/jdwp/object_registry.h
+++ b/runtime/jdwp/object_registry.h
@@ -75,10 +75,6 @@
     return down_cast<T>(InternalGet(id, error));
   }
 
-  bool Contains(mirror::Object* o) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-    return Contains(o, nullptr);
-  }
-
   void Clear() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void DisableCollection(JDWP::ObjectId id)
@@ -114,9 +110,6 @@
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
-  bool Contains(mirror::Object* o, ObjectRegistryEntry** out_entry)
-      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(lock_);
-
   bool ContainsLocked(Thread* self, mirror::Object* o, int32_t identity_hash_code,
                       ObjectRegistryEntry** out_entry)
       EXCLUSIVE_LOCKS_REQUIRED(lock_) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);