oat_file_manager: Avoid global collision check.
Avoid the global collision check in case we encounter a classloader
that we don't recognize. This seems overly conservative. Instead, trust
that the classloader does the right thing.
Bug: 36480683
Bug: 37777332
Test: make test-art-host
Test: manual testing
Change-Id: I9cf026baab4264a0e4eab3710ab3c15df7c01edf
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 6799918..932d5ed 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -546,8 +546,8 @@
std::vector<const DexFile*> dex_files_loaded;
// Try to get dex files from the given class loader. If the class loader is null, or we do
- // not support one of the class loaders in the chain, conservatively compare against all
- // (non-boot) oat files.
+ // not support one of the class loaders in the chain, we do nothing and assume the collision
+ // check has succeeded.
bool class_loader_ok = false;
{
ScopedObjectAccess soa(Thread::Current());
@@ -566,37 +566,20 @@
} else if (h_class_loader != nullptr) {
VLOG(class_linker) << "Something unsupported with "
<< mirror::Class::PrettyClass(h_class_loader->GetClass());
+
+ // This is a class loader we don't recognize. Our earlier strategy would
+ // be to perform a global duplicate class check (with all loaded oat files)
+ // but that seems overly conservative - we have no way of knowing that
+ // those files are present in the same loader hierarchy. Among other
+ // things, it hurt GMS core and its filtering class loader.
}
}
- // Dex files are registered late - once a class is actually being loaded. We have to compare
- // against the open oat files. Take the oat_file_manager_lock_ that protects oat_files_ accesses.
- ReaderMutexLock mu(Thread::Current(), *Locks::oat_file_manager_lock_);
-
- // Vector that holds the newly opened dex files live, this is done to prevent leaks.
- std::vector<std::unique_ptr<const DexFile>> opened_dex_files;
-
+ // Exit if we find a class loader we don't recognize. Proceed to check shared
+ // libraries and do a full class loader check otherwise.
if (!class_loader_ok) {
- // Add dex files from already loaded oat files, but skip boot.
-
- // Clean up the dex files.
- dex_files_loaded.clear();
-
- std::vector<const OatFile*> boot_oat_files = GetBootOatFiles();
- // The same OatFile can be loaded multiple times at different addresses. In this case, we don't
- // need to check both against each other since they would have resolved the same way at compile
- // time.
- std::unordered_set<std::string> unique_locations;
- for (const std::unique_ptr<const OatFile>& loaded_oat_file : oat_files_) {
- DCHECK_NE(loaded_oat_file.get(), oat_file);
- const std::string& location = loaded_oat_file->GetLocation();
- if (std::find(boot_oat_files.begin(), boot_oat_files.end(), loaded_oat_file.get()) ==
- boot_oat_files.end() && location != oat_file->GetLocation() &&
- unique_locations.find(location) == unique_locations.end()) {
- unique_locations.insert(location);
- AddDexFilesFromOat(loaded_oat_file.get(), &dex_files_loaded, &opened_dex_files);
- }
- }
+ LOG(WARNING) << "Skipping duplicate class check due to unrecognized classloader";
+ return false;
}
// Exit if shared libraries are ok. Do a full duplicate classes check otherwise.
@@ -606,6 +589,9 @@
return false;
}
+ // Vector that holds the newly opened dex files live, this is done to prevent leaks.
+ std::vector<std::unique_ptr<const DexFile>> opened_dex_files;
+
ScopedTrace st("Collision check");
// Add dex files from the oat file to check.
std::vector<const DexFile*> dex_files_unloaded;
@@ -677,21 +663,34 @@
if (!accept_oat_file) {
// Failed the collision check. Print warning.
if (Runtime::Current()->IsDexFileFallbackEnabled()) {
- LOG(WARNING) << "Found duplicate classes, falling back to interpreter mode for "
- << dex_location;
+ if (!oat_file_assistant.HasOriginalDexFiles()) {
+ // We need to fallback but don't have original dex files. We have to
+ // fallback to opening the existing oat file. This is potentially
+ // unsafe so we warn about it.
+ accept_oat_file = true;
+
+ LOG(WARNING) << "Dex location " << dex_location << " does not seem to include dex file. "
+ << "Allow oat file use. This is potentially dangerous.";
+ } else {
+ // We have to fallback and found original dex files - extract them from an APK.
+ // Also warn about this operation because it's potentially wasteful.
+ LOG(WARNING) << "Found duplicate classes, falling back to extracting from APK : "
+ << dex_location;
+ LOG(WARNING) << "NOTE: This wastes RAM and hurts startup performance.";
+ }
} else {
+ // TODO: We should remove this. The fact that we're here implies -Xno-dex-file-fallback
+ // was set, which means that we should never fallback. If we don't have original dex
+ // files, we should just fail resolution as the flag intended.
+ if (!oat_file_assistant.HasOriginalDexFiles()) {
+ accept_oat_file = true;
+ }
+
LOG(WARNING) << "Found duplicate classes, dex-file-fallback disabled, will be failing to "
" load classes for " << dex_location;
}
- LOG(WARNING) << error_msg;
- // However, if the app was part of /system and preopted, there is no original dex file
- // available. In that case grudgingly accept the oat file.
- if (!oat_file_assistant.HasOriginalDexFiles()) {
- accept_oat_file = true;
- LOG(WARNING) << "Dex location " << dex_location << " does not seem to include dex file. "
- << "Allow oat file use. This is potentially dangerous.";
- }
+ LOG(WARNING) << error_msg;
}
if (accept_oat_file) {