Ensure that all redefinition created dex file get on classpath
We were not adding some DexFiles to the ClassPath if there were
multiple classes from the same classloader being redefined at the same
time. We fixed this issue and made a test for it.
Test: mma -j40 test-art-host
Change-Id: I6e8961c8602367ebec5d5a948d71e58f3be2f6d7
diff --git a/runtime/openjdkjvmti/ti_class_loader.cc b/runtime/openjdkjvmti/ti_class_loader.cc
index c2f1792..afec0bf 100644
--- a/runtime/openjdkjvmti/ti_class_loader.cc
+++ b/runtime/openjdkjvmti/ti_class_loader.cc
@@ -62,7 +62,7 @@
art::Handle<art::mirror::ClassLoader> loader,
const art::DexFile* dex_file) {
art::ScopedObjectAccessUnchecked soa(self);
- art::StackHandleScope<2> hs(self);
+ art::StackHandleScope<3> hs(self);
if (art::ClassLinker::IsBootClassLoader(soa, loader.Get())) {
art::Runtime::Current()->GetClassLinker()->AppendToBootClassPath(self, *dex_file);
return true;
@@ -72,8 +72,9 @@
if (java_dex_file_obj.IsNull()) {
return false;
}
+ art::Handle<art::mirror::LongArray> old_cookie(hs.NewHandle(GetDexFileCookie(java_dex_file_obj)));
art::Handle<art::mirror::LongArray> cookie(hs.NewHandle(
- AllocateNewDexFileCookie(self, java_dex_file_obj, dex_file)));
+ AllocateNewDexFileCookie(self, old_cookie, dex_file)));
if (cookie.IsNull()) {
return false;
}
@@ -99,12 +100,8 @@
}
}
-// TODO Really wishing I had that mirror of java.lang.DexFile now.
-art::ObjPtr<art::mirror::LongArray> ClassLoaderHelper::AllocateNewDexFileCookie(
- art::Thread* self,
- art::Handle<art::mirror::Object> java_dex_file_obj,
- const art::DexFile* dex_file) {
- art::StackHandleScope<2> hs(self);
+art::ObjPtr<art::mirror::LongArray> ClassLoaderHelper::GetDexFileCookie(
+ art::Handle<art::mirror::Object> java_dex_file_obj) {
// mCookie is nulled out if the DexFile has been closed but mInternalCookie sticks around until
// the object is finalized. Since they always point to the same array if mCookie is not null we
// just use the mInternalCookie field. We will update one or both of these fields later.
@@ -113,9 +110,15 @@
"mInternalCookie", "Ljava/lang/Object;");
// TODO Add check that mCookie is either null or same as mInternalCookie
CHECK(internal_cookie_field != nullptr);
- art::Handle<art::mirror::LongArray> cookie(
- hs.NewHandle(internal_cookie_field->GetObject(java_dex_file_obj.Get())->AsLongArray()));
- // TODO Maybe make these non-fatal.
+ return internal_cookie_field->GetObject(java_dex_file_obj.Get())->AsLongArray();
+}
+
+// TODO Really wishing I had that mirror of java.lang.DexFile now.
+art::ObjPtr<art::mirror::LongArray> ClassLoaderHelper::AllocateNewDexFileCookie(
+ art::Thread* self,
+ art::Handle<art::mirror::LongArray> cookie,
+ const art::DexFile* dex_file) {
+ art::StackHandleScope<1> hs(self);
CHECK(cookie.Get() != nullptr);
CHECK_GE(cookie->GetLength(), 1);
art::Handle<art::mirror::LongArray> new_cookie(
@@ -128,8 +131,9 @@
// TODO Should I clear this field?
// TODO This is a really crappy thing here with the first element being different.
new_cookie->SetWithoutChecks<false>(0, cookie->GetWithoutChecks(0));
+ // This must match the casts in runtime/native/dalvik_system_DexFile.cc:ConvertDexFilesToJavaArray
new_cookie->SetWithoutChecks<false>(
- 1, static_cast<int64_t>(reinterpret_cast<intptr_t>(dex_file)));
+ 1, static_cast<int64_t>(reinterpret_cast<uintptr_t>(dex_file)));
new_cookie->Memcpy(2, cookie.Get(), 1, cookie->GetLength() - 1);
return new_cookie.Get();
}
diff --git a/runtime/openjdkjvmti/ti_class_loader.h b/runtime/openjdkjvmti/ti_class_loader.h
index 17ed0eb..1ac4988 100644
--- a/runtime/openjdkjvmti/ti_class_loader.h
+++ b/runtime/openjdkjvmti/ti_class_loader.h
@@ -82,9 +82,12 @@
art::Thread* self, art::Handle<art::mirror::ClassLoader> loader)
REQUIRES_SHARED(art::Locks::mutator_lock_);
+ static art::ObjPtr<art::mirror::LongArray> GetDexFileCookie(
+ art::Handle<art::mirror::Object> java_dex_file) REQUIRES_SHARED(art::Locks::mutator_lock_);
+
static art::ObjPtr<art::mirror::LongArray> AllocateNewDexFileCookie(
art::Thread* self,
- art::Handle<art::mirror::Object> java_dex_file,
+ art::Handle<art::mirror::LongArray> old_dex_file_cookie,
const art::DexFile* new_dex_file) REQUIRES_SHARED(art::Locks::mutator_lock_);
static void UpdateJavaDexFile(art::ObjPtr<art::mirror::Object> java_dex_file,
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index 4b8108a..eb4c2f9 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -701,6 +701,60 @@
DISALLOW_COPY_AND_ASSIGN(RedefinitionDataHolder);
};
+// Looks through the previously allocated cookies to see if we need to update them with another new
+// dexfile. This is so that even if multiple classes with the same classloader are redefined at
+// once they are all added to the classloader.
+bool Redefiner::ClassRedefinition::AllocateAndRememberNewDexFileCookie(
+ int32_t klass_index,
+ art::Handle<art::mirror::ClassLoader> source_class_loader,
+ art::Handle<art::mirror::Object> dex_file_obj,
+ /*out*/RedefinitionDataHolder* holder) {
+ art::StackHandleScope<2> hs(driver_->self_);
+ art::MutableHandle<art::mirror::LongArray> old_cookie(
+ hs.NewHandle<art::mirror::LongArray>(nullptr));
+ bool has_older_cookie = false;
+ // See if we already have a cookie that a previous redefinition got from the same classloader.
+ for (int32_t i = 0; i < klass_index; i++) {
+ if (holder->GetSourceClassLoader(i) == source_class_loader.Get()) {
+ // Since every instance of this classloader should have the same cookie associated with it we
+ // can stop looking here.
+ has_older_cookie = true;
+ old_cookie.Assign(holder->GetNewDexFileCookie(i));
+ break;
+ }
+ }
+ if (old_cookie.IsNull()) {
+ // No older cookie. Get it directly from the dex_file_obj
+ // We should not have seen this classloader elsewhere.
+ CHECK(!has_older_cookie);
+ old_cookie.Assign(ClassLoaderHelper::GetDexFileCookie(dex_file_obj));
+ }
+ // Use the old cookie to generate the new one with the new DexFile* added in.
+ art::Handle<art::mirror::LongArray>
+ new_cookie(hs.NewHandle(ClassLoaderHelper::AllocateNewDexFileCookie(driver_->self_,
+ old_cookie,
+ dex_file_.get())));
+ // Make sure the allocation worked.
+ if (new_cookie.IsNull()) {
+ return false;
+ }
+
+ // Save the cookie.
+ holder->SetNewDexFileCookie(klass_index, new_cookie.Get());
+ // If there are other copies of this same classloader we need to make sure that we all have the
+ // same cookie.
+ if (has_older_cookie) {
+ for (int32_t i = 0; i < klass_index; i++) {
+ // We will let the GC take care of the cookie we allocated for this one.
+ if (holder->GetSourceClassLoader(i) == source_class_loader.Get()) {
+ holder->SetNewDexFileCookie(i, new_cookie.Get());
+ }
+ }
+ }
+
+ return true;
+}
+
bool Redefiner::ClassRedefinition::FinishRemainingAllocations(
int32_t klass_index, /*out*/RedefinitionDataHolder* holder) {
art::ScopedObjectAccessUnchecked soa(driver_->self_);
@@ -719,11 +773,8 @@
RecordFailure(ERR(INTERNAL), "Unable to find dex file!");
return false;
}
- holder->SetNewDexFileCookie(klass_index,
- ClassLoaderHelper::AllocateNewDexFileCookie(driver_->self_,
- dex_file_obj,
- dex_file_.get()).Ptr());
- if (holder->GetNewDexFileCookie(klass_index) == nullptr) {
+ // Allocate the new dex file cookie.
+ if (!AllocateAndRememberNewDexFileCookie(klass_index, loader, dex_file_obj, holder)) {
driver_->self_->AssertPendingOOMException();
driver_->self_->ClearException();
RecordFailure(ERR(OUT_OF_MEMORY), "Unable to allocate dex file array for class loader");
diff --git a/runtime/openjdkjvmti/ti_redefine.h b/runtime/openjdkjvmti/ti_redefine.h
index 5bcaef8..5aa7dde 100644
--- a/runtime/openjdkjvmti/ti_redefine.h
+++ b/runtime/openjdkjvmti/ti_redefine.h
@@ -145,6 +145,13 @@
bool FinishRemainingAllocations(int32_t klass_index, /*out*/RedefinitionDataHolder* holder)
REQUIRES_SHARED(art::Locks::mutator_lock_);
+ bool AllocateAndRememberNewDexFileCookie(
+ int32_t klass_index,
+ art::Handle<art::mirror::ClassLoader> source_class_loader,
+ art::Handle<art::mirror::Object> dex_file_obj,
+ /*out*/RedefinitionDataHolder* holder)
+ REQUIRES_SHARED(art::Locks::mutator_lock_);
+
void FindAndAllocateObsoleteMethods(art::mirror::Class* art_klass)
REQUIRES(art::Locks::mutator_lock_);