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

Change-Id: Id202bbac3318bade89a4133a9bcb1ee01e8b6182

diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 71b238b..6f689d7 100644 (file)
--- 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 3dca12a..3d7624d 100644 (file)
--- 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 ec28685..995ea99 100644 (file)
--- 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<void*> 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<void*> 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  // ART_TARGET_ANDROID
   }
   if (dlopen_handle_ == nullptr) {

diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 0af6716..b5fdcbd 100644 (file)
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -728,28 +728,6 @@ std::vector<std::unique_ptr<const DexFile>> 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<std::string, size_t>(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<const OatFile*> boot_oat_files = GetBootOatFiles();

diff --git a/runtime/oat_file_manager.h b/runtime/oat_file_manager.h
index a1d1275..45ac4b7 100644 (file)
--- a/runtime/oat_file_manager.h
+++ b/runtime/oat_file_manager.h
@@ -64,16 +64,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_;
@@ -132,7 +122,6 @@ class OatFileManager {
       REQUIRES(Locks::oat_file_manager_lock_);
 
   std::set<std::unique_ptr<const OatFile>> oat_files_ GUARDED_BY(Locks::oat_file_manager_lock_);
-  std::unordered_map<std::string, size_t> oat_file_count_ GUARDED_BY(Locks::oat_file_count_lock_);
   bool have_non_pic_oat_file_;
 
   DISALLOW_COPY_AND_ASSIGN(OatFileManager);