From: Mathieu Chartier Date: Wed, 14 Oct 2015 17:55:30 +0000 (-0700) Subject: Allocate dex cache arrays in their class loader's linear alloc X-Git-Tag: android-x86-7.1-r1~889^2~136^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=d57d454a11ac6f49eaa397ec14d6231e3a2727b7;p=android-x86%2Fart.git 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 --- diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index 70c9dc1cd..1b54a510f 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -65,6 +65,7 @@ $(ART_TEST_TARGET_GTEST_MainStripped_DEX): $(ART_TEST_TARGET_GTEST_Main_DEX) # Dex file dependencies for each gtest. ART_GTEST_class_linker_test_DEX_DEPS := Interfaces MultiDex MyClass Nested Statics StaticsFromCode ART_GTEST_compiler_driver_test_DEX_DEPS := AbstractMethod StaticLeafMethods +ART_GTEST_dex_cache_test_DEX_DEPS := Main ART_GTEST_dex_file_test_DEX_DEPS := GetMethodSignature Main Nested ART_GTEST_exception_test_DEX_DEPS := ExceptionHandle ART_GTEST_instrumentation_test_DEX_DEPS := Instrumentation diff --git a/compiler/driver/compiler_driver-inl.h b/compiler/driver/compiler_driver-inl.h index e535afd27..1a7dbe3a9 100644 --- a/compiler/driver/compiler_driver-inl.h +++ b/compiler/driver/compiler_driver-inl.h @@ -370,7 +370,9 @@ inline int CompilerDriver::IsFastInvoke( nullptr, kVirtual); } else { StackHandleScope<1> hs(soa.Self()); - auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile(*devirt_target->dex_file))); + auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile( + *devirt_target->dex_file, + class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get())))); called_method = class_linker->ResolveMethod( *devirt_target->dex_file, devirt_target->dex_method_index, target_dex_cache, class_loader, nullptr, kVirtual); diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 74f19a102..8324bf30d 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -953,7 +953,9 @@ void CompilerDriver::LoadImageClasses(TimingLogger* timings) { uint16_t exception_type_idx = exception_type.first; const DexFile* dex_file = exception_type.second; StackHandleScope<2> hs2(self); - Handle dex_cache(hs2.NewHandle(class_linker->RegisterDexFile(*dex_file))); + Handle dex_cache(hs2.NewHandle(class_linker->RegisterDexFile( + *dex_file, + Runtime::Current()->GetLinearAlloc()))); Handle klass(hs2.NewHandle( class_linker->ResolveType(*dex_file, exception_type_idx, dex_cache, NullHandle()))); @@ -2010,9 +2012,11 @@ class ResolveTypeVisitor : public CompilationVisitor { ClassLinker* class_linker = manager_->GetClassLinker(); const DexFile& dex_file = *manager_->GetDexFile(); StackHandleScope<2> hs(soa.Self()); - Handle dex_cache(hs.NewHandle(class_linker->RegisterDexFile(dex_file))); Handle class_loader( hs.NewHandle(soa.Decode(manager_->GetClassLoader()))); + Handle dex_cache(hs.NewHandle(class_linker->RegisterDexFile( + dex_file, + class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get())))); mirror::Class* klass = class_linker->ResolveType(dex_file, type_idx, dex_cache, class_loader); if (klass == nullptr) { diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 680e2d7b4..17c528209 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1401,7 +1401,7 @@ class Dex2Oat FINAL { } ScopedObjectAccess soa(self); dex_caches_.push_back(soa.AddLocalReference( - class_linker->RegisterDexFile(*dex_file))); + class_linker->RegisterDexFile(*dex_file, Runtime::Current()->GetLinearAlloc()))); } // If we use a swap file, ensure we are above the threshold to make it necessary. diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index f5f774883..dbf536575 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -1420,8 +1420,10 @@ class OatDumper { uint32_t method_access_flags) { if ((method_access_flags & kAccNative) == 0) { ScopedObjectAccess soa(Thread::Current()); + Runtime* const runtime = Runtime::Current(); Handle dex_cache( - hs->NewHandle(Runtime::Current()->GetClassLinker()->RegisterDexFile(*dex_file))); + hs->NewHandle(runtime->GetClassLinker()->RegisterDexFile(*dex_file, + runtime->GetLinearAlloc()))); DCHECK(options_.class_loader_ != nullptr); return verifier::MethodVerifier::VerifyMethodAndDump( soa.Self(), vios, dex_method_idx, dex_file, dex_cache, *options_.class_loader_, @@ -2400,14 +2402,13 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti // Need to register dex files to get a working dex cache. ScopedObjectAccess soa(self); ClassLinker* class_linker = runtime->GetClassLinker(); - Runtime::Current()->GetOatFileManager().RegisterOatFile( - std::unique_ptr(oat_file)); + runtime->GetOatFileManager().RegisterOatFile(std::unique_ptr(oat_file)); std::vector class_path; for (const OatFile::OatDexFile* odf : oat_file->GetOatDexFiles()) { std::string error_msg; const DexFile* const dex_file = OpenDexFile(odf, &error_msg); CHECK(dex_file != nullptr) << error_msg; - class_linker->RegisterDexFile(*dex_file); + class_linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc()); class_path.push_back(dex_file); } diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 02f2e0b20..27fcfb0a0 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; } diff --git a/runtime/class_linker.h b/runtime/class_linker.h index 93161f7bb..a70967d49 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -319,7 +319,7 @@ class ClassLinker { SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!dex_lock_, !Roles::uninterruptible_); - mirror::DexCache* RegisterDexFile(const DexFile& dex_file) + mirror::DexCache* RegisterDexFile(const DexFile& dex_file, LinearAlloc* linear_alloc) REQUIRES(!dex_lock_) SHARED_REQUIRES(Locks::mutator_lock_); void RegisterDexFile(const DexFile& dex_file, Handle dex_cache) @@ -532,6 +532,12 @@ class ClassLinker { static 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) + SHARED_REQUIRES(Locks::mutator_lock_) + REQUIRES(!Locks::classlinker_classes_lock_); + private: struct ClassLoaderData { jweak weak_root; // Weak root to enable class unloading. @@ -570,7 +576,9 @@ class ClassLinker { mirror::Class* AllocClass(Thread* self, uint32_t class_size) SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!Roles::uninterruptible_); - mirror::DexCache* AllocDexCache(Thread* self, const DexFile& dex_file) + mirror::DexCache* AllocDexCache(Thread* self, + const DexFile& dex_file, + LinearAlloc* linear_alloc) SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!Roles::uninterruptible_); diff --git a/runtime/mirror/dex_cache_test.cc b/runtime/mirror/dex_cache_test.cc index 8fb860fa6..48f2ca59e 100644 --- a/runtime/mirror/dex_cache_test.cc +++ b/runtime/mirror/dex_cache_test.cc @@ -20,9 +20,8 @@ #include "class_linker.h" #include "common_runtime_test.h" -#include "gc/heap.h" -#include "mirror/object_array-inl.h" -#include "mirror/object-inl.h" +#include "linear_alloc.h" +#include "mirror/class_loader-inl.h" #include "handle_scope-inl.h" #include "scoped_thread_state_change.h" @@ -36,7 +35,9 @@ TEST_F(DexCacheTest, Open) { StackHandleScope<1> hs(soa.Self()); ASSERT_TRUE(java_lang_dex_file_ != nullptr); Handle dex_cache( - hs.NewHandle(class_linker_->AllocDexCache(soa.Self(), *java_lang_dex_file_))); + hs.NewHandle(class_linker_->AllocDexCache(soa.Self(), + *java_lang_dex_file_, + Runtime::Current()->GetLinearAlloc()))); ASSERT_TRUE(dex_cache.Get() != nullptr); EXPECT_EQ(java_lang_dex_file_->NumStringIds(), dex_cache->NumStrings()); @@ -45,5 +46,21 @@ TEST_F(DexCacheTest, Open) { EXPECT_EQ(java_lang_dex_file_->NumFieldIds(), dex_cache->NumResolvedFields()); } +TEST_F(DexCacheTest, LinearAlloc) { + ScopedObjectAccess soa(Thread::Current()); + jobject jclass_loader(LoadDex("Main")); + ASSERT_TRUE(jclass_loader != nullptr); + Runtime* const runtime = Runtime::Current(); + ClassLinker* const class_linker = runtime->GetClassLinker(); + StackHandleScope<1> hs(soa.Self()); + Handle class_loader(hs.NewHandle( + soa.Decode(jclass_loader))); + mirror::Class* klass = class_linker->FindClass(soa.Self(), "LMain;", class_loader); + ASSERT_TRUE(klass != nullptr); + LinearAlloc* const linear_alloc = klass->GetClassLoader()->GetAllocator(); + EXPECT_NE(linear_alloc, runtime->GetLinearAlloc()); + EXPECT_TRUE(linear_alloc->Contains(klass->GetDexCache()->GetResolvedMethods())); +} + } // namespace mirror } // namespace art diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index e9ce02bd0..9fb5d0db6 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -264,10 +264,12 @@ static jclass DexFile_defineClassNative(JNIEnv* env, if (dex_class_def != nullptr) { ScopedObjectAccess soa(env); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - class_linker->RegisterDexFile(*dex_file); StackHandleScope<1> hs(soa.Self()); Handle class_loader( hs.NewHandle(soa.Decode(javaLoader))); + class_linker->RegisterDexFile( + *dex_file, + class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get())); mirror::Class* result = class_linker->DefineClass(soa.Self(), descriptor.c_str(), hash, diff --git a/runtime/native/dalvik_system_VMRuntime.cc b/runtime/native/dalvik_system_VMRuntime.cc index 4f957233c..4c5dc3ad2 100644 --- a/runtime/native/dalvik_system_VMRuntime.cc +++ b/runtime/native/dalvik_system_VMRuntime.cc @@ -497,7 +497,8 @@ static void VMRuntime_preloadDexCaches(JNIEnv* env, jobject) { const DexFile* dex_file = boot_class_path[i]; CHECK(dex_file != nullptr); StackHandleScope<1> hs(soa.Self()); - Handle dex_cache(hs.NewHandle(linker->RegisterDexFile(*dex_file))); + Handle dex_cache( + hs.NewHandle(linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc()))); if (kPreloadDexCachesStrings) { for (size_t j = 0; j < dex_cache->NumStrings(); j++) {