Use unique_ptr to track ownership of dex files.

Bug: 18809837
Change-Id: Ie571eae8fc19ee9207390cff5c7e2a38071b126a

diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc
index c30cc04..a02e25e 100644 (file)
--- a/compiler/driver/compiler_driver_test.cc
+++ b/compiler/driver/compiler_driver_test.cc
@@ -106,36 +106,37 @@ TEST_F(CompilerDriverTest, DISABLED_LARGE_CompileDexLibCore) {
 
   // All libcore references should resolve
   ScopedObjectAccess soa(Thread::Current());
-  const DexFile* dex = java_lang_dex_file_;
-  mirror::DexCache* dex_cache = class_linker_->FindDexCache(*dex);
-  EXPECT_EQ(dex->NumStringIds(), dex_cache->NumStrings());
+  ASSERT_TRUE(java_lang_dex_file_ != NULL);
+  const DexFile& dex = *java_lang_dex_file_;
+  mirror::DexCache* dex_cache = class_linker_->FindDexCache(dex);
+  EXPECT_EQ(dex.NumStringIds(), dex_cache->NumStrings());
   for (size_t i = 0; i < dex_cache->NumStrings(); i++) {
     const mirror::String* string = dex_cache->GetResolvedString(i);
     EXPECT_TRUE(string != NULL) << "string_idx=" << i;
   }
-  EXPECT_EQ(dex->NumTypeIds(), dex_cache->NumResolvedTypes());
+  EXPECT_EQ(dex.NumTypeIds(), dex_cache->NumResolvedTypes());
   for (size_t i = 0; i < dex_cache->NumResolvedTypes(); i++) {
     mirror::Class* type = dex_cache->GetResolvedType(i);
     EXPECT_TRUE(type != NULL) << "type_idx=" << i
-                              << " " << dex->GetTypeDescriptor(dex->GetTypeId(i));
+                              << " " << dex.GetTypeDescriptor(dex.GetTypeId(i));
   }
-  EXPECT_EQ(dex->NumMethodIds(), dex_cache->NumResolvedMethods());
+  EXPECT_EQ(dex.NumMethodIds(), dex_cache->NumResolvedMethods());
   for (size_t i = 0; i < dex_cache->NumResolvedMethods(); i++) {
     mirror::ArtMethod* method = dex_cache->GetResolvedMethod(i);
     EXPECT_TRUE(method != NULL) << "method_idx=" << i
-                                << " " << dex->GetMethodDeclaringClassDescriptor(dex->GetMethodId(i))
-                                << " " << dex->GetMethodName(dex->GetMethodId(i));
+                                << " " << dex.GetMethodDeclaringClassDescriptor(dex.GetMethodId(i))
+                                << " " << dex.GetMethodName(dex.GetMethodId(i));
     EXPECT_TRUE(method->GetEntryPointFromQuickCompiledCode() != NULL) << "method_idx=" << i
                                            << " "
-                                           << dex->GetMethodDeclaringClassDescriptor(dex->GetMethodId(i))
-                                           << " " << dex->GetMethodName(dex->GetMethodId(i));
+                                           << dex.GetMethodDeclaringClassDescriptor(dex.GetMethodId(i))
+                                           << " " << dex.GetMethodName(dex.GetMethodId(i));
   }
-  EXPECT_EQ(dex->NumFieldIds(), dex_cache->NumResolvedFields());
+  EXPECT_EQ(dex.NumFieldIds(), dex_cache->NumResolvedFields());
   for (size_t i = 0; i < dex_cache->NumResolvedFields(); i++) {
     mirror::ArtField* field = dex_cache->GetResolvedField(i);
     EXPECT_TRUE(field != NULL) << "field_idx=" << i
-                               << " " << dex->GetFieldDeclaringClassDescriptor(dex->GetFieldId(i))
-                               << " " << dex->GetFieldName(dex->GetFieldId(i));
+                               << " " << dex.GetFieldDeclaringClassDescriptor(dex.GetFieldId(i))
+                               << " " << dex.GetFieldName(dex.GetFieldId(i));
   }
 
   // TODO check Class::IsVerified for all classes

diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc
index d141538..b3ab370 100644 (file)
--- a/compiler/oat_test.cc
+++ b/compiler/oat_test.cc
@@ -39,10 +39,10 @@ class OatTest : public CommonCompilerTest {
 
   void CheckMethod(mirror::ArtMethod* method,
                    const OatFile::OatMethod& oat_method,
-                   const DexFile* dex_file)
+                   const DexFile& dex_file)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
     const CompiledMethod* compiled_method =
-        compiler_driver_->GetCompiledMethod(MethodReference(dex_file,
+        compiler_driver_->GetCompiledMethod(MethodReference(&dex_file,
                                                             method->GetDexMethodIndex()));
 
     if (compiled_method == nullptr) {
@@ -130,22 +130,23 @@ TEST_F(OatTest, WriteRead) {
   ASSERT_EQ(4096U, oat_header.GetImageFileLocationOatDataBegin());
   ASSERT_EQ("lue.art", std::string(oat_header.GetStoreValueByKey(OatHeader::kImageLocationKey)));
 
-  const DexFile* dex_file = java_lang_dex_file_;
-  uint32_t dex_file_checksum = dex_file->GetLocationChecksum();
-  const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_file->GetLocation().c_str(),
+  ASSERT_TRUE(java_lang_dex_file_ != nullptr);
+  const DexFile& dex_file = *java_lang_dex_file_;
+  uint32_t dex_file_checksum = dex_file.GetLocationChecksum();
+  const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_file.GetLocation().c_str(),
                                                                     &dex_file_checksum);
   ASSERT_TRUE(oat_dex_file != nullptr);
-  CHECK_EQ(dex_file->GetLocationChecksum(), oat_dex_file->GetDexFileLocationChecksum());
+  CHECK_EQ(dex_file.GetLocationChecksum(), oat_dex_file->GetDexFileLocationChecksum());
   ScopedObjectAccess soa(Thread::Current());
-  for (size_t i = 0; i < dex_file->NumClassDefs(); i++) {
-    const DexFile::ClassDef& class_def = dex_file->GetClassDef(i);
-    const uint8_t* class_data = dex_file->GetClassData(class_def);
+  for (size_t i = 0; i < dex_file.NumClassDefs(); i++) {
+    const DexFile::ClassDef& class_def = dex_file.GetClassDef(i);
+    const uint8_t* class_data = dex_file.GetClassData(class_def);
     size_t num_virtual_methods = 0;
     if (class_data != nullptr) {
-      ClassDataItemIterator it(*dex_file, class_data);
+      ClassDataItemIterator it(dex_file, class_data);
       num_virtual_methods = it.NumVirtualMethods();
     }
-    const char* descriptor = dex_file->GetClassDescriptor(class_def);
+    const char* descriptor = dex_file.GetClassDescriptor(class_def);
     StackHandleScope<1> hs(soa.Self());
     mirror::Class* klass = class_linker->FindClass(soa.Self(), descriptor,
                                                    NullHandle<mirror::ClassLoader>());

diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 4f279f2..c8b27db 100644 (file)
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -445,7 +445,15 @@ class Dex2Oat FINAL {
       timings_(timings) {}
 
   ~Dex2Oat() {
-    LogCompletionTime();  // Needs to be before since it accesses the runtime.
+    // Free opened dex files before deleting the runtime_, because ~DexFile
+    // uses MemMap, which is shut down by ~Runtime.
+    class_path_files_.clear();
+    opened_dex_files_.clear();
+
+    // Log completion time before deleting the runtime_, because this accesses
+    // the runtime.
+    LogCompletionTime();
+
     if (kIsDebugBuild || (RUNNING_ON_VALGRIND != 0)) {
       delete runtime_;  // See field declaration for why this is manual.
     }
@@ -1101,18 +1109,24 @@ class Dex2Oat FINAL {
               << error_msg;
           return false;
         }
-        if (!DexFile::OpenFromZip(*zip_archive.get(), zip_location_, &error_msg, &dex_files_)) {
+        if (!DexFile::OpenFromZip(*zip_archive.get(), zip_location_, &error_msg, &opened_dex_files_)) {
           LOG(ERROR) << "Failed to open dex from file descriptor for zip file '" << zip_location_
               << "': " << error_msg;
           return false;
         }
+        for (auto& dex_file : opened_dex_files_) {
+          dex_files_.push_back(dex_file.get());
+        }
         ATRACE_END();
       } else {
-        size_t failure_count = OpenDexFiles(dex_filenames_, dex_locations_, dex_files_);
+        size_t failure_count = OpenDexFiles(dex_filenames_, dex_locations_, &opened_dex_files_);
         if (failure_count > 0) {
           LOG(ERROR) << "Failed to open some dex files: " << failure_count;
           return false;
         }
+        for (auto& dex_file : opened_dex_files_) {
+          dex_files_.push_back(dex_file.get());
+        }
       }
 
       constexpr bool kSaveDexInput = false;
@@ -1186,9 +1200,13 @@ class Dex2Oat FINAL {
     Thread* self = Thread::Current();
     if (!boot_image_option_.empty()) {
       ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
-      std::vector<const DexFile*> class_path_files(dex_files_);
-      OpenClassPathFiles(runtime_->GetClassPathString(), class_path_files);
+      OpenClassPathFiles(runtime_->GetClassPathString(), dex_files_, &class_path_files_);
       ScopedObjectAccess soa(self);
+      std::vector<const DexFile*> class_path_files(dex_files_);
+      for (auto& class_path_file : class_path_files_) {
+        class_path_files.push_back(class_path_file.get());
+      }
+
       for (size_t i = 0; i < class_path_files.size(); i++) {
         class_linker->RegisterDexFile(*class_path_files[i]);
       }
@@ -1439,7 +1457,8 @@ class Dex2Oat FINAL {
  private:
   static size_t OpenDexFiles(const std::vector<const char*>& dex_filenames,
                              const std::vector<const char*>& dex_locations,
-                             std::vector<const DexFile*>& dex_files) {
+                             std::vector<std::unique_ptr<const DexFile>>* dex_files) {
+    DCHECK(dex_files != nullptr) << "OpenDexFiles out-param is NULL";
     size_t failure_count = 0;
     for (size_t i = 0; i < dex_filenames.size(); i++) {
       const char* dex_filename = dex_filenames[i];
@@ -1450,7 +1469,7 @@ class Dex2Oat FINAL {
         LOG(WARNING) << "Skipping non-existent dex file '" << dex_filename << "'";
         continue;
       }
-      if (!DexFile::Open(dex_filename, dex_location, &error_msg, &dex_files)) {
+      if (!DexFile::Open(dex_filename, dex_location, &error_msg, dex_files)) {
         LOG(WARNING) << "Failed to open .dex from file '" << dex_filename << "': " << error_msg;
         ++failure_count;
       }
@@ -1470,10 +1489,12 @@ class Dex2Oat FINAL {
     return false;
   }
 
-  // Appends to dex_files any elements of class_path that it doesn't already
-  // contain. This will open those dex files as necessary.
+  // Appends to opened_dex_files any elements of class_path that dex_files
+  // doesn't already contain. This will open those dex files as necessary.
   static void OpenClassPathFiles(const std::string& class_path,
-                                 std::vector<const DexFile*>& dex_files) {
+                                 std::vector<const DexFile*> dex_files,
+                                 std::vector<std::unique_ptr<const DexFile>>* opened_dex_files) {
+    DCHECK(opened_dex_files != nullptr) << "OpenClassPathFiles out-param is NULL";
     std::vector<std::string> parsed;
     Split(class_path, ':', &parsed);
     // Take Locks::mutator_lock_ so that lock ordering on the ClassLinker::dex_lock_ is maintained.
@@ -1483,7 +1504,7 @@ class Dex2Oat FINAL {
         continue;
       }
       std::string error_msg;
-      if (!DexFile::Open(parsed[i].c_str(), parsed[i].c_str(), &error_msg, &dex_files)) {
+      if (!DexFile::Open(parsed[i].c_str(), parsed[i].c_str(), &error_msg, opened_dex_files)) {
         LOG(WARNING) << "Failed to open dex file '" << parsed[i] << "': " << error_msg;
       }
     }
@@ -1623,6 +1644,9 @@ class Dex2Oat FINAL {
   DexFileToMethodInlinerMap method_inliner_map_;
   std::unique_ptr<QuickCompilerCallbacks> callbacks_;
 
+  // Ownership for the class path files.
+  std::vector<std::unique_ptr<const DexFile>> class_path_files_;
+
   // Not a unique_ptr as we want to just exit on non-debug builds, not bringing the runtime down
   // in an orderly fashion. The destructor takes care of deleting this.
   Runtime* runtime_;
@@ -1655,6 +1679,7 @@ class Dex2Oat FINAL {
   bool is_host_;
   std::string android_root_;
   std::vector<const DexFile*> dex_files_;
+  std::vector<std::unique_ptr<const DexFile>> opened_dex_files_;
   std::unique_ptr<CompilerDriver> driver_;
   std::vector<std::string> verbose_methods_;
   bool dump_stats_;

diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc
index de4ea36..931cca7 100644 (file)
--- a/oatdump/oatdump.cc
+++ b/oatdump/oatdump.cc
@@ -1968,13 +1968,13 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti
   ScopedObjectAccess soa(self);
   ClassLinker* class_linker = runtime->GetClassLinker();
   class_linker->RegisterOatFile(oat_file);
-  std::vector<const DexFile*> dex_files;
+  std::vector<std::unique_ptr<const DexFile>> dex_files;
   for (const OatFile::OatDexFile* odf : oat_file->GetOatDexFiles()) {
     std::string error_msg;
-    const DexFile* dex_file = odf->OpenDexFile(&error_msg);
+    std::unique_ptr<const DexFile> dex_file = odf->OpenDexFile(&error_msg);
     CHECK(dex_file != nullptr) << error_msg;
     class_linker->RegisterDexFile(*dex_file);
-    dex_files.push_back(dex_file);
+    dex_files.push_back(std::move(dex_file));
   }
 
   // Need a class loader.
@@ -1983,7 +1983,11 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti
       soa.Env()->AllocObject(WellKnownClasses::dalvik_system_PathClassLoader));
   jobject class_loader = soa.Env()->NewGlobalRef(class_loader_local.get());
   // Fake that we're a compiler.
-  runtime->SetCompileTimeClassPath(class_loader, dex_files);
+  std::vector<const DexFile*> class_path;
+  for (auto& dex_file : dex_files) {
+    class_path.push_back(dex_file.get());
+  }
+  runtime->SetCompileTimeClassPath(class_loader, class_path);
 
   // Use the class loader while dumping.
   StackHandleScope<1> scope(self);

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index d119a56..438cebf 100644 (file)
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -246,7 +246,7 @@ ClassLinker::ClassLinker(InternTable* intern_table)
   memset(find_array_class_cache_, 0, kFindArrayCacheSize * sizeof(mirror::Class*));
 }
 
-void ClassLinker::InitWithoutImage(const std::vector<const DexFile*>& boot_class_path) {
+void ClassLinker::InitWithoutImage(std::vector<std::unique_ptr<const DexFile>> boot_class_path) {
   VLOG(startup) << "ClassLinker::Init";
   CHECK(!Runtime::Current()->GetHeap()->HasImageSpace()) << "Runtime has image. We should use it.";
 
@@ -405,9 +405,10 @@ void ClassLinker::InitWithoutImage(const std::vector<const DexFile*>& boot_class
   // DexCache instances. Needs to be after String, Field, Method arrays since AllocDexCache uses
   // these roots.
   CHECK_NE(0U, boot_class_path.size());
-  for (const DexFile* dex_file : boot_class_path) {
-    CHECK(dex_file != nullptr);
+  for (auto& dex_file : boot_class_path) {
+    CHECK(dex_file.get() != nullptr);
     AppendToBootClassPath(self, *dex_file);
+    opened_dex_files_.push_back(std::move(dex_file));
   }
 
   // now we can use FindSystemClass
@@ -794,7 +795,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file,
                                          const uint32_t* dex_location_checksum,
                                          bool generated,
                                          std::vector<std::string>* error_msgs,
-                                         std::vector<const DexFile*>* dex_files) {
+                                         std::vector<std::unique_ptr<const DexFile>>* dex_files) {
   if (oat_file == nullptr) {
     return false;
   }
@@ -841,12 +842,12 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file,
     }
 
     if (success) {
-      const DexFile* dex_file = oat_dex_file->OpenDexFile(&error_msg);
-      if (dex_file == nullptr) {
+      std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error_msg);
+      if (dex_file.get() == nullptr) {
         success = false;
         error_msgs->push_back(error_msg);
       } else {
-        dex_files->push_back(dex_file);
+        dex_files->push_back(std::move(dex_file));
       }
     }
 
@@ -864,14 +865,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file,
   if (success) {
     return true;
   } else {
-    // Free all the dex files we have loaded.
-    auto it = dex_files->begin() + old_size;
-    auto it_end = dex_files->end();
-    for (; it != it_end; it++) {
-      delete *it;
-    }
-    dex_files->erase(dex_files->begin() + old_size, it_end);
-
+    dex_files->erase(dex_files->begin() + old_size, dex_files->end());
     return false;
   }
 }
@@ -882,7 +876,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file,
 // multidex ahead of time.
 bool ClassLinker::OpenDexFilesFromOat(const char* dex_location, const char* oat_location,
                                       std::vector<std::string>* error_msgs,
-                                      std::vector<const DexFile*>* dex_files) {
+                                      std::vector<std::unique_ptr<const DexFile>>* dex_files) {
   // 1) Check whether we have an open oat file.
   // This requires a dex checksum, use the "primary" one.
   uint32_t dex_location_checksum;
@@ -1232,15 +1226,15 @@ bool ClassLinker::VerifyOatWithDexFile(const OatFile* oat_file,
                                 error_msg->c_str());
       return false;
     }
-    dex_file.reset(oat_dex_file->OpenDexFile(error_msg));
+    dex_file = oat_dex_file->OpenDexFile(error_msg);
   } else {
     bool verified = VerifyOatAndDexFileChecksums(oat_file, dex_location, *dex_location_checksum,
                                                  kRuntimeISA, error_msg);
     if (!verified) {
       return false;
     }
-    dex_file.reset(oat_file->GetOatDexFile(dex_location,
-                                           dex_location_checksum)->OpenDexFile(error_msg));
+    dex_file = oat_file->GetOatDexFile(dex_location,
+                                       dex_location_checksum)->OpenDexFile(error_msg);
   }
   return dex_file.get() != nullptr;
 }
@@ -1685,8 +1679,8 @@ void ClassLinker::InitFromImage() {
                                                                      nullptr);
     CHECK(oat_dex_file != nullptr) << oat_file.GetLocation() << " " << dex_file_location;
     std::string error_msg;
-    const DexFile* dex_file = oat_dex_file->OpenDexFile(&error_msg);
-    if (dex_file == nullptr) {
+    std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error_msg);
+    if (dex_file.get() == nullptr) {
       LOG(FATAL) << "Failed to open dex file " << dex_file_location
                  << " from within oat file " << oat_file.GetLocation()
                  << " error '" << error_msg << "'";
@@ -1695,7 +1689,8 @@ void ClassLinker::InitFromImage() {
 
     CHECK_EQ(dex_file->GetLocationChecksum(), oat_dex_file->GetDexFileLocationChecksum());
 
-    AppendToBootClassPath(*dex_file, dex_cache);
+    AppendToBootClassPath(*dex_file.get(), dex_cache);
+    opened_dex_files_.push_back(std::move(dex_file));
   }
 
   // Set classes on AbstractMethod early so that IsMethod tests can be performed during the live
@@ -1928,7 +1923,6 @@ ClassLinker::~ClassLinker() {
   mirror::ShortArray::ResetArrayClass();
   mirror::Throwable::ResetClass();
   mirror::StackTraceElement::ResetClass();
-  STLDeleteElements(&boot_class_path_);
   STLDeleteElements(&oat_files_);
 }
 

diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 6461835..6570c5f 100644 (file)
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -105,7 +105,7 @@ class ClassLinker {
   ~ClassLinker();
 
   // Initialize class linker by bootstraping from dex files.
-  void InitWithoutImage(const std::vector<const DexFile*>& boot_class_path)
+  void InitWithoutImage(std::vector<std::unique_ptr<const DexFile>> boot_class_path)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   // Initialize class linker from one or more images.
@@ -324,7 +324,7 @@ class ClassLinker {
   // (if multidex) into the given vector.
   bool OpenDexFilesFromOat(const char* dex_location, const char* oat_location,
                            std::vector<std::string>* error_msgs,
-                           std::vector<const DexFile*>* dex_files)
+                           std::vector<std::unique_ptr<const DexFile>>* dex_files)
       LOCKS_EXCLUDED(dex_lock_, Locks::mutator_lock_);
 
   // Returns true if the given oat file has the same image checksum as the image it is paired with.
@@ -722,6 +722,7 @@ class ClassLinker {
   const void* GetRuntimeQuickGenericJniStub() const;
 
   std::vector<const DexFile*> boot_class_path_;
+  std::vector<std::unique_ptr<const DexFile>> opened_dex_files_;
 
   mutable ReaderWriterMutex dex_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
   std::vector<size_t> new_dex_cache_roots_ GUARDED_BY(dex_lock_);

diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 4f09460..6c7c1e2 100644 (file)
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -342,28 +342,26 @@ class ClassLinkerTest : public CommonRuntimeTest {
     }
   }
 
-  void AssertDexFile(const DexFile* dex, mirror::ClassLoader* class_loader)
+  void AssertDexFile(const DexFile& dex, mirror::ClassLoader* class_loader)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-    ASSERT_TRUE(dex != nullptr);
-
     // Verify all the classes defined in this file
-    for (size_t i = 0; i < dex->NumClassDefs(); i++) {
-      const DexFile::ClassDef& class_def = dex->GetClassDef(i);
-      const char* descriptor = dex->GetClassDescriptor(class_def);
+    for (size_t i = 0; i < dex.NumClassDefs(); i++) {
+      const DexFile::ClassDef& class_def = dex.GetClassDef(i);
+      const char* descriptor = dex.GetClassDescriptor(class_def);
       AssertDexFileClass(class_loader, descriptor);
     }
     // Verify all the types referenced by this file
-    for (size_t i = 0; i < dex->NumTypeIds(); i++) {
-      const DexFile::TypeId& type_id = dex->GetTypeId(i);
-      const char* descriptor = dex->GetTypeDescriptor(type_id);
+    for (size_t i = 0; i < dex.NumTypeIds(); i++) {
+      const DexFile::TypeId& type_id = dex.GetTypeId(i);
+      const char* descriptor = dex.GetTypeDescriptor(type_id);
       AssertDexFileClass(class_loader, descriptor);
     }
     class_linker_->VisitRoots(TestRootVisitor, nullptr, kVisitRootFlagAllRoots);
     // Verify the dex cache has resolution methods in all resolved method slots
-    mirror::DexCache* dex_cache = class_linker_->FindDexCache(*dex);
+    mirror::DexCache* dex_cache = class_linker_->FindDexCache(dex);
     mirror::ObjectArray<mirror::ArtMethod>* resolved_methods = dex_cache->GetResolvedMethods();
     for (size_t i = 0; i < static_cast<size_t>(resolved_methods->GetLength()); i++) {
-      EXPECT_TRUE(resolved_methods->Get(i) != nullptr) << dex->GetLocation() << " i=" << i;
+      EXPECT_TRUE(resolved_methods->Get(i) != nullptr) << dex.GetLocation() << " i=" << i;
     }
   }
 
@@ -744,7 +742,8 @@ TEST_F(ClassLinkerTest, FindClass) {
 
 TEST_F(ClassLinkerTest, LibCore) {
   ScopedObjectAccess soa(Thread::Current());
-  AssertDexFile(java_lang_dex_file_, nullptr);
+  ASSERT_TRUE(java_lang_dex_file_ != nullptr);
+  AssertDexFile(*java_lang_dex_file_, nullptr);
 }
 
 // The first reference array element must be a multiple of 4 bytes from the

diff --git a/runtime/common_runtime_test.cc b/runtime/common_runtime_test.cc
index 75ba9dd..e017699 100644 (file)
--- a/runtime/common_runtime_test.cc
+++ b/runtime/common_runtime_test.cc
@@ -102,7 +102,11 @@ void ScratchFile::Unlink() {
 }
 
 CommonRuntimeTest::CommonRuntimeTest() {}
-CommonRuntimeTest::~CommonRuntimeTest() {}
+CommonRuntimeTest::~CommonRuntimeTest() {
+  // Ensure the dex files are cleaned up before the runtime.
+  loaded_dex_files_.clear();
+  runtime_.reset();
+}
 
 void CommonRuntimeTest::SetUpAndroidRoot() {
   if (IsHost()) {
@@ -181,15 +185,15 @@ std::string CommonRuntimeTest::GetCoreOatLocation() {
   return GetCoreFileLocation("oat");
 }
 
-const DexFile* CommonRuntimeTest::LoadExpectSingleDexFile(const char* location) {
-  std::vector<const DexFile*> dex_files;
+std::unique_ptr<const DexFile> CommonRuntimeTest::LoadExpectSingleDexFile(const char* location) {
+  std::vector<std::unique_ptr<const DexFile>> dex_files;
   std::string error_msg;
   if (!DexFile::Open(location, location, &error_msg, &dex_files)) {
     LOG(FATAL) << "Could not open .dex file '" << location << "': " << error_msg << "\n";
-    return nullptr;
+    UNREACHABLE();
   } else {
     CHECK_EQ(1U, dex_files.size()) << "Expected only one dex file in " << location;
-    return dex_files[0];
+    return std::move(dex_files[0]);
   }
 }
 
@@ -222,6 +226,9 @@ void CommonRuntimeTest::SetUp() {
   class_linker_ = runtime_->GetClassLinker();
   class_linker_->FixupDexCaches(runtime_->GetResolutionMethod());
   class_linker_->RunRootClinits();
+  boot_class_path_ = class_linker_->GetBootClassPath();
+  java_lang_dex_file_ = boot_class_path_[0];
+
 
   // Runtime::Create acquired the mutator_lock_ that is normally given away when we
   // Runtime::Start, give it away now and then switch to a more managable ScopedObjectAccess.
@@ -285,8 +292,6 @@ void CommonRuntimeTest::TearDown() {
   IcuCleanupFn icu_cleanup_fn = reinterpret_cast<IcuCleanupFn>(sym);
   (*icu_cleanup_fn)();
 
-  STLDeleteElements(&opened_dex_files_);
-
   Runtime::Current()->GetHeap()->VerifyHeap();  // Check for heap corruption after the test
 }
 
@@ -323,7 +328,7 @@ std::string CommonRuntimeTest::GetTestAndroidRoot() {
 #define ART_TARGET_NATIVETEST_DIR_STRING ""
 #endif
 
-std::vector<const DexFile*> CommonRuntimeTest::OpenTestDexFiles(const char* name) {
+std::vector<std::unique_ptr<const DexFile>> CommonRuntimeTest::OpenTestDexFiles(const char* name) {
   CHECK(name != nullptr);
   std::string filename;
   if (IsHost()) {
@@ -336,28 +341,30 @@ std::vector<const DexFile*> CommonRuntimeTest::OpenTestDexFiles(const char* name
   filename += name;
   filename += ".jar";
   std::string error_msg;
-  std::vector<const DexFile*> dex_files;
+  std::vector<std::unique_ptr<const DexFile>> dex_files;
   bool success = DexFile::Open(filename.c_str(), filename.c_str(), &error_msg, &dex_files);
   CHECK(success) << "Failed to open '" << filename << "': " << error_msg;
-  for (const DexFile* dex_file : dex_files) {
+  for (auto& dex_file : dex_files) {
     CHECK_EQ(PROT_READ, dex_file->GetPermissions());
     CHECK(dex_file->IsReadOnly());
   }
-  opened_dex_files_.insert(opened_dex_files_.end(), dex_files.begin(), dex_files.end());
   return dex_files;
 }
 
-const DexFile* CommonRuntimeTest::OpenTestDexFile(const char* name) {
-  std::vector<const DexFile*> vector = OpenTestDexFiles(name);
+std::unique_ptr<const DexFile> CommonRuntimeTest::OpenTestDexFile(const char* name) {
+  std::vector<std::unique_ptr<const DexFile>> vector = OpenTestDexFiles(name);
   EXPECT_EQ(1U, vector.size());
-  return vector[0];
+  return std::move(vector[0]);
 }
 
 jobject CommonRuntimeTest::LoadDex(const char* dex_name) {
-  std::vector<const DexFile*> dex_files = OpenTestDexFiles(dex_name);
+  std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles(dex_name);
+  std::vector<const DexFile*> class_path;
   CHECK_NE(0U, dex_files.size());
-  for (const DexFile* dex_file : dex_files) {
+  for (auto& dex_file : dex_files) {
+    class_path.push_back(dex_file.get());
     class_linker_->RegisterDexFile(*dex_file);
+    loaded_dex_files_.push_back(std::move(dex_file));
   }
   Thread* self = Thread::Current();
   JNIEnvExt* env = self->GetJniEnv();
@@ -365,7 +372,7 @@ jobject CommonRuntimeTest::LoadDex(const char* dex_name) {
       env->AllocObject(WellKnownClasses::dalvik_system_PathClassLoader));
   jobject class_loader = env->NewGlobalRef(class_loader_local.get());
   self->SetClassLoaderOverride(class_loader_local.get());
-  Runtime::Current()->SetCompileTimeClassPath(class_loader, dex_files);
+  Runtime::Current()->SetCompileTimeClassPath(class_loader, class_path);
   return class_loader;
 }
 

diff --git a/runtime/common_runtime_test.h b/runtime/common_runtime_test.h
index 35dc30f..38a9733 100644 (file)
--- a/runtime/common_runtime_test.h
+++ b/runtime/common_runtime_test.h
@@ -87,7 +87,7 @@ class CommonRuntimeTest : public testing::Test {
   // File location to core.oat, e.g. $ANDROID_HOST_OUT/system/framework/core.oat
   static std::string GetCoreOatLocation();
 
-  const DexFile* LoadExpectSingleDexFile(const char* location);
+  std::unique_ptr<const DexFile> LoadExpectSingleDexFile(const char* location);
 
   virtual void SetUp();
 
@@ -106,26 +106,30 @@ class CommonRuntimeTest : public testing::Test {
 
   std::string GetTestAndroidRoot();
 
-  std::vector<const DexFile*> OpenTestDexFiles(const char* name)
+  std::vector<std::unique_ptr<const DexFile>> OpenTestDexFiles(const char* name)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  const DexFile* OpenTestDexFile(const char* name) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+  std::unique_ptr<const DexFile> OpenTestDexFile(const char* name)
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   jobject LoadDex(const char* dex_name) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   std::string android_data_;
   std::string dalvik_cache_;
-  const DexFile* java_lang_dex_file_;  // owned by runtime_
-  std::vector<const DexFile*> boot_class_path_;  // owned by runtime_
+
   std::unique_ptr<Runtime> runtime_;
-  // Owned by the runtime
+
+  // The class_linker_, java_lang_dex_file_, and boot_class_path_ are all
+  // owned by the runtime.
   ClassLinker* class_linker_;
+  const DexFile* java_lang_dex_file_;
+  std::vector<const DexFile*> boot_class_path_;
 
  private:
   static std::string GetCoreFileLocation(const char* suffix);
 
   std::unique_ptr<CompilerCallbacks> callbacks_;
-  std::vector<const DexFile*> opened_dex_files_;
+  std::vector<std::unique_ptr<const DexFile>> loaded_dex_files_;
 };
 
 // Sets a CheckJni abort hook to catch failures. Note that this will cause CheckJNI to carry on

diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 3d4184b..3f6175f 100644 (file)
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -125,7 +125,8 @@ bool DexFile::GetChecksum(const char* filename, uint32_t* checksum, std::string*
 }
 
 bool DexFile::Open(const char* filename, const char* location, std::string* error_msg,
-                   std::vector<const DexFile*>* dex_files) {
+                   std::vector<std::unique_ptr<const DexFile>>* dex_files) {
+  DCHECK(dex_files != nullptr) << "DexFile::Open: out-param is NULL";
   uint32_t magic;
   ScopedFd fd(OpenAndReadMagic(filename, &magic, error_msg));
   if (fd.get() == -1) {
@@ -139,7 +140,7 @@ bool DexFile::Open(const char* filename, const char* location, std::string* erro
     std::unique_ptr<const DexFile> dex_file(DexFile::OpenFile(fd.release(), location, true,
                                                               error_msg));
     if (dex_file.get() != nullptr) {
-      dex_files->push_back(dex_file.release());
+      dex_files->push_back(std::move(dex_file));
       return true;
     } else {
       return false;
@@ -179,8 +180,8 @@ bool DexFile::DisableWrite() const {
   }
 }
 
-const DexFile* DexFile::OpenFile(int fd, const char* location, bool verify,
-                                 std::string* error_msg) {
+std::unique_ptr<const DexFile> DexFile::OpenFile(int fd, const char* location, bool verify,
+                                                 std::string* error_msg) {
   CHECK(location != nullptr);
   std::unique_ptr<MemMap> map;
   {
@@ -224,13 +225,14 @@ const DexFile* DexFile::OpenFile(int fd, const char* location, bool verify,
     return nullptr;
   }
 
-  return dex_file.release();
+  return dex_file;
 }
 
 const char* DexFile::kClassesDex = "classes.dex";
 
 bool DexFile::OpenZip(int fd, const std::string& location, std::string* error_msg,
-                      std::vector<const  DexFile*>* dex_files) {
+                      std::vector<std::unique_ptr<const DexFile>>* dex_files) {
+  DCHECK(dex_files != nullptr) << "DexFile::OpenZip: out-param is NULL";
   std::unique_ptr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd, location.c_str(), error_msg));
   if (zip_archive.get() == nullptr) {
     DCHECK(!error_msg->empty());
@@ -239,10 +241,10 @@ bool DexFile::OpenZip(int fd, const std::string& location, std::string* error_ms
   return DexFile::OpenFromZip(*zip_archive, location, error_msg, dex_files);
 }
 
-const DexFile* DexFile::OpenMemory(const std::string& location,
-                                   uint32_t location_checksum,
-                                   MemMap* mem_map,
-                                   std::string* error_msg) {
+std::unique_ptr<const DexFile> DexFile::OpenMemory(const std::string& location,
+                                                   uint32_t location_checksum,
+                                                   MemMap* mem_map,
+                                                   std::string* error_msg) {
   return OpenMemory(mem_map->Begin(),
                     mem_map->Size(),
                     location,
@@ -251,9 +253,9 @@ const DexFile* DexFile::OpenMemory(const std::string& location,
                     error_msg);
 }
 
-const DexFile* DexFile::Open(const ZipArchive& zip_archive, const char* entry_name,
-                             const std::string& location, std::string* error_msg,
-                             ZipOpenErrorCode* error_code) {
+std::unique_ptr<const DexFile> DexFile::Open(const ZipArchive& zip_archive, const char* entry_name,
+                                             const std::string& location, std::string* error_msg,
+                                             ZipOpenErrorCode* error_code) {
   CHECK(!location.empty());
   std::unique_ptr<ZipEntry> zip_entry(zip_archive.Find(entry_name, error_msg));
   if (zip_entry.get() == NULL) {
@@ -287,11 +289,13 @@ const DexFile* DexFile::Open(const ZipArchive& zip_archive, const char* entry_na
     return nullptr;
   }
   *error_code = ZipOpenErrorCode::kNoError;
-  return dex_file.release();
+  return dex_file;
 }
 
 bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& location,
-                          std::string* error_msg, std::vector<const DexFile*>* dex_files) {
+                          std::string* error_msg,
+                          std::vector<std::unique_ptr<const DexFile>>* dex_files) {
+  DCHECK(dex_files != nullptr) << "DexFile::OpenFromZip: out-param is NULL";
   ZipOpenErrorCode error_code;
   std::unique_ptr<const DexFile> dex_file(Open(zip_archive, kClassesDex, location, error_msg,
                                                &error_code));
@@ -299,7 +303,7 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca
     return false;
   } else {
     // Had at least classes.dex.
-    dex_files->push_back(dex_file.release());
+    dex_files->push_back(std::move(dex_file));
 
     // Now try some more.
     size_t i = 2;
@@ -318,7 +322,7 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca
         }
         break;
       } else {
-        dex_files->push_back(next_dex_file.release());
+        dex_files->push_back(std::move(next_dex_file));
       }
 
       i++;
@@ -329,18 +333,17 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca
 }
 
 
-const DexFile* DexFile::OpenMemory(const uint8_t* base,
-                                   size_t size,
-                                   const std::string& location,
-                                   uint32_t location_checksum,
-                                   MemMap* mem_map, std::string* error_msg) {
+std::unique_ptr<const DexFile> DexFile::OpenMemory(const uint8_t* base,
+                                                   size_t size,
+                                                   const std::string& location,
+                                                   uint32_t location_checksum,
+                                                   MemMap* mem_map, std::string* error_msg) {
   CHECK_ALIGNED(base, 4);  // various dex file structures must be word aligned
   std::unique_ptr<DexFile> dex_file(new DexFile(base, size, location, location_checksum, mem_map));
   if (!dex_file->Init(error_msg)) {
-    return nullptr;
-  } else {
-    return dex_file.release();
+    dex_file.reset();
   }
+  return std::unique_ptr<const DexFile>(dex_file.release());
 }
 
 DexFile::DexFile(const uint8_t* base, size_t size,

diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index a71ca42..019c8e6 100644 (file)
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -385,19 +385,20 @@ class DexFile {
 
   // Opens .dex files found in the container, guessing the container format based on file extension.
   static bool Open(const char* filename, const char* location, std::string* error_msg,
-                   std::vector<const DexFile*>* dex_files);
+                   std::vector<std::unique_ptr<const DexFile>>* dex_files);
 
   // Opens .dex file, backed by existing memory
-  static const DexFile* Open(const uint8_t* base, size_t size,
-                             const std::string& location,
-                             uint32_t location_checksum,
-                             std::string* error_msg) {
+  static std::unique_ptr<const DexFile> Open(const uint8_t* base, size_t size,
+                                             const std::string& location,
+                                             uint32_t location_checksum,
+                                             std::string* error_msg) {
     return OpenMemory(base, size, location, location_checksum, NULL, error_msg);
   }
 
   // Open all classesXXX.dex files from a zip archive.
   static bool OpenFromZip(const ZipArchive& zip_archive, const std::string& location,
-                          std::string* error_msg, std::vector<const DexFile*>* dex_files);
+                          std::string* error_msg,
+                          std::vector<std::unique_ptr<const DexFile>>* dex_files);
 
   // Closes a .dex file.
   virtual ~DexFile();
@@ -892,11 +893,12 @@ class DexFile {
 
  private:
   // Opens a .dex file
-  static const DexFile* OpenFile(int fd, const char* location, bool verify, std::string* error_msg);
+  static std::unique_ptr<const DexFile> OpenFile(int fd, const char* location,
+                                                 bool verify, std::string* error_msg);
 
   // Opens dex files from within a .jar, .zip, or .apk file
   static bool OpenZip(int fd, const std::string& location, std::string* error_msg,
-                      std::vector<const DexFile*>* dex_files);
+                      std::vector<std::unique_ptr<const DexFile>>* dex_files);
 
   enum class ZipOpenErrorCode {  // private
     kNoError,
@@ -909,23 +911,23 @@ class DexFile {
 
   // Opens .dex file from the entry_name in a zip archive. error_code is undefined when non-nullptr
   // return.
-  static const DexFile* Open(const ZipArchive& zip_archive, const char* entry_name,
-                             const std::string& location, std::string* error_msg,
-                             ZipOpenErrorCode* error_code);
+  static std::unique_ptr<const DexFile> Open(const ZipArchive& zip_archive, const char* entry_name,
+                                             const std::string& location, std::string* error_msg,
+                                             ZipOpenErrorCode* error_code);
 
   // Opens a .dex file at the given address backed by a MemMap
-  static const DexFile* OpenMemory(const std::string& location,
-                                   uint32_t location_checksum,
-                                   MemMap* mem_map,
-                                   std::string* error_msg);
+  static std::unique_ptr<const DexFile> OpenMemory(const std::string& location,
+                                                   uint32_t location_checksum,
+                                                   MemMap* mem_map,
+                                                   std::string* error_msg);
 
   // Opens a .dex file at the given address, optionally backed by a MemMap
-  static const DexFile* OpenMemory(const uint8_t* dex_file,
-                                   size_t size,
-                                   const std::string& location,
-                                   uint32_t location_checksum,
-                                   MemMap* mem_map,
-                                   std::string* error_msg);
+  static std::unique_ptr<const DexFile> OpenMemory(const uint8_t* dex_file,
+                                                   size_t size,
+                                                   const std::string& location,
+                                                   uint32_t location_checksum,
+                                                   MemMap* mem_map,
+                                                   std::string* error_msg);
 
   DexFile(const uint8_t* base, size_t size,
           const std::string& location,

diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc
index 0b54d47..7f5a181 100644 (file)
--- a/runtime/dex_file_test.cc
+++ b/runtime/dex_file_test.cc
@@ -32,8 +32,8 @@ class DexFileTest : public CommonRuntimeTest {};
 
 TEST_F(DexFileTest, Open) {
   ScopedObjectAccess soa(Thread::Current());
-  const DexFile* dex(OpenTestDexFile("Nested"));
-  ASSERT_TRUE(dex != NULL);
+  std::unique_ptr<const DexFile> dex(OpenTestDexFile("Nested"));
+  ASSERT_TRUE(dex.get() != NULL);
 }
 
 static const uint8_t kBase64Map[256] = {
@@ -133,8 +133,8 @@ static const char kRawDex[] =
   "AAACAAAAQAEAAAEgAAACAAAAVAEAAAYgAAACAAAAiAEAAAEQAAABAAAAqAEAAAIgAAAPAAAArgEA"
   "AAMgAAACAAAAiAIAAAQgAAADAAAAlAIAAAAgAAACAAAAqwIAAAAQAAABAAAAxAIAAA==";
 
-static const DexFile* OpenDexFileBase64(const char* base64,
-                                        const char* location) {
+static std::unique_ptr<const DexFile> OpenDexFileBase64(const char* base64,
+                                                        const char* location) {
   // decode base64
   CHECK(base64 != NULL);
   size_t length;
@@ -155,11 +155,11 @@ static const DexFile* OpenDexFileBase64(const char* base64,
   // read dex file
   ScopedObjectAccess soa(Thread::Current());
   std::string error_msg;
-  std::vector<const DexFile*> tmp;
+  std::vector<std::unique_ptr<const DexFile>> tmp;
   bool success = DexFile::Open(location, location, &error_msg, &tmp);
   CHECK(success) << error_msg;
   EXPECT_EQ(1U, tmp.size());
-  const DexFile* dex_file = tmp[0];
+  std::unique_ptr<const DexFile> dex_file = std::move(tmp[0]);
   EXPECT_EQ(PROT_READ, dex_file->GetPermissions());
   EXPECT_TRUE(dex_file->IsReadOnly());
   return dex_file;
@@ -198,7 +198,7 @@ TEST_F(DexFileTest, Header) {
 
 TEST_F(DexFileTest, GetLocationChecksum) {
   ScopedObjectAccess soa(Thread::Current());
-  const DexFile* raw(OpenTestDexFile("Main"));
+  std::unique_ptr<const DexFile> raw(OpenTestDexFile("Main"));
   EXPECT_NE(raw->GetHeader().checksum_, raw->GetLocationChecksum());
 }
 
@@ -213,8 +213,8 @@ TEST_F(DexFileTest, GetChecksum) {
 
 TEST_F(DexFileTest, ClassDefs) {
   ScopedObjectAccess soa(Thread::Current());
-  const DexFile* raw(OpenTestDexFile("Nested"));
-  ASSERT_TRUE(raw != NULL);
+  std::unique_ptr<const DexFile> raw(OpenTestDexFile("Nested"));
+  ASSERT_TRUE(raw.get() != nullptr);
   EXPECT_EQ(2U, raw->NumClassDefs());
 
   const DexFile::ClassDef& c0 = raw->GetClassDef(0);
@@ -226,8 +226,8 @@ TEST_F(DexFileTest, ClassDefs) {
 
 TEST_F(DexFileTest, GetMethodSignature) {
   ScopedObjectAccess soa(Thread::Current());
-  const DexFile* raw(OpenTestDexFile("GetMethodSignature"));
-  ASSERT_TRUE(raw != NULL);
+  std::unique_ptr<const DexFile> raw(OpenTestDexFile("GetMethodSignature"));
+  ASSERT_TRUE(raw.get() != nullptr);
   EXPECT_EQ(1U, raw->NumClassDefs());
 
   const DexFile::ClassDef& class_def = raw->GetClassDef(0);
@@ -276,8 +276,8 @@ TEST_F(DexFileTest, GetMethodSignature) {
 
 TEST_F(DexFileTest, FindStringId) {
   ScopedObjectAccess soa(Thread::Current());
-  const DexFile* raw(OpenTestDexFile("GetMethodSignature"));
-  ASSERT_TRUE(raw != NULL);
+  std::unique_ptr<const DexFile> raw(OpenTestDexFile("GetMethodSignature"));
+  ASSERT_TRUE(raw.get() != nullptr);
   EXPECT_EQ(1U, raw->NumClassDefs());
 
   const char* strings[] = { "LGetMethodSignature;", "Ljava/lang/Float;", "Ljava/lang/Object;",

diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index ec1e5f0..00ca8a9 100644 (file)
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -101,8 +101,9 @@ static inline uint8_t* DecodeBase64(const char* src, size_t* dst_size) {
   return dst.release();
 }
 
-static const DexFile* OpenDexFileBase64(const char* base64, const char* location,
-                                        std::string* error_msg) {
+static std::unique_ptr<const DexFile> OpenDexFileBase64(const char* base64,
+                                                        const char* location,
+                                                        std::string* error_msg) {
   // decode base64
   CHECK(base64 != NULL);
   size_t length;
@@ -122,11 +123,11 @@ static const DexFile* OpenDexFileBase64(const char* base64, const char* location
 
   // read dex file
   ScopedObjectAccess soa(Thread::Current());
-  std::vector<const DexFile*> tmp;
+  std::vector<std::unique_ptr<const DexFile>> tmp;
   bool success = DexFile::Open(location, location, error_msg, &tmp);
   CHECK(success) << error_msg;
   EXPECT_EQ(1U, tmp.size());
-  const DexFile* dex_file = tmp[0];
+  std::unique_ptr<const DexFile> dex_file = std::move(tmp[0]);
   EXPECT_EQ(PROT_READ, dex_file->GetPermissions());
   EXPECT_TRUE(dex_file->IsReadOnly());
   return dex_file;
@@ -166,8 +167,9 @@ static void FixUpChecksum(uint8_t* dex_file) {
   header->checksum_ = adler_checksum;
 }
 
-static const DexFile* FixChecksumAndOpen(uint8_t* bytes, size_t length, const char* location,
-                                         std::string* error_msg) {
+static std::unique_ptr<const DexFile> FixChecksumAndOpen(uint8_t* bytes, size_t length,
+                                                         const char* location,
+                                                         std::string* error_msg) {
   // Check data.
   CHECK(bytes != nullptr);
 
@@ -187,12 +189,12 @@ static const DexFile* FixChecksumAndOpen(uint8_t* bytes, size_t length, const ch
 
   // read dex file
   ScopedObjectAccess soa(Thread::Current());
-  std::vector<const DexFile*> tmp;
+  std::vector<std::unique_ptr<const DexFile>> tmp;
   if (!DexFile::Open(location, location, error_msg, &tmp)) {
     return nullptr;
   }
   EXPECT_EQ(1U, tmp.size());
-  const DexFile* dex_file = tmp[0];
+  std::unique_ptr<const DexFile> dex_file = std::move(tmp[0]);
   EXPECT_EQ(PROT_READ, dex_file->GetPermissions());
   EXPECT_TRUE(dex_file->IsReadOnly());
   return dex_file;

diff --git a/runtime/mirror/dex_cache_test.cc b/runtime/mirror/dex_cache_test.cc
index ef6fc67..53e5534 100644 (file)
--- a/runtime/mirror/dex_cache_test.cc
+++ b/runtime/mirror/dex_cache_test.cc
@@ -34,6 +34,7 @@ class DexCacheTest : public CommonRuntimeTest {};
 TEST_F(DexCacheTest, Open) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<1> hs(soa.Self());
+  ASSERT_TRUE(java_lang_dex_file_ != NULL);
   Handle<DexCache> dex_cache(
       hs.NewHandle(class_linker_->AllocDexCache(soa.Self(), *java_lang_dex_file_)));
   ASSERT_TRUE(dex_cache.Get() != NULL);

diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc
index 44c6d87..037072d 100644 (file)
--- a/runtime/native/dalvik_system_DexFile.cc
+++ b/runtime/native/dalvik_system_DexFile.cc
@@ -115,7 +115,8 @@ static jlong DexFile_openDexFileNative(JNIEnv* env, jclass, jstring javaSourceNa
   }
 
   ClassLinker* linker = Runtime::Current()->GetClassLinker();
-  std::unique_ptr<std::vector<const DexFile*>> dex_files(new std::vector<const DexFile*>());
+  std::unique_ptr<std::vector<std::unique_ptr<const DexFile>>> dex_files(
+      new std::vector<std::unique_ptr<const DexFile>>());
   std::vector<std::string> error_msgs;
 
   bool success = linker->OpenDexFilesFromOat(sourceName.c_str(), outputName.c_str(), &error_msgs,
@@ -143,9 +144,11 @@ static jlong DexFile_openDexFileNative(JNIEnv* env, jclass, jstring javaSourceNa
   }
 }
 
-static std::vector<const DexFile*>* toDexFiles(jlong dex_file_address, JNIEnv* env) {
-  std::vector<const DexFile*>* dex_files = reinterpret_cast<std::vector<const DexFile*>*>(
-      static_cast<uintptr_t>(dex_file_address));
+static std::vector<std::unique_ptr<const DexFile>>*
+toDexFiles(jlong dex_file_address, JNIEnv* env) {
+  std::vector<std::unique_ptr<const DexFile>>* dex_files
+    = reinterpret_cast<std::vector<std::unique_ptr<const DexFile>>*>(
+        static_cast<uintptr_t>(dex_file_address));
   if (UNLIKELY(dex_files == nullptr)) {
     ScopedObjectAccess soa(env);
     ThrowNullPointerException(NULL, "dex_file == null");
@@ -154,27 +157,29 @@ static std::vector<const DexFile*>* toDexFiles(jlong dex_file_address, JNIEnv* e
 }
 
 static void DexFile_closeDexFile(JNIEnv* env, jclass, jlong cookie) {
-  std::unique_ptr<std::vector<const DexFile*>> dex_files(toDexFiles(cookie, env));
+  std::unique_ptr<std::vector<std::unique_ptr<const DexFile>>> dex_files(toDexFiles(cookie, env));
   if (dex_files.get() == nullptr) {
     return;
   }
   ScopedObjectAccess soa(env);
 
-  size_t index = 0;
-  for (const DexFile* dex_file : *dex_files) {
+  // The Runtime currently never unloads classes, which means any registered
+  // dex files must be kept around forever in case they are used. We
+  // accomplish this here by explicitly leaking those dex files that are
+  // registered.
+  //
+  // TODO: The Runtime should support unloading of classes and freeing of the
+  // dex files for those unloaded classes rather than leaking dex files here.
+  for (auto& dex_file : *dex_files) {
     if (Runtime::Current()->GetClassLinker()->IsDexFileRegistered(*dex_file)) {
-      (*dex_files)[index] = nullptr;
+      dex_file.release();
     }
-    index++;
   }
-
-  STLDeleteElements(dex_files.get());
-  // Unique_ptr will delete the vector itself.
 }
 
 static jclass DexFile_defineClassNative(JNIEnv* env, jclass, jstring javaName, jobject javaLoader,
                                         jlong cookie) {
-  std::vector<const DexFile*>* dex_files = toDexFiles(cookie, env);
+  std::vector<std::unique_ptr<const DexFile>>* dex_files = toDexFiles(cookie, env);
   if (dex_files == NULL) {
     VLOG(class_linker) << "Failed to find dex_file";
     return NULL;
@@ -186,7 +191,7 @@ static jclass DexFile_defineClassNative(JNIEnv* env, jclass, jstring javaName, j
   }
   const std::string descriptor(DotToDescriptor(class_name.c_str()));
   const size_t hash(ComputeModifiedUtf8Hash(descriptor.c_str()));
-  for (const DexFile* dex_file : *dex_files) {
+  for (auto& dex_file : *dex_files) {
     const DexFile::ClassDef* dex_class_def = dex_file->FindClassDef(descriptor.c_str(), hash);
     if (dex_class_def != nullptr) {
       ScopedObjectAccess soa(env);
@@ -218,13 +223,13 @@ struct CharPointerComparator {
 // Note: this can be an expensive call, as we sort out duplicates in MultiDex files.
 static jobjectArray DexFile_getClassNameList(JNIEnv* env, jclass, jlong cookie) {
   jobjectArray result = nullptr;
-  std::vector<const DexFile*>* dex_files = toDexFiles(cookie, env);
+  std::vector<std::unique_ptr<const DexFile>>* dex_files = toDexFiles(cookie, env);
 
   if (dex_files != nullptr) {
     // Push all class descriptors into a set. Use set instead of unordered_set as we want to
     // retrieve all in the end.
     std::set<const char*, CharPointerComparator> descriptors;
-    for (const DexFile* dex_file : *dex_files) {
+    for (auto& dex_file : *dex_files) {
       for (size_t i = 0; i < dex_file->NumClassDefs(); ++i) {
         const DexFile::ClassDef& class_def = dex_file->GetClassDef(i);
         const char* descriptor = dex_file->GetClassDescriptor(class_def);

diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 1c6cc8b..358519b 100644 (file)
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -454,7 +454,7 @@ size_t OatFile::OatDexFile::FileSize() const {
   return reinterpret_cast<const DexFile::Header*>(dex_file_pointer_)->file_size_;
 }
 
-const DexFile* OatFile::OatDexFile::OpenDexFile(std::string* error_msg) const {
+std::unique_ptr<const DexFile> OatFile::OatDexFile::OpenDexFile(std::string* error_msg) const {
   return DexFile::Open(dex_file_pointer_, FileSize(), dex_file_location_,
                        dex_file_location_checksum_, error_msg);
 }

diff --git a/runtime/oat_file.h b/runtime/oat_file.h
index 831ba1e..6ae3c3e 100644 (file)
--- a/runtime/oat_file.h
+++ b/runtime/oat_file.h
@@ -210,7 +210,7 @@ class OatFile {
   class OatDexFile {
    public:
     // Opens the DexFile referred to by this OatDexFile from within the containing OatFile.
-    const DexFile* OpenDexFile(std::string* error_msg) const;
+    std::unique_ptr<const DexFile> OpenDexFile(std::string* error_msg) const;
 
     const OatFile* GetOatFile() const {
       return oat_file_;

diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index fb6034d..c553d54 100644 (file)
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -625,8 +625,9 @@ void Runtime::StartDaemonThreads() {
 }
 
 static bool OpenDexFilesFromImage(const std::string& image_location,
-                                  std::vector<const DexFile*>& dex_files,
+                                  std::vector<std::unique_ptr<const DexFile>>* dex_files,
                                   size_t* failures) {
+  DCHECK(dex_files != nullptr) << "OpenDexFilesFromImage: out-param is NULL";
   std::string system_filename;
   bool has_system = false;
   std::string cache_filename_unused;
@@ -670,11 +671,11 @@ static bool OpenDexFilesFromImage(const std::string& image_location,
       *failures += 1;
       continue;
     }
-    const DexFile* dex_file = oat_dex_file->OpenDexFile(&error_msg);
-    if (dex_file == nullptr) {
+    std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error_msg);
+    if (dex_file.get() == nullptr) {
       *failures += 1;
     } else {
-      dex_files.push_back(dex_file);
+      dex_files->push_back(std::move(dex_file));
     }
   }
   Runtime::Current()->GetClassLinker()->RegisterOatFile(oat_file.release());
@@ -685,7 +686,8 @@ static bool OpenDexFilesFromImage(const std::string& image_location,
 static size_t OpenDexFiles(const std::vector<std::string>& dex_filenames,
                            const std::vector<std::string>& dex_locations,
                            const std::string& image_location,
-                           std::vector<const DexFile*>& dex_files) {
+                           std::vector<std::unique_ptr<const DexFile>>* dex_files) {
+  DCHECK(dex_files != nullptr) << "OpenDexFiles: out-param is NULL";
   size_t failure_count = 0;
   if (!image_location.empty() && OpenDexFilesFromImage(image_location, dex_files, &failure_count)) {
     return failure_count;
@@ -699,7 +701,7 @@ static size_t OpenDexFiles(const std::vector<std::string>& dex_filenames,
       LOG(WARNING) << "Skipping non-existent dex file '" << dex_filename << "'";
       continue;
     }
-    if (!DexFile::Open(dex_filename, dex_location, &error_msg, &dex_files)) {
+    if (!DexFile::Open(dex_filename, dex_location, &error_msg, dex_files)) {
       LOG(WARNING) << "Failed to open .dex from file '" << dex_filename << "': " << error_msg;
       ++failure_count;
     }
@@ -877,9 +879,9 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized)
       CHECK_EQ(dex_filenames.size(), dex_locations.size());
     }
 
-    std::vector<const DexFile*> boot_class_path;
-    OpenDexFiles(dex_filenames, dex_locations, options->image_, boot_class_path);
-    class_linker_->InitWithoutImage(boot_class_path);
+    std::vector<std::unique_ptr<const DexFile>> boot_class_path;
+    OpenDexFiles(dex_filenames, dex_locations, options->image_, &boot_class_path);
+    class_linker_->InitWithoutImage(std::move(boot_class_path));
     // TODO: Should we move the following to InitWithoutImage?
     SetInstructionSet(kRuntimeISA);
     for (int i = 0; i < Runtime::kLastCalleeSaveType; i++) {

diff --git a/runtime/runtime.h b/runtime/runtime.h
index e319963..d58fe3c 100644 (file)
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -425,6 +425,9 @@ class Runtime {
       LOCKS_EXCLUDED(method_verifier_lock_);
 
   const std::vector<const DexFile*>& GetCompileTimeClassPath(jobject class_loader);
+
+  // The caller is responsible for ensuring the class_path DexFiles remain
+  // valid as long as the Runtime object remains valid.
   void SetCompileTimeClassPath(jobject class_loader, std::vector<const DexFile*>& class_path);
 
   void StartProfiler(const char* profile_output_filename);

diff --git a/runtime/verifier/method_verifier_test.cc b/runtime/verifier/method_verifier_test.cc
index 770ca7e..f67adc1 100644 (file)
--- a/runtime/verifier/method_verifier_test.cc
+++ b/runtime/verifier/method_verifier_test.cc
@@ -41,14 +41,12 @@ class MethodVerifierTest : public CommonRuntimeTest {
         << error_msg;
   }
 
-  void VerifyDexFile(const DexFile* dex)
+  void VerifyDexFile(const DexFile& dex)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-    ASSERT_TRUE(dex != NULL);
-
     // Verify all the classes defined in this file
-    for (size_t i = 0; i < dex->NumClassDefs(); i++) {
-      const DexFile::ClassDef& class_def = dex->GetClassDef(i);
-      const char* descriptor = dex->GetClassDescriptor(class_def);
+    for (size_t i = 0; i < dex.NumClassDefs(); i++) {
+      const DexFile::ClassDef& class_def = dex.GetClassDef(i);
+      const char* descriptor = dex.GetClassDescriptor(class_def);
       VerifyClass(descriptor);
     }
   }
@@ -56,7 +54,8 @@ class MethodVerifierTest : public CommonRuntimeTest {
 
 TEST_F(MethodVerifierTest, LibCore) {
   ScopedObjectAccess soa(Thread::Current());
-  VerifyDexFile(java_lang_dex_file_);
+  ASSERT_TRUE(java_lang_dex_file_ != nullptr);
+  VerifyDexFile(*java_lang_dex_file_);
 }
 
 }  // namespace verifier