Fix potential linear alloc memory leak
Previously, if we created a linear alloc for a class loader but
never created the class table, the linear alloc would never get
freed since it would have no corresponding ClassLoaderData.
Fixes valgrind-test-art-host-gtest-oat_test
Bug: 27384882
Change-Id: Ic8f35b58c3117127a39521b6b9d25ef12c72040c
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index b5e6532..5f26b5d 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2855,8 +2855,9 @@
WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
LinearAlloc* allocator = class_loader->GetAllocator();
if (allocator == nullptr) {
- allocator = Runtime::Current()->CreateLinearAlloc();
- class_loader->SetAllocator(allocator);
+ RegisterClassLoader(class_loader);
+ allocator = class_loader->GetAllocator();
+ CHECK(allocator != nullptr);
}
return allocator;
}
@@ -4817,24 +4818,31 @@
Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(new_class);
}
+void ClassLinker::RegisterClassLoader(mirror::ClassLoader* class_loader) {
+ CHECK(class_loader->GetAllocator() == nullptr);
+ CHECK(class_loader->GetClassTable() == nullptr);
+ Thread* const self = Thread::Current();
+ ClassLoaderData data;
+ data.weak_root = self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader);
+ // Create and set the class table.
+ data.class_table = new ClassTable;
+ class_loader->SetClassTable(data.class_table);
+ // Create and set the linear allocator.
+ data.allocator = Runtime::Current()->CreateLinearAlloc();
+ class_loader->SetAllocator(data.allocator);
+ // Add to the list so that we know to free the data later.
+ class_loaders_.push_back(data);
+}
+
ClassTable* ClassLinker::InsertClassTableForClassLoader(mirror::ClassLoader* class_loader) {
if (class_loader == nullptr) {
return &boot_class_table_;
}
ClassTable* class_table = class_loader->GetClassTable();
if (class_table == nullptr) {
- class_table = new ClassTable;
- Thread* const self = Thread::Current();
- ClassLoaderData data;
- data.weak_root = self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader);
- data.class_table = class_table;
- // Don't already have a class table, add it to the class loader.
- CHECK(class_loader->GetClassTable() == nullptr);
- class_loader->SetClassTable(data.class_table);
- // Should have been set when we registered the dex file.
- data.allocator = class_loader->GetAllocator();
- CHECK(data.allocator != nullptr);
- class_loaders_.push_back(data);
+ RegisterClassLoader(class_loader);
+ class_table = class_loader->GetClassTable();
+ DCHECK(class_table != nullptr);
}
return class_table;
}
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 729617d..0a75b27 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -576,12 +576,12 @@
// Unlike GetOrCreateAllocatorForClassLoader, GetAllocatorForClassLoader asserts that the
// allocator for this class loader is already created.
- static LinearAlloc* GetAllocatorForClassLoader(mirror::ClassLoader* class_loader)
+ LinearAlloc* GetAllocatorForClassLoader(mirror::ClassLoader* class_loader)
SHARED_REQUIRES(Locks::mutator_lock_);
// Return the linear alloc for a class loader if it is already allocated, otherwise allocate and
// set it. TODO: Consider using a lock other than classlinker_classes_lock_.
- static LinearAlloc* GetOrCreateAllocatorForClassLoader(mirror::ClassLoader* class_loader)
+ LinearAlloc* GetOrCreateAllocatorForClassLoader(mirror::ClassLoader* class_loader)
REQUIRES(!Locks::classlinker_classes_lock_)
SHARED_REQUIRES(Locks::mutator_lock_);
@@ -980,9 +980,16 @@
mirror::Class* LookupClassFromBootImage(const char* descriptor)
SHARED_REQUIRES(Locks::mutator_lock_);
+ // Register a class loader and create its class table and allocator. Should not be called if
+ // these are already created.
+ void RegisterClassLoader(mirror::ClassLoader* class_loader)
+ SHARED_REQUIRES(Locks::mutator_lock_)
+ REQUIRES(Locks::classlinker_classes_lock_);
+
// Returns null if not found.
ClassTable* ClassTableForClassLoader(mirror::ClassLoader* class_loader)
SHARED_REQUIRES(Locks::mutator_lock_, Locks::classlinker_classes_lock_);
+
// Insert a new class table if not found.
ClassTable* InsertClassTableForClassLoader(mirror::ClassLoader* class_loader)
SHARED_REQUIRES(Locks::mutator_lock_)
diff --git a/runtime/stack.cc b/runtime/stack.cc
index b1f1ed6..ee5da8e 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -739,7 +739,7 @@
// Check class linker linear allocs.
mirror::Class* klass = method->GetDeclaringClass();
LinearAlloc* const class_linear_alloc = (klass != nullptr)
- ? ClassLinker::GetAllocatorForClassLoader(klass->GetClassLoader())
+ ? runtime->GetClassLinker()->GetAllocatorForClassLoader(klass->GetClassLoader())
: linear_alloc;
if (!class_linear_alloc->Contains(method)) {
// Check image space.
