From 39c86bc9de106e3641ecab2374a24e41d0430694 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Wed, 16 Jul 2014 14:45:03 +0100 Subject: [PATCH] Make ART fail gracefully when it can't update the desired code. ART was exiting with a fatal error when it couldn't clean an obsolete file. Relaxing this and failing gracefully preserves the behaviour that Dalvik had. Bug: 15313272 (cherry picked from commit c54aea7f4acd1a32bb298d43c20e3e0217638926) Change-Id: I862a8925a0edd6370e94af8fa984a64099240029 --- runtime/class_linker.cc | 28 +++++++++++++++++++++++++--- runtime/class_linker.h | 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 2e51cf83b..c4855bd63 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -824,9 +824,20 @@ bool ClassLinker::OpenDexFilesFromOat(const char* dex_location, const char* oat_ } } else { // TODO: What to lock here? + bool obsolete_file_cleanup_failed; open_oat_file.reset(FindOatFileContainingDexFileFromDexLocation(dex_location, dex_location_checksum_pointer, - kRuntimeISA, error_msgs)); + kRuntimeISA, error_msgs, + &obsolete_file_cleanup_failed)); + // There's no point in going forward and eventually try to regenerate the + // file if we couldn't remove the obsolete one. Mostly likely we will fail + // with the same error when trying to write the new file. + // In case the clean up failure is due to permission issues it's *mandatory* + // to stop to avoid regenerating under the wrong user. + // TODO: should we maybe do this only when we get permission issues? (i.e. EACCESS). + if (obsolete_file_cleanup_failed) { + return false; + } } needs_registering = true; } @@ -1084,7 +1095,9 @@ const OatFile* ClassLinker::FindOatFileContainingDexFileFromDexLocation( const char* dex_location, const uint32_t* const dex_location_checksum, InstructionSet isa, - std::vector* error_msgs) { + std::vector* error_msgs, + bool* obsolete_file_cleanup_failed) { + *obsolete_file_cleanup_failed = false; // Look for an existing file next to dex. for example, for // /foo/bar/baz.jar, look for /foo/bar//baz.odex. std::string odex_filename(DexFilenameToOdexFilename(dex_location, isa)); @@ -1111,9 +1124,18 @@ const OatFile* ClassLinker::FindOatFileContainingDexFileFromDexLocation( if (oat_file != nullptr) { return oat_file; } + if (!open_failed && TEMP_FAILURE_RETRY(unlink(cache_location.c_str())) != 0) { - PLOG(FATAL) << "Failed to remove obsolete oat file from " << cache_location; + std::string error_msg = StringPrintf("Failed to remove obsolete file from %s when searching" + "for dex file %s: %s", + cache_location.c_str(), dex_location, strerror(errno)); + error_msgs->push_back(error_msg); + VLOG(class_linker) << error_msg; + // Let the caller know that we couldn't remove the obsolete file. + // This is a good indication that further writes may fail as well. + *obsolete_file_cleanup_failed = true; } + std::string compound_msg = StringPrintf("Failed to open oat file from %s (error '%s') or %s " "(error '%s').", odex_filename.c_str(), error_msg.c_str(), cache_location.c_str(), cache_error_msg.c_str()); diff --git a/runtime/class_linker.h b/runtime/class_linker.h index d6cceffbb..c17f88d6d 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -571,7 +571,8 @@ class ClassLinker { const OatFile* FindOatFileContainingDexFileFromDexLocation(const char* location, const uint32_t* const location_checksum, InstructionSet isa, - std::vector* error_msgs) + std::vector* error_msgs, + bool* obsolete_file_cleanup_failed) LOCKS_EXCLUDED(dex_lock_, Locks::mutator_lock_); // Find a verify an oat file with the given dex file. Will return nullptr when the oat file -- 2.11.0