Add class table field to class loader
Fixes bug with the class table where the comparator would cause read
barriers and break the map strict ordering properties.
Bug: 22957957
Change-Id: I8dbc042db6e22e2172ab4ec58ddf1db0345dcaaa
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index b85a129..4ce3129 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -1372,6 +1372,11 @@
<< "Missing relocation for AbstractMethod.artMethod " << PrettyMethod(src_method);
dest->SetArtMethod(
reinterpret_cast<ArtMethod*>(image_begin_ + it->second.offset));
+ } else if (!klass->IsArrayClass() && klass->IsSubClass(down_cast<mirror::Class*>(
+ Thread::Current()->DecodeJObject(WellKnownClasses::java_lang_ClassLoader)))) {
+ // If src is a ClassLoader, set the class table to null so that it gets recreated by the
+ // ClassLoader.
+ down_cast<mirror::ClassLoader*>(copy)->SetClassTable(nullptr);
}
FixupVisitor visitor(this, copy);
orig->VisitReferences<true /*visit class*/>(visitor, visitor);
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index a306c30..7821da3 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1273,23 +1273,16 @@
// Moving concurrent:
// Need to make sure to not copy ArtMethods without doing read barriers since the roots are
// marked concurrently and we don't hold the classlinker_classes_lock_ when we do the copy.
- std::vector<std::pair<GcRoot<mirror::ClassLoader>, ClassTable*>> reinsert;
- for (auto it = classes_.begin(); it != classes_.end(); ) {
- it->second->VisitRoots(visitor, flags);
- const GcRoot<mirror::ClassLoader>& root = it->first;
- mirror::ClassLoader* old_ref = root.Read<kWithoutReadBarrier>();
- root.VisitRootIfNonNull(visitor, RootInfo(kRootVMInternal));
- mirror::ClassLoader* new_ref = root.Read<kWithoutReadBarrier>();
- if (new_ref != old_ref) {
- reinsert.push_back(*it);
- it = classes_.erase(it);
- } else {
- ++it;
+ boot_class_table_.VisitRoots(visitor, flags);
+ for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
+ // May be null for boot ClassLoader.
+ root.VisitRoot(visitor, RootInfo(kRootVMInternal));
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ if (class_table != nullptr) {
+ // May be null if we have no classes.
+ class_table->VisitRoots(visitor, flags);
}
}
- for (auto& pair : reinsert) {
- classes_.Put(pair.first, pair.second);
- }
} else if ((flags & kVisitRootFlagNewRoots) != 0) {
for (auto& root : new_class_roots_) {
mirror::Class* old_ref = root.Read<kWithoutReadBarrier>();
@@ -1346,10 +1339,12 @@
}
void ClassLinker::VisitClassesInternal(ClassVisitor* visitor) {
- for (auto& pair : classes_) {
- ClassTable* const class_table = pair.second;
- if (!class_table->Visit(visitor)) {
- return;
+ if (boot_class_table_.Visit(visitor)) {
+ for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ if (class_table != nullptr && !class_table->Visit(visitor)) {
+ return;
+ }
}
}
}
@@ -1469,7 +1464,10 @@
mirror::LongArray::ResetArrayClass();
mirror::ShortArray::ResetArrayClass();
STLDeleteElements(&oat_files_);
- STLDeleteValues(&classes_);
+ for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ delete class_table;
+ }
}
mirror::PointerArray* ClassLinker::AllocPointerArray(Thread* self, size_t length) {
@@ -2907,8 +2905,12 @@
void ClassLinker::MoveClassTableToPreZygote() {
WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
- for (auto& class_table : classes_) {
- class_table.second->FreezeSnapshot();
+ boot_class_table_.FreezeSnapshot();
+ for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ if (class_table != nullptr) {
+ class_table->FreezeSnapshot();
+ }
}
}
@@ -2941,10 +2943,15 @@
MoveImageClassesToClassTable();
}
WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
- for (auto& pair : classes_) {
+ const size_t hash = ComputeModifiedUtf8Hash(descriptor);
+ mirror::Class* klass = boot_class_table_.Lookup(descriptor, hash);
+ if (klass != nullptr) {
+ result.push_back(klass);
+ }
+ for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
// There can only be one class with the same descriptor per class loader.
- ClassTable* const class_table = pair.second;
- mirror::Class* klass = class_table->Lookup(descriptor, ComputeModifiedUtf8Hash(descriptor));
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ klass = class_table->Lookup(descriptor, hash);
if (klass != nullptr) {
result.push_back(klass);
}
@@ -3998,22 +4005,21 @@
}
ClassTable* ClassLinker::InsertClassTableForClassLoader(mirror::ClassLoader* class_loader) {
- auto it = classes_.find(GcRoot<mirror::ClassLoader>(class_loader));
- if (it != classes_.end()) {
- return it->second;
+ if (class_loader == nullptr) {
+ return &boot_class_table_;
}
- // Class table for loader not found, add it to the table.
- auto* const class_table = new ClassTable;
- classes_.Put(GcRoot<mirror::ClassLoader>(class_loader), class_table);
+ ClassTable* class_table = class_loader->GetClassTable();
+ if (class_table == nullptr) {
+ class_table = new ClassTable;
+ class_loaders_.push_back(class_loader);
+ // Don't already have a class table, add it to the class loader.
+ class_loader->SetClassTable(class_table);
+ }
return class_table;
}
ClassTable* ClassLinker::ClassTableForClassLoader(mirror::ClassLoader* class_loader) {
- auto it = classes_.find(GcRoot<mirror::ClassLoader>(class_loader));
- if (it != classes_.end()) {
- return it->second;
- }
- return nullptr;
+ return class_loader == nullptr ? &boot_class_table_ : class_loader->GetClassTable();
}
bool ClassLinker::LinkClass(Thread* self, const char* descriptor, Handle<mirror::Class> klass,
@@ -5649,28 +5655,33 @@
}
void ClassLinker::DumpForSigQuit(std::ostream& os) {
- Thread* self = Thread::Current();
+ ScopedObjectAccess soa(Thread::Current());
if (dex_cache_image_class_lookup_required_) {
- ScopedObjectAccess soa(self);
MoveImageClassesToClassTable();
}
- ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_);
+ ReaderMutexLock mu(soa.Self(), *Locks::classlinker_classes_lock_);
os << "Zygote loaded classes=" << NumZygoteClasses() << " post zygote classes="
<< NumNonZygoteClasses() << "\n";
}
size_t ClassLinker::NumZygoteClasses() const {
- size_t sum = 0;
- for (auto& pair : classes_) {
- sum += pair.second->NumZygoteClasses();
+ size_t sum = boot_class_table_.NumZygoteClasses();
+ for (const GcRoot<mirror::ClassLoader>& root : class_loaders_) {
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ if (class_table != nullptr) {
+ sum += class_table->NumZygoteClasses();
+ }
}
return sum;
}
size_t ClassLinker::NumNonZygoteClasses() const {
- size_t sum = 0;
- for (auto& pair : classes_) {
- sum += pair.second->NumNonZygoteClasses();
+ size_t sum = boot_class_table_.NumNonZygoteClasses();
+ for (const GcRoot<mirror::ClassLoader>& root : class_loaders_) {
+ ClassTable* const class_table = root.Read()->GetClassTable();
+ if (class_table != nullptr) {
+ sum += class_table->NumNonZygoteClasses();
+ }
}
return sum;
}
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 17d6be6..7243a25 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -475,25 +475,18 @@
void DropFindArrayClassCache() SHARED_REQUIRES(Locks::mutator_lock_);
private:
- class CompareClassLoaderGcRoot {
- public:
- bool operator()(const GcRoot<mirror::ClassLoader>& a, const GcRoot<mirror::ClassLoader>& b)
- const SHARED_REQUIRES(Locks::mutator_lock_) {
- return a.Read() < b.Read();
- }
- };
-
- typedef SafeMap<GcRoot<mirror::ClassLoader>, ClassTable*, CompareClassLoaderGcRoot>
- ClassLoaderClassTable;
-
void VisitClassesInternal(ClassVisitor* visitor)
REQUIRES(Locks::classlinker_classes_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
// Returns the number of zygote and image classes.
- size_t NumZygoteClasses() const REQUIRES(Locks::classlinker_classes_lock_);
+ size_t NumZygoteClasses() const
+ REQUIRES(Locks::classlinker_classes_lock_)
+ SHARED_REQUIRES(Locks::mutator_lock_);
// Returns the number of non zygote nor image classes.
- size_t NumNonZygoteClasses() const REQUIRES(Locks::classlinker_classes_lock_);
+ size_t NumNonZygoteClasses() const
+ REQUIRES(Locks::classlinker_classes_lock_)
+ SHARED_REQUIRES(Locks::mutator_lock_);
OatFile& GetImageOatFile(gc::space::ImageSpace* space)
REQUIRES(!dex_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
@@ -707,8 +700,13 @@
std::vector<GcRoot<mirror::DexCache>> dex_caches_ GUARDED_BY(dex_lock_);
std::vector<const OatFile*> oat_files_ GUARDED_BY(dex_lock_);
- // This contains strong roots. To enable concurrent root scanning of the class table.
- ClassLoaderClassTable classes_ GUARDED_BY(Locks::classlinker_classes_lock_);
+ // This contains the class laoders which have class tables. It is populated by
+ // InsertClassTableForClassLoader.
+ std::vector<GcRoot<mirror::ClassLoader>> class_loaders_
+ GUARDED_BY(Locks::classlinker_classes_lock_);
+
+ // Boot class path table. Since the class loader for this is null.
+ ClassTable boot_class_table_ GUARDED_BY(Locks::classlinker_classes_lock_);
// New class roots, only used by CMS since the GC needs to mark these in the pause.
std::vector<GcRoot<mirror::Class>> new_class_roots_ GUARDED_BY(Locks::classlinker_classes_lock_);
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 4212dda..3c84d8f 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -546,6 +546,7 @@
struct ClassLoaderOffsets : public CheckOffsets<mirror::ClassLoader> {
ClassLoaderOffsets() : CheckOffsets<mirror::ClassLoader>(false, "Ljava/lang/ClassLoader;") {
+ addOffset(OFFSETOF_MEMBER(mirror::ClassLoader, class_table_), "classTable");
addOffset(OFFSETOF_MEMBER(mirror::ClassLoader, packages_), "packages");
addOffset(OFFSETOF_MEMBER(mirror::ClassLoader, parent_), "parent");
addOffset(OFFSETOF_MEMBER(mirror::ClassLoader, proxyCache_), "proxyCache");
diff --git a/runtime/mirror/class_loader.h b/runtime/mirror/class_loader.h
index 134f1cd..940aaa6 100644
--- a/runtime/mirror/class_loader.h
+++ b/runtime/mirror/class_loader.h
@@ -22,6 +22,7 @@
namespace art {
struct ClassLoaderOffsets;
+class ClassTable;
namespace mirror {
@@ -35,12 +36,23 @@
ClassLoader* GetParent() SHARED_REQUIRES(Locks::mutator_lock_) {
return GetFieldObject<ClassLoader>(OFFSET_OF_OBJECT_MEMBER(ClassLoader, parent_));
}
+ ClassTable* GetClassTable() SHARED_REQUIRES(Locks::mutator_lock_) {
+ return reinterpret_cast<ClassTable*>(
+ GetField64(OFFSET_OF_OBJECT_MEMBER(ClassLoader, class_table_)));
+ }
+ void SetClassTable(ClassTable* class_table) SHARED_REQUIRES(Locks::mutator_lock_) {
+ SetField64<false>(OFFSET_OF_OBJECT_MEMBER(ClassLoader, class_table_),
+ reinterpret_cast<uint64_t>(class_table));
+ }
private:
// Field order required by test "ValidateFieldOrderOfJavaCppUnionClasses".
HeapReference<Object> packages_;
HeapReference<ClassLoader> parent_;
HeapReference<Object> proxyCache_;
+ // Native pointer to class table, need to zero this out when image writing.
+ uint32_t padding_ ATTRIBUTE_UNUSED;
+ uint64_t class_table_;
friend struct art::ClassLoaderOffsets; // for verifying offset information
DISALLOW_IMPLICIT_CONSTRUCTORS(ClassLoader);
