From d57d454a11ac6f49eaa397ec14d6231e3a2727b7 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Wed, 14 Oct 2015 10:55:30 -0700 Subject: Allocate dex cache arrays in their class loader's linear alloc Fixes memory leak for class unloading where the dex cache arrays used to be in the runtime linear alloc which never got freed. TODO: Some of the callers like the compiler just use the runtime linear alloc. We could clean this up if we want to have class unloading during compilation for some reason. Added regression test. Bug: 22720414 Change-Id: Ia50333a06a339efbdaedb5ad94b7a1ae841124ec --- runtime/class_linker.cc | 63 ++++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 24 deletions(-) (limited to 'runtime/class_linker.cc') diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 02f2e0b207..27fcfb0a02 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -1189,7 +1189,9 @@ mirror::PointerArray* ClassLinker::AllocPointerArray(Thread* self, size_t length static_cast(mirror::IntArray::Alloc(self, length))); } -mirror::DexCache* ClassLinker::AllocDexCache(Thread* self, const DexFile& dex_file) { +mirror::DexCache* ClassLinker::AllocDexCache(Thread* self, + const DexFile& dex_file, + LinearAlloc* linear_alloc) { StackHandleScope<6> hs(self); auto dex_cache(hs.NewHandle(down_cast( GetClassRoot(kJavaLangDexCache)->AllocObject(self)))); @@ -1208,18 +1210,13 @@ mirror::DexCache* ClassLinker::AllocDexCache(Thread* self, const DexFile& dex_fi dex_file.NumMethodIds() != 0u || dex_file.NumFieldIds() != 0u) { // NOTE: We "leak" the raw_arrays because we never destroy the dex cache. DCHECK(image_pointer_size_ == 4u || image_pointer_size_ == 8u); - if (sizeof(void*) == 8u && image_pointer_size_ == 4u) { - // When cross-compiling for a 32-bit target on a 64-bit host, we need these arrays - // in the low 4GiB address space so that we can store pointers in 32-bit fields. - // This is conveniently provided by the linear allocator. - raw_arrays = reinterpret_cast( - Runtime::Current()->GetLinearAlloc()->Alloc(self, layout.Size())); // Zero-initialized. - } else { - raw_arrays = reinterpret_cast(calloc(layout.Size(), 1u)); // Zero-initialized. - if (raw_arrays == nullptr) { - return nullptr; - } - } + // When cross-compiling for a 32-bit target on a 64-bit host, we need these arrays + // in the low 4GiB address space so that we can store pointers in 32-bit fields. + // This is conveniently provided by the linear allocator. + raw_arrays = reinterpret_cast( + (sizeof(void*) == 8u && image_pointer_size_ == 4u) + ? Runtime::Current()->GetLinearAlloc()->Alloc(self, layout.Size()) // Zero-initialized. + : linear_alloc->Alloc(self, layout.Size())); // Zero-initialized. } GcRoot* strings = (dex_file.NumStringIds() == 0u) ? nullptr : reinterpret_cast*>(raw_arrays + layout.StringsOffset()); @@ -1590,7 +1587,9 @@ mirror::Class* ClassLinker::DefineClass(Thread* self, self->AssertPendingOOMException(); return nullptr; } - mirror::DexCache* dex_cache = RegisterDexFile(dex_file); + mirror::DexCache* dex_cache = RegisterDexFile( + dex_file, + GetOrCreateAllocatorForClassLoader(class_loader.Get())); if (dex_cache == nullptr) { self->AssertPendingOOMException(); return nullptr; @@ -2093,6 +2092,19 @@ LinearAlloc* ClassLinker::GetAllocatorForClassLoader(mirror::ClassLoader* class_ return allocator; } +LinearAlloc* ClassLinker::GetOrCreateAllocatorForClassLoader(mirror::ClassLoader* class_loader) { + if (class_loader == nullptr) { + return Runtime::Current()->GetLinearAlloc(); + } + WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); + LinearAlloc* allocator = class_loader->GetAllocator(); + if (allocator == nullptr) { + allocator = Runtime::Current()->CreateLinearAlloc(); + class_loader->SetAllocator(allocator); + } + return allocator; +} + void ClassLinker::LoadClassMembers(Thread* self, const DexFile& dex_file, const uint8_t* class_data, @@ -2251,7 +2263,10 @@ void ClassLinker::LoadMethod(Thread* self, void ClassLinker::AppendToBootClassPath(Thread* self, const DexFile& dex_file) { StackHandleScope<1> hs(self); - Handle dex_cache(hs.NewHandle(AllocDexCache(self, dex_file))); + Handle dex_cache(hs.NewHandle(AllocDexCache( + self, + dex_file, + Runtime::Current()->GetLinearAlloc()))); CHECK(dex_cache.Get() != nullptr) << "Failed to allocate dex cache for " << dex_file.GetLocation(); AppendToBootClassPath(dex_file, dex_cache); @@ -2287,7 +2302,7 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file, dex_cache->SetDexFile(&dex_file); } -mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file) { +mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file, LinearAlloc* linear_alloc) { Thread* self = Thread::Current(); { ReaderMutexLock mu(self, dex_lock_); @@ -2300,7 +2315,7 @@ mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file) { // suspend all threads and another thread may need the dex_lock_ to // get to a suspend point. StackHandleScope<1> hs(self); - Handle h_dex_cache(hs.NewHandle(AllocDexCache(self, dex_file))); + Handle h_dex_cache(hs.NewHandle(AllocDexCache(self, dex_file, linear_alloc))); WriterMutexLock mu(self, dex_lock_); mirror::DexCache* dex_cache = FindDexCacheLocked(self, dex_file, true); if (dex_cache != nullptr) { @@ -3097,6 +3112,9 @@ mirror::Class* ClassLinker::CreateProxyClass(ScopedObjectAccessAlreadyRunnable& std::string descriptor(GetDescriptorForProxy(klass.Get())); const size_t hash = ComputeModifiedUtf8Hash(descriptor.c_str()); + // Needs to be before we insert the class so that the allocator field is set. + LinearAlloc* const allocator = GetOrCreateAllocatorForClassLoader(klass->GetClassLoader()); + // Insert the class before loading the fields as the field roots // (ArtField::declaring_class_) are only visited from the class // table. There can't be any suspend points between inserting the @@ -3104,9 +3122,6 @@ mirror::Class* ClassLinker::CreateProxyClass(ScopedObjectAccessAlreadyRunnable& mirror::Class* existing = InsertClass(descriptor.c_str(), klass.Get(), hash); CHECK(existing == nullptr); - // Needs to be after we insert the class so that the allocator field is set. - LinearAlloc* const allocator = GetAllocatorForClassLoader(klass->GetClassLoader()); - // Instance fields are inherited, but we add a couple of static fields... const size_t num_fields = 2; LengthPrefixedArray* sfields = AllocArtFieldArray(self, allocator, num_fields); @@ -3945,13 +3960,13 @@ ClassTable* ClassLinker::InsertClassTableForClassLoader(mirror::ClassLoader* cla ClassLoaderData data; data.weak_root = self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader); data.class_table = class_table; - data.allocator = Runtime::Current()->CreateLinearAlloc(); - class_loaders_.push_back(data); // Don't already have a class table, add it to the class loader. CHECK(class_loader->GetClassTable() == nullptr); - CHECK(class_loader->GetAllocator() == nullptr); class_loader->SetClassTable(data.class_table); - class_loader->SetAllocator(data.allocator); + // Should have been set when we registered the dex file. + data.allocator = class_loader->GetAllocator(); + CHECK(data.allocator != nullptr); + class_loaders_.push_back(data); } return class_table; } -- cgit v1.2.3-59-g8ed1b