From: Mathieu Chartier Date: Thu, 2 Jun 2016 18:48:30 +0000 (-0700) Subject: Hold dex caches live in class table X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=f284d448e3edd428b6ade473d0993028638b2064;p=android-x86%2Fart.git Hold dex caches live in class table Prevents temporary dex caches being unloaded for the same dex file. Usually this is OK, but if someone resolved a string in that dex cache, it could leave stale pointers in BSS. Also it can use extra memory in linear alloc if we allocate dex cache arrays multiple times. Bug: 29083330 Change-Id: Ia44668f013ceef1f5eb80f653a48d0f8004548c9 --- diff --git a/compiler/driver/compiler_driver-inl.h b/compiler/driver/compiler_driver-inl.h index 3cb63e708..94f5acc2b 100644 --- a/compiler/driver/compiler_driver-inl.h +++ b/compiler/driver/compiler_driver-inl.h @@ -390,9 +390,8 @@ inline int CompilerDriver::IsFastInvoke( *devirt_target->dex_file, devirt_target->dex_method_index, dex_cache, class_loader, nullptr, kVirtual); } else { - auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile( - *devirt_target->dex_file, - class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get())))); + auto target_dex_cache(hs.NewHandle(class_linker->RegisterDexFile(*devirt_target->dex_file, + 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 d20f51001..0387bcda9 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -1057,9 +1057,8 @@ 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, - Runtime::Current()->GetLinearAlloc()))); + Handle dex_cache(hs2.NewHandle(class_linker->RegisterDexFile(*dex_file, + nullptr))); Handle klass(hs2.NewHandle( class_linker->ResolveType(*dex_file, exception_type_idx, @@ -2089,7 +2088,7 @@ class ResolveTypeVisitor : public CompilationVisitor { hs.NewHandle(soa.Decode(manager_->GetClassLoader()))); Handle dex_cache(hs.NewHandle(class_linker->RegisterDexFile( dex_file, - class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get())))); + class_loader.Get()))); mirror::Class* klass = class_linker->ResolveType(dex_file, type_idx, dex_cache, class_loader); if (klass == nullptr) { diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc index 21e198c12..6d1f94491 100644 --- a/compiler/oat_test.cc +++ b/compiler/oat_test.cc @@ -199,7 +199,7 @@ class OatTest : public CommonCompilerTest { for (const std::unique_ptr& dex_file : opened_dex_files) { dex_files.push_back(dex_file.get()); ScopedObjectAccess soa(Thread::Current()); - class_linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc()); + class_linker->RegisterDexFile(*dex_file, nullptr); } linker::MultiOatRelativePatcher patcher(compiler_driver_->GetInstructionSet(), instruction_set_features_.get()); @@ -491,10 +491,7 @@ TEST_F(OatTest, EmptyTextSection) { ClassLinker* const class_linker = Runtime::Current()->GetClassLinker(); for (const DexFile* dex_file : dex_files) { ScopedObjectAccess soa(Thread::Current()); - class_linker->RegisterDexFile( - *dex_file, - class_linker->GetOrCreateAllocatorForClassLoader( - soa.Decode(class_loader))); + class_linker->RegisterDexFile(*dex_file, soa.Decode(class_loader)); } compiler_driver_->SetDexFilesForOatFile(dex_files); compiler_driver_->CompileAll(class_loader, dex_files, &timings); diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index cce83f32b..c4754ce42 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1462,7 +1462,8 @@ class Dex2Oat FINAL { for (const auto& dex_file : dex_files_) { ScopedObjectAccess soa(self); dex_caches_.push_back(soa.AddLocalReference( - class_linker->RegisterDexFile(*dex_file, Runtime::Current()->GetLinearAlloc()))); + class_linker->RegisterDexFile(*dex_file, + soa.Decode(class_loader_)))); } return true; diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index f5458c067..aa4635d6a 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -1118,8 +1118,7 @@ class OatDumper { ScopedObjectAccess soa(Thread::Current()); Runtime* const runtime = Runtime::Current(); Handle dex_cache( - hs->NewHandle(runtime->GetClassLinker()->RegisterDexFile(*dex_file, - runtime->GetLinearAlloc()))); + hs->NewHandle(runtime->GetClassLinker()->RegisterDexFile(*dex_file, nullptr))); DCHECK(options_.class_loader_ != nullptr); return verifier::MethodVerifier::VerifyMethodAndDump( soa.Self(), vios, dex_method_idx, dex_file, dex_cache, *options_.class_loader_, @@ -2283,7 +2282,7 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti std::string error_msg; const DexFile* const dex_file = OpenDexFile(odf, &error_msg); CHECK(dex_file != nullptr) << error_msg; - class_linker->RegisterDexFile(*dex_file, runtime->GetLinearAlloc()); + class_linker->RegisterDexFile(*dex_file, nullptr); class_path.push_back(dex_file); } diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 9e144dddd..e789db6a1 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -2469,9 +2469,7 @@ mirror::Class* ClassLinker::DefineClass(Thread* self, self->AssertPendingOOMException(); return nullptr; } - mirror::DexCache* dex_cache = RegisterDexFile( - dex_file, - GetOrCreateAllocatorForClassLoader(class_loader.Get())); + mirror::DexCache* dex_cache = RegisterDexFile(dex_file, class_loader.Get()); if (dex_cache == nullptr) { self->AssertPendingOOMException(); return nullptr; @@ -3230,7 +3228,8 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file, dex_caches_.push_back(data); } -mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file, LinearAlloc* linear_alloc) { +mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file, + mirror::ClassLoader* class_loader) { Thread* self = Thread::Current(); { ReaderMutexLock mu(self, dex_lock_); @@ -3239,21 +3238,31 @@ mirror::DexCache* ClassLinker::RegisterDexFile(const DexFile& dex_file, LinearAl return dex_cache; } } + LinearAlloc* const linear_alloc = GetOrCreateAllocatorForClassLoader(class_loader); + DCHECK(linear_alloc != nullptr); + ClassTable* table; + { + WriterMutexLock mu(self, *Locks::classlinker_classes_lock_); + table = InsertClassTableForClassLoader(class_loader); + } // Don't alloc while holding the lock, since allocation may need to // 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, linear_alloc))); - WriterMutexLock mu(self, dex_lock_); - mirror::DexCache* dex_cache = FindDexCacheLocked(self, dex_file, true); - if (dex_cache != nullptr) { - return dex_cache; - } - if (h_dex_cache.Get() == nullptr) { - self->AssertPendingOOMException(); - return nullptr; + { + WriterMutexLock mu(self, dex_lock_); + mirror::DexCache* dex_cache = FindDexCacheLocked(self, dex_file, true); + if (dex_cache != nullptr) { + return dex_cache; + } + if (h_dex_cache.Get() == nullptr) { + self->AssertPendingOOMException(); + return nullptr; + } + RegisterDexFileLocked(dex_file, h_dex_cache); } - RegisterDexFileLocked(dex_file, h_dex_cache); + table->InsertStrongRoot(h_dex_cache.Get()); return h_dex_cache.Get(); } @@ -7991,7 +8000,7 @@ void ClassLinker::InsertDexFileInToClassLoader(mirror::Object* dex_file, WriterMutexLock mu(self, *Locks::classlinker_classes_lock_); ClassTable* const table = ClassTableForClassLoader(class_loader); DCHECK(table != nullptr); - if (table->InsertDexFile(dex_file) && class_loader != nullptr) { + if (table->InsertStrongRoot(dex_file) && class_loader != nullptr) { // It was not already inserted, perform the write barrier to let the GC know the class loader's // class table was modified. Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(class_loader); diff --git a/runtime/class_linker.h b/runtime/class_linker.h index f6ce545a1..17b87cda7 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -377,7 +377,8 @@ class ClassLinker { SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!dex_lock_, !Roles::uninterruptible_); - mirror::DexCache* RegisterDexFile(const DexFile& dex_file, LinearAlloc* linear_alloc) + mirror::DexCache* RegisterDexFile(const DexFile& dex_file, + mirror::ClassLoader* class_loader) REQUIRES(!dex_lock_) SHARED_REQUIRES(Locks::mutator_lock_); void RegisterDexFile(const DexFile& dex_file, Handle dex_cache) diff --git a/runtime/class_table-inl.h b/runtime/class_table-inl.h index 42e320ae7..d52365df6 100644 --- a/runtime/class_table-inl.h +++ b/runtime/class_table-inl.h @@ -29,7 +29,7 @@ void ClassTable::VisitRoots(Visitor& visitor) { visitor.VisitRoot(root.AddressWithoutBarrier()); } } - for (GcRoot& root : dex_files_) { + for (GcRoot& root : strong_roots_) { visitor.VisitRoot(root.AddressWithoutBarrier()); } } @@ -42,7 +42,7 @@ void ClassTable::VisitRoots(const Visitor& visitor) { visitor.VisitRoot(root.AddressWithoutBarrier()); } } - for (GcRoot& root : dex_files_) { + for (GcRoot& root : strong_roots_) { visitor.VisitRoot(root.AddressWithoutBarrier()); } } diff --git a/runtime/class_table.cc b/runtime/class_table.cc index 8267c68b2..f5ebcc519 100644 --- a/runtime/class_table.cc +++ b/runtime/class_table.cc @@ -146,15 +146,15 @@ uint32_t ClassTable::ClassDescriptorHashEquals::operator()(const char* descripto return ComputeModifiedUtf8Hash(descriptor); } -bool ClassTable::InsertDexFile(mirror::Object* dex_file) { +bool ClassTable::InsertStrongRoot(mirror::Object* obj) { WriterMutexLock mu(Thread::Current(), lock_); - DCHECK(dex_file != nullptr); - for (GcRoot& root : dex_files_) { - if (root.Read() == dex_file) { + DCHECK(obj != nullptr); + for (GcRoot& root : strong_roots_) { + if (root.Read() == obj) { return false; } } - dex_files_.push_back(GcRoot(dex_file)); + strong_roots_.push_back(GcRoot(obj)); return true; } diff --git a/runtime/class_table.h b/runtime/class_table.h index 686381d35..854a85e81 100644 --- a/runtime/class_table.h +++ b/runtime/class_table.h @@ -133,8 +133,8 @@ class ClassTable { REQUIRES(!lock_) SHARED_REQUIRES(Locks::mutator_lock_); - // Return true if we inserted the dex file, false if it already exists. - bool InsertDexFile(mirror::Object* dex_file) + // Return true if we inserted the strong root, false if it already exists. + bool InsertStrongRoot(mirror::Object* obj) REQUIRES(!lock_) SHARED_REQUIRES(Locks::mutator_lock_); @@ -162,9 +162,10 @@ class ClassTable { mutable ReaderWriterMutex lock_; // We have a vector to help prevent dirty pages after the zygote forks by calling FreezeSnapshot. std::vector classes_ GUARDED_BY(lock_); - // Dex files used by the class loader which may not be owned by the class loader. We keep these - // live so that we do not have issues closing any of the dex files. - std::vector> dex_files_ GUARDED_BY(lock_); + // Extra strong roots that can be either dex files or dex caches. Dex files used by the class + // loader which may not be owned by the class loader must be held strongly live. Also dex caches + // are held live to prevent them being unloading once they have classes in them. + std::vector> strong_roots_ GUARDED_BY(lock_); }; } // namespace art diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index f30f7a641..8c7c96610 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -278,9 +278,7 @@ static jclass DexFile_defineClassNative(JNIEnv* env, StackHandleScope<1> hs(soa.Self()); Handle class_loader( hs.NewHandle(soa.Decode(javaLoader))); - class_linker->RegisterDexFile( - *dex_file, - class_linker->GetOrCreateAllocatorForClassLoader(class_loader.Get())); + class_linker->RegisterDexFile(*dex_file, 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 6c943dc17..79b18aa84 100644 --- a/runtime/native/dalvik_system_VMRuntime.cc +++ b/runtime/native/dalvik_system_VMRuntime.cc @@ -504,8 +504,7 @@ 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, runtime->GetLinearAlloc()))); + Handle dex_cache(hs.NewHandle(linker->RegisterDexFile(*dex_file, nullptr))); if (kPreloadDexCachesStrings) { for (size_t j = 0; j < dex_cache->NumStrings(); j++) {