Make sure that const-class linkage is preserved, try again.
This CL causes occasional test failures on the build servers
which we were not able to reproduce locally. So we add some
some additional debug output to help pinpoint the cause.
Bug: 30627598
Bug: 33231647
Test: m test-art-host
This reverts commit 171cf811a1cdf8b1cbc5151505d8630741ce4cf3.
Change-Id: Id56a3f0e86e8212fd547e09c61794401bff47fb0
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index f3aba97..216ec9e 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1949,13 +1949,36 @@
void Visit(ObjPtr<mirror::ClassLoader> class_loader)
REQUIRES_SHARED(Locks::classlinker_classes_lock_, Locks::mutator_lock_) OVERRIDE {
ClassTable* const class_table = class_loader->GetClassTable();
- if (!done_ && class_table != nullptr && !class_table->Visit(*visitor_)) {
- // If the visitor ClassTable returns false it means that we don't need to continue.
- done_ = true;
+ if (!done_ && class_table != nullptr) {
+ DefiningClassLoaderFilterVisitor visitor(class_loader, visitor_);
+ if (!class_table->Visit(visitor)) {
+ // If the visitor ClassTable returns false it means that we don't need to continue.
+ done_ = true;
+ }
}
}
private:
+ // Class visitor that limits the class visits from a ClassTable to the classes with
+ // the provided defining class loader. This filter is used to avoid multiple visits
+ // of the same class which can be recorded for multiple initiating class loaders.
+ class DefiningClassLoaderFilterVisitor : public ClassVisitor {
+ public:
+ DefiningClassLoaderFilterVisitor(ObjPtr<mirror::ClassLoader> defining_class_loader,
+ ClassVisitor* visitor)
+ : defining_class_loader_(defining_class_loader), visitor_(visitor) { }
+
+ bool operator()(ObjPtr<mirror::Class> klass) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (klass->GetClassLoader() != defining_class_loader_) {
+ return true;
+ }
+ return (*visitor_)(klass);
+ }
+
+ ObjPtr<mirror::ClassLoader> const defining_class_loader_;
+ ClassVisitor* const visitor_;
+ };
+
ClassVisitor* const visitor_;
// If done is true then we don't need to do any more visiting.
bool done_;
@@ -2540,56 +2563,109 @@
}
} else {
ScopedObjectAccessUnchecked soa(self);
- ObjPtr<mirror::Class> cp_klass;
- if (FindClassInBaseDexClassLoader(soa, self, descriptor, hash, class_loader, &cp_klass)) {
- // The chain was understood. So the value in cp_klass is either the class we were looking
- // for, or not found.
- if (cp_klass != nullptr) {
- return cp_klass.Ptr();
- }
- // TODO: We handle the boot classpath loader in FindClassInBaseDexClassLoader. Try to unify
- // this and the branch above. TODO: throw the right exception here.
+ ObjPtr<mirror::Class> result_ptr;
+ bool descriptor_equals;
+ bool known_hierarchy =
+ FindClassInBaseDexClassLoader(soa, self, descriptor, hash, class_loader, &result_ptr);
+ if (result_ptr != nullptr) {
+ // The chain was understood and we found the class. We still need to add the class to
+ // the class table to protect from racy programs that can try and redefine the path list
+ // which would change the Class<?> returned for subsequent evaluation of const-class.
+ DCHECK(known_hierarchy);
+ DCHECK(result_ptr->DescriptorEquals(descriptor));
+ descriptor_equals = true;
+ } else {
+ // Either the chain wasn't understood or the class wasn't found.
+ //
+ // If the chain was understood but we did not find the class, let the Java-side
+ // rediscover all this and throw the exception with the right stack trace. Note that
+ // the Java-side could still succeed for racy programs if another thread is actively
+ // modifying the class loader's path list.
- // We'll let the Java-side rediscover all this and throw the exception with the right stack
- // trace.
- }
-
- if (Runtime::Current()->IsAotCompiler()) {
- // Oops, compile-time, can't run actual class-loader code.
- ObjPtr<mirror::Throwable> pre_allocated = Runtime::Current()->GetPreAllocatedNoClassDefFoundError();
- self->SetException(pre_allocated);
- return nullptr;
- }
-
- ScopedLocalRef<jobject> class_loader_object(soa.Env(),
- soa.AddLocalReference<jobject>(class_loader.Get()));
- std::string class_name_string(DescriptorToDot(descriptor));
- ScopedLocalRef<jobject> result(soa.Env(), nullptr);
- {
- ScopedThreadStateChange tsc(self, kNative);
- ScopedLocalRef<jobject> class_name_object(soa.Env(),
- soa.Env()->NewStringUTF(class_name_string.c_str()));
- if (class_name_object.get() == nullptr) {
- DCHECK(self->IsExceptionPending()); // OOME.
+ if (Runtime::Current()->IsAotCompiler()) {
+ // Oops, compile-time, can't run actual class-loader code.
+ ObjPtr<mirror::Throwable> pre_allocated =
+ Runtime::Current()->GetPreAllocatedNoClassDefFoundError();
+ self->SetException(pre_allocated);
return nullptr;
}
- CHECK(class_loader_object.get() != nullptr);
- result.reset(soa.Env()->CallObjectMethod(class_loader_object.get(),
- WellKnownClasses::java_lang_ClassLoader_loadClass,
- class_name_object.get()));
+
+ ScopedLocalRef<jobject> class_loader_object(
+ soa.Env(), soa.AddLocalReference<jobject>(class_loader.Get()));
+ std::string class_name_string(DescriptorToDot(descriptor));
+ ScopedLocalRef<jobject> result(soa.Env(), nullptr);
+ {
+ ScopedThreadStateChange tsc(self, kNative);
+ ScopedLocalRef<jobject> class_name_object(
+ soa.Env(), soa.Env()->NewStringUTF(class_name_string.c_str()));
+ if (class_name_object.get() == nullptr) {
+ DCHECK(self->IsExceptionPending()); // OOME.
+ return nullptr;
+ }
+ CHECK(class_loader_object.get() != nullptr);
+ result.reset(soa.Env()->CallObjectMethod(class_loader_object.get(),
+ WellKnownClasses::java_lang_ClassLoader_loadClass,
+ class_name_object.get()));
+ }
+ if (self->IsExceptionPending()) {
+ // If the ClassLoader threw, pass that exception up.
+ // However, to comply with the RI behavior, first check if another thread succeeded.
+ result_ptr = LookupClass(self, descriptor, hash, class_loader.Get());
+ if (result_ptr != nullptr && !result_ptr->IsErroneous()) {
+ self->ClearException();
+ return EnsureResolved(self, descriptor, result_ptr);
+ }
+ return nullptr;
+ } else if (result.get() == nullptr) {
+ // broken loader - throw NPE to be compatible with Dalvik
+ ThrowNullPointerException(StringPrintf("ClassLoader.loadClass returned null for %s",
+ class_name_string.c_str()).c_str());
+ return nullptr;
+ }
+ result_ptr = soa.Decode<mirror::Class>(result.get());
+ // Check the name of the returned class.
+ descriptor_equals = result_ptr->DescriptorEquals(descriptor);
}
- if (self->IsExceptionPending()) {
- // If the ClassLoader threw, pass that exception up.
- return nullptr;
- } else if (result.get() == nullptr) {
- // broken loader - throw NPE to be compatible with Dalvik
- ThrowNullPointerException(StringPrintf("ClassLoader.loadClass returned null for %s",
- class_name_string.c_str()).c_str());
- return nullptr;
- } else {
- // success, return mirror::Class*
- return soa.Decode<mirror::Class>(result.get()).Ptr();
+
+ // Try to insert the class to the class table, checking for mismatch.
+ ObjPtr<mirror::Class> old;
+ {
+ ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_);
+ ClassTable* const class_table = InsertClassTableForClassLoader(class_loader.Get());
+ old = class_table->Lookup(descriptor, hash);
+ if (old == nullptr) {
+ old = result_ptr; // For the comparison below, after releasing the lock.
+ if (descriptor_equals) {
+ class_table->InsertWithHash(result_ptr.Ptr(), hash);
+ Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(class_loader.Get());
+ } // else throw below, after releasing the lock.
+ }
}
+ if (UNLIKELY(old != result_ptr)) {
+ // Return `old` (even if `!descriptor_equals`) to mimic the RI behavior for parallel
+ // capable class loaders. (All class loaders are considered parallel capable on Android.)
+ mirror::Class* loader_class = class_loader->GetClass();
+ const char* loader_class_name =
+ loader_class->GetDexFile().StringByTypeIdx(loader_class->GetDexTypeIndex());
+ LOG(WARNING) << "Initiating class loader of type " << DescriptorToDot(loader_class_name)
+ << " is not well-behaved; it returned a different Class for racing loadClass(\""
+ << DescriptorToDot(descriptor) << "\").";
+ return EnsureResolved(self, descriptor, old);
+ }
+ if (UNLIKELY(!descriptor_equals)) {
+ std::string result_storage;
+ const char* result_name = result_ptr->GetDescriptor(&result_storage);
+ std::string loader_storage;
+ const char* loader_class_name = class_loader->GetClass()->GetDescriptor(&loader_storage);
+ ThrowNoClassDefFoundError(
+ "Initiating class loader of type %s returned class %s instead of %s.",
+ DescriptorToDot(loader_class_name).c_str(),
+ DescriptorToDot(result_name).c_str(),
+ DescriptorToDot(descriptor).c_str());
+ return nullptr;
+ }
+ // success, return mirror::Class*
+ return result_ptr.Ptr();
}
UNREACHABLE();
}
@@ -3670,12 +3746,6 @@
Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(klass);
}
-bool ClassLinker::RemoveClass(const char* descriptor, ObjPtr<mirror::ClassLoader> class_loader) {
- WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
- ClassTable* const class_table = ClassTableForClassLoader(class_loader);
- return class_table != nullptr && class_table->Remove(descriptor);
-}
-
mirror::Class* ClassLinker::LookupClass(Thread* self,
const char* descriptor,
size_t hash,
@@ -3726,7 +3796,8 @@
REQUIRES_SHARED(Locks::classlinker_classes_lock_, Locks::mutator_lock_) OVERRIDE {
ClassTable* const class_table = class_loader->GetClassTable();
ObjPtr<mirror::Class> klass = class_table->Lookup(descriptor_, hash_);
- if (klass != nullptr) {
+ // Add `klass` only if `class_loader` is its defining (not just initiating) class loader.
+ if (klass != nullptr && klass->GetClassLoader() == class_loader) {
result_->push_back(klass);
}
}
@@ -3745,6 +3816,7 @@
const size_t hash = ComputeModifiedUtf8Hash(descriptor);
ObjPtr<mirror::Class> klass = boot_class_table_.Lookup(descriptor, hash);
if (klass != nullptr) {
+ DCHECK(klass->GetClassLoader() == nullptr);
result.push_back(klass);
}
LookupClassesVisitor visitor(descriptor, hash, &result);
@@ -8052,8 +8124,8 @@
REQUIRES_SHARED(Locks::classlinker_classes_lock_, Locks::mutator_lock_) OVERRIDE {
ClassTable* const class_table = class_loader->GetClassTable();
if (class_table != nullptr) {
- num_zygote_classes += class_table->NumZygoteClasses();
- num_non_zygote_classes += class_table->NumNonZygoteClasses();
+ num_zygote_classes += class_table->NumZygoteClasses(class_loader);
+ num_non_zygote_classes += class_table->NumNonZygoteClasses(class_loader);
}
}
@@ -8064,13 +8136,13 @@
size_t ClassLinker::NumZygoteClasses() const {
CountClassesVisitor visitor;
VisitClassLoaders(&visitor);
- return visitor.num_zygote_classes + boot_class_table_.NumZygoteClasses();
+ return visitor.num_zygote_classes + boot_class_table_.NumZygoteClasses(nullptr);
}
size_t ClassLinker::NumNonZygoteClasses() const {
CountClassesVisitor visitor;
VisitClassLoaders(&visitor);
- return visitor.num_non_zygote_classes + boot_class_table_.NumNonZygoteClasses();
+ return visitor.num_non_zygote_classes + boot_class_table_.NumNonZygoteClasses(nullptr);
}
size_t ClassLinker::NumLoadedClasses() {