From af0e482f81ed77c86b0e67ad5f1a7f7dbefda348 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Tue, 24 May 2016 15:04:22 -0700 Subject: [PATCH] Don't use dlopen on host for already loaded oat files. Because the behavior of dlopen on the host is different then the target in that case, and it causes tests to fail. Bug: 28826195 (cherry picked from commit a206c745dbb64b14f05c87891d425475c2f6d63a) Change-Id: I29b04be07b4d26dc1ac5e6f35550745eb15e6728 --- runtime/base/mutex.cc | 10 +++---- runtime/base/mutex.h | 8 +++--- runtime/oat_file.cc | 63 +++++++++++++++++++++------------------------ runtime/oat_file_manager.cc | 22 ---------------- runtime/oat_file_manager.h | 11 -------- 5 files changed, 39 insertions(+), 75 deletions(-) diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc index 620bf9c8b..d1713ed9b 100644 --- a/runtime/base/mutex.cc +++ b/runtime/base/mutex.cc @@ -49,7 +49,7 @@ Mutex* Locks::modify_ldt_lock_ = nullptr; MutatorMutex* Locks::mutator_lock_ = nullptr; Mutex* Locks::profiler_lock_ = nullptr; ReaderWriterMutex* Locks::oat_file_manager_lock_ = nullptr; -ReaderWriterMutex* Locks::oat_file_count_lock_ = nullptr; +Mutex* Locks::host_dlopen_handles_lock_ = nullptr; Mutex* Locks::reference_processor_lock_ = nullptr; Mutex* Locks::reference_queue_cleared_references_lock_ = nullptr; Mutex* Locks::reference_queue_finalizer_references_lock_ = nullptr; @@ -953,7 +953,7 @@ void Locks::Init() { DCHECK(deoptimization_lock_ != nullptr); DCHECK(heap_bitmap_lock_ != nullptr); DCHECK(oat_file_manager_lock_ != nullptr); - DCHECK(oat_file_count_lock_ != nullptr); + DCHECK(host_dlopen_handles_lock_ != nullptr); DCHECK(intern_table_lock_ != nullptr); DCHECK(jni_libraries_lock_ != nullptr); DCHECK(logging_lock_ != nullptr); @@ -1042,9 +1042,9 @@ void Locks::Init() { DCHECK(oat_file_manager_lock_ == nullptr); oat_file_manager_lock_ = new ReaderWriterMutex("OatFile manager lock", current_lock_level); - UPDATE_CURRENT_LOCK_LEVEL(kOatFileCountLock); - DCHECK(oat_file_count_lock_ == nullptr); - oat_file_count_lock_ = new ReaderWriterMutex("OatFile count lock", current_lock_level); + UPDATE_CURRENT_LOCK_LEVEL(kHostDlOpenHandlesLock); + DCHECK(host_dlopen_handles_lock_ == nullptr); + host_dlopen_handles_lock_ = new Mutex("host dlopen handles lock", current_lock_level); UPDATE_CURRENT_LOCK_LEVEL(kInternTableLock); DCHECK(intern_table_lock_ == nullptr); diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h index 3dca12a36..3d7624d97 100644 --- a/runtime/base/mutex.h +++ b/runtime/base/mutex.h @@ -83,7 +83,7 @@ enum LockLevel { kDexFileToMethodInlinerMapLock, kInternTableLock, kOatFileSecondaryLookupLock, - kOatFileCountLock, + kHostDlOpenHandlesLock, kOatFileManagerLock, kTracingUniqueMethodsLock, kTracingStreamingLock, @@ -651,11 +651,11 @@ class Locks { // Guards opened oat files in OatFileManager. static ReaderWriterMutex* oat_file_manager_lock_ ACQUIRED_AFTER(modify_ldt_lock_); - // Guards opened oat files in OatFileManager. - static ReaderWriterMutex* oat_file_count_lock_ ACQUIRED_AFTER(oat_file_manager_lock_); + // Guards dlopen_handles_ in DlOpenOatFile. + static Mutex* host_dlopen_handles_lock_ ACQUIRED_AFTER(oat_file_manager_lock_); // Guards intern table. - static Mutex* intern_table_lock_ ACQUIRED_AFTER(oat_file_count_lock_); + static Mutex* intern_table_lock_ ACQUIRED_AFTER(host_dlopen_handles_lock_); // Guards reference processor. static Mutex* reference_processor_lock_ ACQUIRED_AFTER(intern_table_lock_); diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index 6ef00b961..0acd37f79 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -490,40 +490,23 @@ bool OatFileBase::Setup(const char* abs_dex_location, std::string* error_msg) { // OatFile via dlopen // //////////////////////// -static bool RegisterOatFileLocation(const std::string& location) { - if (!kIsTargetBuild) { - Runtime* const runtime = Runtime::Current(); - if (runtime != nullptr && !runtime->IsAotCompiler()) { - return runtime->GetOatFileManager().RegisterOatFileLocation(location); - } - return false; - } - return true; -} - -static void UnregisterOatFileLocation(const std::string& location) { - if (!kIsTargetBuild) { - Runtime* const runtime = Runtime::Current(); - if (runtime != nullptr && !runtime->IsAotCompiler()) { - runtime->GetOatFileManager().UnRegisterOatFileLocation(location); - } - } -} - class DlOpenOatFile FINAL : public OatFileBase { public: DlOpenOatFile(const std::string& filename, bool executable) : OatFileBase(filename, executable), dlopen_handle_(nullptr), - shared_objects_before_(0), - first_oat_(RegisterOatFileLocation(filename)) { + shared_objects_before_(0) { } ~DlOpenOatFile() { if (dlopen_handle_ != nullptr) { dlclose(dlopen_handle_); + + if (!kIsTargetBuild) { + MutexLock mu(Thread::Current(), *Locks::host_dlopen_handles_lock_); + host_dlopen_handles_.erase(dlopen_handle_); + } } - UnregisterOatFileLocation(GetLocation()); } protected: @@ -554,6 +537,17 @@ class DlOpenOatFile FINAL : public OatFileBase { uint8_t* oat_file_begin, std::string* error_msg); + // On the host, if the same library is loaded again with dlopen the same + // file handle is returned. This differs from the behavior of dlopen on the + // target, where dlopen reloads the library at a different address every + // time you load it. The runtime relies on the target behavior to ensure + // each instance of the loaded library has a unique dex cache. To avoid + // problems, we fall back to our own linker in the case when the same + // library is opened multiple times on host. dlopen_handles_ is used to + // detect that case. + // Guarded by host_dlopen_handles_lock_; + static std::unordered_set host_dlopen_handles_; + // dlopen handle during runtime. void* dlopen_handle_; // TODO: Unique_ptr with custom deleter. @@ -564,12 +558,11 @@ class DlOpenOatFile FINAL : public OatFileBase { // (optimistically) optimize the PreSetup stage (see comment there). size_t shared_objects_before_; - // Track the registration status (= was this the first oat file) for the location. - const bool first_oat_; - DISALLOW_COPY_AND_ASSIGN(DlOpenOatFile); }; +std::unordered_set DlOpenOatFile::host_dlopen_handles_; + void DlOpenOatFile::PreLoad() { #ifdef __APPLE__ UNUSED(shared_objects_before_); @@ -628,12 +621,6 @@ bool DlOpenOatFile::Load(const std::string& elf_filename, *error_msg = "DlOpen disabled for host."; return false; } - // For RAII, tracking multiple loads is done in the constructor and destructor. The result is - // stored in the first_oat_ flag. - if (!first_oat_) { - *error_msg = "Loading oat files multiple times with dlopen not supported on host."; - return false; - } } bool success = Dlopen(elf_filename, oat_file_begin, error_msg); @@ -671,8 +658,18 @@ bool DlOpenOatFile::Dlopen(const std::string& elf_filename, } // (pic boot image). dlopen_handle_ = android_dlopen_ext(absolute_path.get(), RTLD_NOW, &extinfo); #else - dlopen_handle_ = dlopen(absolute_path.get(), RTLD_NOW); UNUSED(oat_file_begin); + static_assert(!kIsTargetBuild, "host_dlopen_handles_ will leak handles"); + dlopen_handle_ = dlopen(absolute_path.get(), RTLD_NOW); + if (dlopen_handle_ != nullptr) { + MutexLock mu(Thread::Current(), *Locks::host_dlopen_handles_lock_); + if (!host_dlopen_handles_.insert(dlopen_handle_).second) { + dlclose(dlopen_handle_); + dlopen_handle_ = nullptr; + *error_msg = StringPrintf("host dlopen re-opened '%s'", elf_filename.c_str()); + return false; + } + } #endif } if (dlopen_handle_ == nullptr) { diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc index aff479f15..42596e329 100644 --- a/runtime/oat_file_manager.cc +++ b/runtime/oat_file_manager.cc @@ -731,28 +731,6 @@ std::vector> OatFileManager::OpenDexFilesFromOat( return dex_files; } -bool OatFileManager::RegisterOatFileLocation(const std::string& oat_location) { - WriterMutexLock mu(Thread::Current(), *Locks::oat_file_count_lock_); - auto it = oat_file_count_.find(oat_location); - if (it != oat_file_count_.end()) { - ++it->second; - return false; - } - oat_file_count_.insert(std::pair(oat_location, 1u)); - return true; -} - -void OatFileManager::UnRegisterOatFileLocation(const std::string& oat_location) { - WriterMutexLock mu(Thread::Current(), *Locks::oat_file_count_lock_); - auto it = oat_file_count_.find(oat_location); - if (it != oat_file_count_.end()) { - --it->second; - if (it->second == 0) { - oat_file_count_.erase(it); - } - } -} - void OatFileManager::DumpForSigQuit(std::ostream& os) { ReaderMutexLock mu(Thread::Current(), *Locks::oat_file_manager_lock_); std::vector boot_oat_files = GetBootOatFiles(); diff --git a/runtime/oat_file_manager.h b/runtime/oat_file_manager.h index ad8c0520c..1d5b87285 100644 --- a/runtime/oat_file_manager.h +++ b/runtime/oat_file_manager.h @@ -65,16 +65,6 @@ class OatFileManager { const OatFile* FindOpenedOatFileFromDexLocation(const std::string& dex_base_location) const REQUIRES(!Locks::oat_file_manager_lock_); - // Attempt to reserve a location, returns false if it is already reserved or already in used by - // an oat file. - bool RegisterOatFileLocation(const std::string& oat_location) - REQUIRES(!Locks::oat_file_count_lock_); - - // Unreserve oat file location, should only be used for error cases since RegisterOatFile will - // remove the reserved location. - void UnRegisterOatFileLocation(const std::string& oat_location) - REQUIRES(!Locks::oat_file_count_lock_); - // Returns true if we have a non pic oat file. bool HaveNonPicOatFile() const { return have_non_pic_oat_file_; @@ -137,7 +127,6 @@ class OatFileManager { REQUIRES(Locks::oat_file_manager_lock_); std::set> oat_files_ GUARDED_BY(Locks::oat_file_manager_lock_); - std::unordered_map oat_file_count_ GUARDED_BY(Locks::oat_file_count_lock_); bool have_non_pic_oat_file_; // The compiler filter used for oat files loaded by the oat file manager. -- 2.11.0