diff options
-rw-r--r-- | compiler/image_test.cc | 5 | ||||
-rw-r--r-- | compiler/image_writer.cc | 12 | ||||
-rw-r--r-- | dex2oat/dex2oat.cc | 56 | ||||
-rw-r--r-- | oatdump/oatdump.cc | 4 | ||||
-rw-r--r-- | patchoat/patchoat.cc | 26 | ||||
-rw-r--r-- | runtime/Android.mk | 1 | ||||
-rw-r--r-- | runtime/base/scoped_flock.cc | 8 | ||||
-rw-r--r-- | runtime/base/unix_file/fd_file.cc | 105 | ||||
-rw-r--r-- | runtime/base/unix_file/fd_file.h | 59 | ||||
-rw-r--r-- | runtime/base/unix_file/fd_file_test.cc | 5 | ||||
-rw-r--r-- | runtime/base/unix_file/random_access_file_test.h | 9 | ||||
-rw-r--r-- | runtime/base/unix_file/random_access_file_utils_test.cc | 4 | ||||
-rw-r--r-- | runtime/common_runtime_test.cc | 9 | ||||
-rw-r--r-- | runtime/dex_file_test.cc | 3 | ||||
-rw-r--r-- | runtime/dex_file_verifier_test.cc | 6 | ||||
-rw-r--r-- | runtime/hprof/hprof.cc | 9 | ||||
-rw-r--r-- | runtime/signal_catcher.cc | 12 | ||||
-rw-r--r-- | runtime/trace.cc | 9 | ||||
-rw-r--r-- | runtime/zip_archive_test.cc | 2 |
19 files changed, 299 insertions, 45 deletions
diff --git a/compiler/image_test.cc b/compiler/image_test.cc index 7e2be3ee0c..dac1ef4c06 100644 --- a/compiler/image_test.cc +++ b/compiler/image_test.cc @@ -105,13 +105,16 @@ TEST_F(ImageTest, WriteRead) { ASSERT_TRUE(success_image); bool success_fixup = ElfWriter::Fixup(dup_oat.get(), writer->GetOatDataBegin()); ASSERT_TRUE(success_fixup); + + ASSERT_EQ(dup_oat->FlushCloseOrErase(), 0) << "Could not flush and close oat file " + << oat_file.GetFilename(); } { std::unique_ptr<File> file(OS::OpenFileForReading(image_file.GetFilename().c_str())); ASSERT_TRUE(file.get() != NULL); ImageHeader image_header; - file->ReadFully(&image_header, sizeof(image_header)); + ASSERT_EQ(file->ReadFully(&image_header, sizeof(image_header)), true); ASSERT_TRUE(image_header.IsValid()); ASSERT_GE(image_header.GetImageBitmapOffset(), sizeof(image_header)); ASSERT_NE(0U, image_header.GetImageBitmapSize()); diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 64d2de1b45..ef1bf81faa 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -149,6 +149,11 @@ bool ImageWriter::Write(const std::string& image_filename, SetOatChecksumFromElfFile(oat_file.get()); + if (oat_file->FlushCloseOrErase() != 0) { + LOG(ERROR) << "Failed to flush and close oat file " << oat_filename << " for " << oat_location; + return false; + } + std::unique_ptr<File> image_file(OS::CreateEmptyFile(image_filename.c_str())); ImageHeader* image_header = reinterpret_cast<ImageHeader*>(image_->Begin()); if (image_file.get() == NULL) { @@ -157,6 +162,7 @@ bool ImageWriter::Write(const std::string& image_filename, } if (fchmod(image_file->Fd(), 0644) != 0) { PLOG(ERROR) << "Failed to make image file world readable: " << image_filename; + image_file->Erase(); return EXIT_FAILURE; } @@ -164,6 +170,7 @@ bool ImageWriter::Write(const std::string& image_filename, CHECK_EQ(image_end_, image_header->GetImageSize()); if (!image_file->WriteFully(image_->Begin(), image_end_)) { PLOG(ERROR) << "Failed to write image file " << image_filename; + image_file->Erase(); return false; } @@ -173,9 +180,14 @@ bool ImageWriter::Write(const std::string& image_filename, image_header->GetImageBitmapSize(), image_header->GetImageBitmapOffset())) { PLOG(ERROR) << "Failed to write image file " << image_filename; + image_file->Erase(); return false; } + if (image_file->FlushCloseOrErase() != 0) { + PLOG(ERROR) << "Failed to flush and close image file " << image_filename; + return false; + } return true; } diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 2d2a82e985..927c5f571f 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -942,9 +942,11 @@ class Dex2Oat FINAL { oat_location_ = oat_filename_; } } else { - oat_file_.reset(new File(oat_fd_, oat_location_)); + oat_file_.reset(new File(oat_fd_, oat_location_, true)); oat_file_->DisableAutoClose(); - oat_file_->SetLength(0); + if (oat_file_->SetLength(0) != 0) { + PLOG(WARNING) << "Truncating oat file " << oat_location_ << " failed."; + } } if (oat_file_.get() == nullptr) { PLOG(ERROR) << "Failed to create oat file: " << oat_location_; @@ -952,6 +954,7 @@ class Dex2Oat FINAL { } if (create_file && fchmod(oat_file_->Fd(), 0644) != 0) { PLOG(ERROR) << "Failed to make oat file world readable: " << oat_location_; + oat_file_->Erase(); return false; } return true; @@ -1075,7 +1078,10 @@ class Dex2Oat FINAL { << ". Try: adb shell chmod 777 /data/local/tmp"; continue; } - tmp_file->WriteFully(dex_file->Begin(), dex_file->Size()); + // This is just dumping files for debugging. Ignore errors, and leave remnants. + UNUSED(tmp_file->WriteFully(dex_file->Begin(), dex_file->Size())); + UNUSED(tmp_file->Flush()); + UNUSED(tmp_file->Close()); LOG(INFO) << "Wrote input to " << tmp_file_name; } } @@ -1266,6 +1272,7 @@ class Dex2Oat FINAL { if (!driver_->WriteElf(android_root_, is_host_, dex_files_, oat_writer.get(), oat_file_.get())) { LOG(ERROR) << "Failed to write ELF file " << oat_file_->GetPath(); + oat_file_->Erase(); return false; } } @@ -1273,8 +1280,8 @@ class Dex2Oat FINAL { // Flush result to disk. { TimingLogger::ScopedTiming t2("dex2oat Flush ELF", timings_); - if (oat_file_->Flush() != 0) { - LOG(ERROR) << "Failed to flush ELF file " << oat_file_->GetPath(); + if (oat_file_->FlushCloseOrErase() != 0) { + PLOG(ERROR) << "Failed to flush ELF file " << oat_file_->GetPath(); return false; } } @@ -1302,7 +1309,13 @@ class Dex2Oat FINAL { // We need to strip after image creation because FixupElf needs to use .strtab. if (oat_unstripped_ != oat_stripped_) { TimingLogger::ScopedTiming t("dex2oat OatFile copy", timings_); - oat_file_.reset(); + if (kUsePortableCompiler) { + if (oat_file_->FlushCloseOrErase() != 0) { + PLOG(ERROR) << "Failed to flush and close oat file: " << oat_location_; + return EXIT_FAILURE; + } + oat_file_.reset(); + } std::unique_ptr<File> in(OS::OpenFileForReading(oat_unstripped_.c_str())); std::unique_ptr<File> out(OS::CreateEmptyFile(oat_stripped_.c_str())); size_t buffer_size = 8192; @@ -1330,6 +1343,7 @@ class Dex2Oat FINAL { std::string error_msg; if (!ElfFile::Strip(oat_file_.get(), &error_msg)) { LOG(ERROR) << "Failed to strip elf file: " << error_msg; + oat_file_->Erase(); return false; } @@ -1338,8 +1352,20 @@ class Dex2Oat FINAL { } else { VLOG(compiler) << "Oat file written successfully without stripping: " << oat_location_; } + if (oat_file_->FlushCloseOrErase() != 0) { + PLOG(ERROR) << "Failed to flush and close oat file: " << oat_location_; + return EXIT_FAILURE; + } + oat_file_.reset(nullptr); } + if (oat_file_.get() != nullptr) { + if (oat_file_->FlushCloseOrErase() != 0) { + PLOG(ERROR) << "Failed to flush and close oat file: " << oat_location_ << "/" + << oat_filename_; + return EXIT_FAILURE; + } + } return true; } @@ -1451,18 +1477,24 @@ class Dex2Oat FINAL { // Destroy ImageWriter before doing FixupElf. image_writer_.reset(); - std::unique_ptr<File> oat_file(OS::OpenFileReadWrite(oat_unstripped_.c_str())); - if (oat_file.get() == nullptr) { - PLOG(ERROR) << "Failed to open ELF file: " << oat_unstripped_; - return false; - } - // Do not fix up the ELF file if we are --compile-pic if (!compiler_options_->GetCompilePic()) { + std::unique_ptr<File> oat_file(OS::OpenFileReadWrite(oat_unstripped_.c_str())); + if (oat_file.get() == nullptr) { + PLOG(ERROR) << "Failed to open ELF file: " << oat_unstripped_; + return false; + } + if (!ElfWriter::Fixup(oat_file.get(), oat_data_begin)) { + oat_file->Erase(); LOG(ERROR) << "Failed to fixup ELF file " << oat_file->GetPath(); return false; } + + if (oat_file->FlushCloseOrErase()) { + PLOG(ERROR) << "Failed to flush and close fixed ELF file " << oat_file->GetPath(); + return false; + } } return true; diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index d6309f7be3..ea71996d1f 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -182,8 +182,8 @@ class OatSymbolizer FINAL : public CodeOutput { bool result = builder_->Write(); - elf_output_->Flush(); - elf_output_->Close(); + // Ignore I/O errors. + UNUSED(elf_output_->FlushClose()); return result; } diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc index 281649e071..b15c7127ee 100644 --- a/patchoat/patchoat.cc +++ b/patchoat/patchoat.cc @@ -904,6 +904,20 @@ static File* CreateOrOpen(const char* name, bool* created) { } } +// Either try to close the file (close=true), or erase it. +static bool FinishFile(File* file, bool close) { + if (close) { + if (file->FlushCloseOrErase() != 0) { + PLOG(ERROR) << "Failed to flush and close file."; + return false; + } + return true; + } else { + file->Erase(); + return false; + } +} + static int patchoat(int argc, char **argv) { InitLogging(argv); MemMap::Init(); @@ -1175,7 +1189,7 @@ static int patchoat(int argc, char **argv) { if (output_image_filename.empty()) { output_image_filename = "output-image-file"; } - output_image.reset(new File(output_image_fd, output_image_filename)); + output_image.reset(new File(output_image_fd, output_image_filename, true)); } else { CHECK(!output_image_filename.empty()); output_image.reset(CreateOrOpen(output_image_filename.c_str(), &new_image_out)); @@ -1189,7 +1203,7 @@ static int patchoat(int argc, char **argv) { if (input_oat_filename.empty()) { input_oat_filename = "input-oat-file"; } - input_oat.reset(new File(input_oat_fd, input_oat_filename)); + input_oat.reset(new File(input_oat_fd, input_oat_filename, false)); if (input_oat == nullptr) { // Unlikely, but ensure exhaustive logging in non-0 exit code case LOG(ERROR) << "Failed to open input oat file by its FD" << input_oat_fd; @@ -1208,7 +1222,7 @@ static int patchoat(int argc, char **argv) { if (output_oat_filename.empty()) { output_oat_filename = "output-oat-file"; } - output_oat.reset(new File(output_oat_fd, output_oat_filename)); + output_oat.reset(new File(output_oat_fd, output_oat_filename, true)); if (output_oat == nullptr) { // Unlikely, but ensure exhaustive logging in non-0 exit code case LOG(ERROR) << "Failed to open output oat file by its FD" << output_oat_fd; @@ -1281,14 +1295,20 @@ static int patchoat(int argc, char **argv) { output_oat.get(), output_image.get(), isa, &timings, output_oat_fd >= 0, // was it opened from FD? new_oat_out); + // The order here doesn't matter. If the first one is successfully saved and the second one + // erased, ImageSpace will still detect a problem and not use the files. + ret = ret && FinishFile(output_image.get(), ret); + ret = ret && FinishFile(output_oat.get(), ret); } else if (have_oat_files) { TimingLogger::ScopedTiming pt("patch oat", &timings); ret = PatchOat::Patch(input_oat.get(), base_delta, output_oat.get(), &timings, output_oat_fd >= 0, // was it opened from FD? new_oat_out); + ret = ret && FinishFile(output_oat.get(), ret); } else if (have_image_files) { TimingLogger::ScopedTiming pt("patch image", &timings); ret = PatchOat::Patch(input_image_location, base_delta, output_image.get(), isa, &timings); + ret = ret && FinishFile(output_image.get(), ret); } else { CHECK(false); ret = true; diff --git a/runtime/Android.mk b/runtime/Android.mk index 25fe45f5cd..58f7940eda 100644 --- a/runtime/Android.mk +++ b/runtime/Android.mk @@ -302,6 +302,7 @@ LIBART_ENUM_OPERATOR_OUT_HEADER_FILES := \ base/allocator.h \ base/mutex.h \ debugger.h \ + base/unix_file/fd_file.h \ dex_file.h \ dex_instruction.h \ gc/allocator/rosalloc.h \ diff --git a/runtime/base/scoped_flock.cc b/runtime/base/scoped_flock.cc index bf091d00d2..0e93eee627 100644 --- a/runtime/base/scoped_flock.cc +++ b/runtime/base/scoped_flock.cc @@ -27,6 +27,9 @@ namespace art { bool ScopedFlock::Init(const char* filename, std::string* error_msg) { while (true) { + if (file_.get() != nullptr) { + UNUSED(file_->FlushCloseOrErase()); // Ignore result. + } file_.reset(OS::OpenFileWithFlags(filename, O_CREAT | O_RDWR)); if (file_.get() == NULL) { *error_msg = StringPrintf("Failed to open file '%s': %s", filename, strerror(errno)); @@ -59,7 +62,7 @@ bool ScopedFlock::Init(const char* filename, std::string* error_msg) { } bool ScopedFlock::Init(File* file, std::string* error_msg) { - file_.reset(new File(dup(file->Fd()))); + file_.reset(new File(dup(file->Fd()), true)); if (file_->Fd() == -1) { file_.reset(); *error_msg = StringPrintf("Failed to duplicate open file '%s': %s", @@ -89,6 +92,9 @@ ScopedFlock::~ScopedFlock() { if (file_.get() != NULL) { int flock_result = TEMP_FAILURE_RETRY(flock(file_->Fd(), LOCK_UN)); CHECK_EQ(0, flock_result); + if (file_->FlushCloseOrErase() != 0) { + PLOG(WARNING) << "Could not close scoped file lock file."; + } } } diff --git a/runtime/base/unix_file/fd_file.cc b/runtime/base/unix_file/fd_file.cc index f29a7ec974..6e5e7a1582 100644 --- a/runtime/base/unix_file/fd_file.cc +++ b/runtime/base/unix_file/fd_file.cc @@ -14,28 +14,68 @@ * limitations under the License. */ -#include "base/logging.h" #include "base/unix_file/fd_file.h" + #include <errno.h> #include <sys/stat.h> #include <sys/types.h> #include <unistd.h> +#include "base/logging.h" + namespace unix_file { -FdFile::FdFile() : fd_(-1), auto_close_(true) { +FdFile::FdFile() : guard_state_(GuardState::kClosed), fd_(-1), auto_close_(true) { } -FdFile::FdFile(int fd) : fd_(fd), auto_close_(true) { +FdFile::FdFile(int fd, bool check_usage) + : guard_state_(check_usage ? GuardState::kBase : GuardState::kNoCheck), + fd_(fd), auto_close_(true) { } -FdFile::FdFile(int fd, const std::string& path) : fd_(fd), file_path_(path), auto_close_(true) { +FdFile::FdFile(int fd, const std::string& path, bool check_usage) + : guard_state_(check_usage ? GuardState::kBase : GuardState::kNoCheck), + fd_(fd), file_path_(path), auto_close_(true) { CHECK_NE(0U, path.size()); } FdFile::~FdFile() { + if (kCheckSafeUsage && (guard_state_ < GuardState::kNoCheck)) { + if (guard_state_ < GuardState::kFlushed) { + LOG(::art::ERROR) << "File " << file_path_ << " wasn't explicitly flushed before destruction."; + } + if (guard_state_ < GuardState::kClosed) { + LOG(::art::ERROR) << "File " << file_path_ << " wasn't explicitly closed before destruction."; + } + CHECK_GE(guard_state_, GuardState::kClosed); + } if (auto_close_ && fd_ != -1) { - Close(); + if (Close() != 0) { + PLOG(::art::WARNING) << "Failed to close file " << file_path_; + } + } +} + +void FdFile::moveTo(GuardState target, GuardState warn_threshold, const char* warning) { + if (kCheckSafeUsage) { + if (guard_state_ < GuardState::kNoCheck) { + if (warn_threshold < GuardState::kNoCheck && guard_state_ >= warn_threshold) { + LOG(::art::ERROR) << warning; + } + guard_state_ = target; + } + } +} + +void FdFile::moveUp(GuardState target, const char* warning) { + if (kCheckSafeUsage) { + if (guard_state_ < GuardState::kNoCheck) { + if (guard_state_ < target) { + guard_state_ = target; + } else if (target < guard_state_) { + LOG(::art::ERROR) << warning; + } + } } } @@ -54,11 +94,28 @@ bool FdFile::Open(const std::string& path, int flags, mode_t mode) { return false; } file_path_ = path; + static_assert(O_RDONLY == 0, "Readonly flag has unexpected value."); + if (kCheckSafeUsage && (flags & (O_RDWR | O_CREAT | O_WRONLY)) != 0) { + // Start in the base state (not flushed, not closed). + guard_state_ = GuardState::kBase; + } else { + // We are not concerned with read-only files. In that case, proper flushing and closing is + // not important. + guard_state_ = GuardState::kNoCheck; + } return true; } int FdFile::Close() { int result = TEMP_FAILURE_RETRY(close(fd_)); + + // Test here, so the file is closed and not leaked. + if (kCheckSafeUsage) { + CHECK_GE(guard_state_, GuardState::kFlushed) << "File " << file_path_ + << " has not been flushed before closing."; + moveUp(GuardState::kClosed, nullptr); + } + if (result == -1) { return -errno; } else { @@ -74,6 +131,7 @@ int FdFile::Flush() { #else int rc = TEMP_FAILURE_RETRY(fsync(fd_)); #endif + moveUp(GuardState::kFlushed, "Flushing closed file."); return (rc == -1) ? -errno : rc; } @@ -92,6 +150,7 @@ int FdFile::SetLength(int64_t new_length) { #else int rc = TEMP_FAILURE_RETRY(ftruncate(fd_, new_length)); #endif + moveTo(GuardState::kBase, GuardState::kClosed, "Truncating closed file."); return (rc == -1) ? -errno : rc; } @@ -107,6 +166,7 @@ int64_t FdFile::Write(const char* buf, int64_t byte_count, int64_t offset) { #else int rc = TEMP_FAILURE_RETRY(pwrite(fd_, buf, byte_count, offset)); #endif + moveTo(GuardState::kBase, GuardState::kClosed, "Writing into closed file."); return (rc == -1) ? -errno : rc; } @@ -135,6 +195,7 @@ bool FdFile::ReadFully(void* buffer, size_t byte_count) { bool FdFile::WriteFully(const void* buffer, size_t byte_count) { const char* ptr = static_cast<const char*>(buffer); + moveTo(GuardState::kBase, GuardState::kClosed, "Writing into closed file."); while (byte_count > 0) { ssize_t bytes_written = TEMP_FAILURE_RETRY(write(fd_, ptr, byte_count)); if (bytes_written == -1) { @@ -146,4 +207,38 @@ bool FdFile::WriteFully(const void* buffer, size_t byte_count) { return true; } +void FdFile::Erase() { + TEMP_FAILURE_RETRY(SetLength(0)); + TEMP_FAILURE_RETRY(Flush()); + TEMP_FAILURE_RETRY(Close()); +} + +int FdFile::FlushCloseOrErase() { + int flush_result = TEMP_FAILURE_RETRY(Flush()); + if (flush_result != 0) { + LOG(::art::ERROR) << "CloseOrErase failed while flushing a file."; + Erase(); + return flush_result; + } + int close_result = TEMP_FAILURE_RETRY(Close()); + if (close_result != 0) { + LOG(::art::ERROR) << "CloseOrErase failed while closing a file."; + Erase(); + return close_result; + } + return 0; +} + +int FdFile::FlushClose() { + int flush_result = TEMP_FAILURE_RETRY(Flush()); + if (flush_result != 0) { + LOG(::art::ERROR) << "FlushClose failed while flushing a file."; + } + int close_result = TEMP_FAILURE_RETRY(Close()); + if (close_result != 0) { + LOG(::art::ERROR) << "FlushClose failed while closing a file."; + } + return (flush_result != 0) ? flush_result : close_result; +} + } // namespace unix_file diff --git a/runtime/base/unix_file/fd_file.h b/runtime/base/unix_file/fd_file.h index 01f4ca2819..8db2ee4a81 100644 --- a/runtime/base/unix_file/fd_file.h +++ b/runtime/base/unix_file/fd_file.h @@ -24,6 +24,9 @@ namespace unix_file { +// If true, check whether Flush and Close are called before destruction. +static constexpr bool kCheckSafeUsage = true; + // A RandomAccessFile implementation backed by a file descriptor. // // Not thread safe. @@ -32,8 +35,8 @@ class FdFile : public RandomAccessFile { FdFile(); // Creates an FdFile using the given file descriptor. Takes ownership of the // file descriptor. (Use DisableAutoClose to retain ownership.) - explicit FdFile(int fd); - explicit FdFile(int fd, const std::string& path); + explicit FdFile(int fd, bool checkUsage); + explicit FdFile(int fd, const std::string& path, bool checkUsage); // Destroys an FdFile, closing the file descriptor if Close hasn't already // been called. (If you care about the return value of Close, call it @@ -47,12 +50,21 @@ class FdFile : public RandomAccessFile { bool Open(const std::string& file_path, int flags, mode_t mode); // RandomAccessFile API. - virtual int Close(); - virtual int64_t Read(char* buf, int64_t byte_count, int64_t offset) const; - virtual int SetLength(int64_t new_length); + virtual int Close() WARN_UNUSED; + virtual int64_t Read(char* buf, int64_t byte_count, int64_t offset) const WARN_UNUSED; + virtual int SetLength(int64_t new_length) WARN_UNUSED; virtual int64_t GetLength() const; - virtual int64_t Write(const char* buf, int64_t byte_count, int64_t offset); - virtual int Flush(); + virtual int64_t Write(const char* buf, int64_t byte_count, int64_t offset) WARN_UNUSED; + virtual int Flush() WARN_UNUSED; + + // Short for SetLength(0); Flush(); Close(); + void Erase(); + + // Try to Flush(), then try to Close(); If either fails, call Erase(). + int FlushCloseOrErase() WARN_UNUSED; + + // Try to Flush and Close(). Attempts both, but returns the first error. + int FlushClose() WARN_UNUSED; // Bonus API. int Fd() const; @@ -61,8 +73,35 @@ class FdFile : public RandomAccessFile { return file_path_; } void DisableAutoClose(); - bool ReadFully(void* buffer, size_t byte_count); - bool WriteFully(const void* buffer, size_t byte_count); + bool ReadFully(void* buffer, size_t byte_count) WARN_UNUSED; + bool WriteFully(const void* buffer, size_t byte_count) WARN_UNUSED; + + // This enum is public so that we can define the << operator over it. + enum class GuardState { + kBase, // Base, file has not been flushed or closed. + kFlushed, // File has been flushed, but not closed. + kClosed, // File has been flushed and closed. + kNoCheck // Do not check for the current file instance. + }; + + protected: + // If the guard state indicates checking (!=kNoCheck), go to the target state "target". Print the + // given warning if the current state is or exceeds warn_threshold. + void moveTo(GuardState target, GuardState warn_threshold, const char* warning); + + // If the guard state indicates checking (<kNoCheck), and is below the target state "target", go + // to "target." If the current state is higher (excluding kNoCheck) than the trg state, print the + // warning. + void moveUp(GuardState target, const char* warning); + + // Forcefully sets the state to the given one. This can overwrite kNoCheck. + void resetGuard(GuardState new_state) { + if (kCheckSafeUsage) { + guard_state_ = new_state; + } + } + + GuardState guard_state_; private: int fd_; @@ -72,6 +111,8 @@ class FdFile : public RandomAccessFile { DISALLOW_COPY_AND_ASSIGN(FdFile); }; +std::ostream& operator<<(std::ostream& os, const FdFile::GuardState& kind); + } // namespace unix_file #endif // ART_RUNTIME_BASE_UNIX_FILE_FD_FILE_H_ diff --git a/runtime/base/unix_file/fd_file_test.cc b/runtime/base/unix_file/fd_file_test.cc index 3481f2ff9f..a7e5b96460 100644 --- a/runtime/base/unix_file/fd_file_test.cc +++ b/runtime/base/unix_file/fd_file_test.cc @@ -24,7 +24,7 @@ namespace unix_file { class FdFileTest : public RandomAccessFileTest { protected: virtual RandomAccessFile* MakeTestFile() { - return new FdFile(fileno(tmpfile())); + return new FdFile(fileno(tmpfile()), false); } }; @@ -53,6 +53,7 @@ TEST_F(FdFileTest, OpenClose) { ASSERT_TRUE(file.Open(good_path, O_CREAT | O_WRONLY)); EXPECT_GE(file.Fd(), 0); EXPECT_TRUE(file.IsOpened()); + EXPECT_EQ(0, file.Flush()); EXPECT_EQ(0, file.Close()); EXPECT_EQ(-1, file.Fd()); EXPECT_FALSE(file.IsOpened()); @@ -60,7 +61,7 @@ TEST_F(FdFileTest, OpenClose) { EXPECT_GE(file.Fd(), 0); EXPECT_TRUE(file.IsOpened()); - file.Close(); + ASSERT_EQ(file.Close(), 0); ASSERT_EQ(unlink(good_path.c_str()), 0); } diff --git a/runtime/base/unix_file/random_access_file_test.h b/runtime/base/unix_file/random_access_file_test.h index 0002433628..e7ace4caaa 100644 --- a/runtime/base/unix_file/random_access_file_test.h +++ b/runtime/base/unix_file/random_access_file_test.h @@ -76,6 +76,8 @@ class RandomAccessFileTest : public testing::Test { ASSERT_EQ(content.size(), static_cast<uint64_t>(file->Write(content.data(), content.size(), 0))); TestReadContent(content, file.get()); + + CleanUp(file.get()); } void TestReadContent(const std::string& content, RandomAccessFile* file) { @@ -131,6 +133,8 @@ class RandomAccessFileTest : public testing::Test { ASSERT_EQ(new_length, file->GetLength()); ASSERT_TRUE(ReadString(file.get(), &new_content)); ASSERT_EQ('\0', new_content[new_length - 1]); + + CleanUp(file.get()); } void TestWrite() { @@ -163,6 +167,11 @@ class RandomAccessFileTest : public testing::Test { ASSERT_EQ(file->GetLength(), new_length); ASSERT_TRUE(ReadString(file.get(), &new_content)); ASSERT_EQ(std::string("hello\0hello", new_length), new_content); + + CleanUp(file.get()); + } + + virtual void CleanUp(RandomAccessFile* file ATTRIBUTE_UNUSED) { } protected: diff --git a/runtime/base/unix_file/random_access_file_utils_test.cc b/runtime/base/unix_file/random_access_file_utils_test.cc index 63179220a2..9457d22424 100644 --- a/runtime/base/unix_file/random_access_file_utils_test.cc +++ b/runtime/base/unix_file/random_access_file_utils_test.cc @@ -37,14 +37,14 @@ TEST_F(RandomAccessFileUtilsTest, CopyFile) { } TEST_F(RandomAccessFileUtilsTest, BadSrc) { - FdFile src(-1); + FdFile src(-1, false); StringFile dst; ASSERT_FALSE(CopyFile(src, &dst)); } TEST_F(RandomAccessFileUtilsTest, BadDst) { StringFile src; - FdFile dst(-1); + FdFile dst(-1, false); // We need some source content to trigger a write. // Copying an empty file is a no-op. diff --git a/runtime/common_runtime_test.cc b/runtime/common_runtime_test.cc index 6e3ebc24d8..03b33e962a 100644 --- a/runtime/common_runtime_test.cc +++ b/runtime/common_runtime_test.cc @@ -59,7 +59,7 @@ ScratchFile::ScratchFile() { filename_ += "/TmpFile-XXXXXX"; int fd = mkstemp(&filename_[0]); CHECK_NE(-1, fd); - file_.reset(new File(fd, GetFilename())); + file_.reset(new File(fd, GetFilename(), true)); } ScratchFile::ScratchFile(const ScratchFile& other, const char* suffix) { @@ -67,7 +67,7 @@ ScratchFile::ScratchFile(const ScratchFile& other, const char* suffix) { filename_ += suffix; int fd = open(filename_.c_str(), O_RDWR | O_CREAT, 0666); CHECK_NE(-1, fd); - file_.reset(new File(fd, GetFilename())); + file_.reset(new File(fd, GetFilename(), true)); } ScratchFile::ScratchFile(File* file) { @@ -88,6 +88,11 @@ void ScratchFile::Unlink() { if (!OS::FileExists(filename_.c_str())) { return; } + if (file_.get() != nullptr) { + if (file_->FlushCloseOrErase() != 0) { + PLOG(WARNING) << "Error closing scratch file."; + } + } int unlink_result = unlink(filename_.c_str()); CHECK_EQ(0, unlink_result); } diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc index 134e284999..b304779568 100644 --- a/runtime/dex_file_test.cc +++ b/runtime/dex_file_test.cc @@ -146,6 +146,9 @@ static const DexFile* OpenDexFileBase64(const char* base64, if (!file->WriteFully(dex_bytes.get(), length)) { PLOG(FATAL) << "Failed to write base64 as dex file"; } + if (file->FlushCloseOrErase() != 0) { + PLOG(FATAL) << "Could not flush and close test file."; + } file.reset(); // read dex file diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc index addd94833e..ec1e5f02fe 100644 --- a/runtime/dex_file_verifier_test.cc +++ b/runtime/dex_file_verifier_test.cc @@ -115,6 +115,9 @@ static const DexFile* OpenDexFileBase64(const char* base64, const char* location if (!file->WriteFully(dex_bytes.get(), length)) { PLOG(FATAL) << "Failed to write base64 as dex file"; } + if (file->FlushCloseOrErase() != 0) { + PLOG(FATAL) << "Could not flush and close test file."; + } file.reset(); // read dex file @@ -177,6 +180,9 @@ static const DexFile* FixChecksumAndOpen(uint8_t* bytes, size_t length, const ch if (!file->WriteFully(bytes, length)) { PLOG(FATAL) << "Failed to write base64 as dex file"; } + if (file->FlushCloseOrErase() != 0) { + PLOG(FATAL) << "Could not flush and close test file."; + } file.reset(); // read dex file diff --git a/runtime/hprof/hprof.cc b/runtime/hprof/hprof.cc index 14d7432508..3069581522 100644 --- a/runtime/hprof/hprof.cc +++ b/runtime/hprof/hprof.cc @@ -475,9 +475,14 @@ class Hprof { } } - std::unique_ptr<File> file(new File(out_fd, filename_)); + std::unique_ptr<File> file(new File(out_fd, filename_, true)); okay = file->WriteFully(header_data_ptr_, header_data_size_) && - file->WriteFully(body_data_ptr_, body_data_size_); + file->WriteFully(body_data_ptr_, body_data_size_); + if (okay) { + okay = file->FlushCloseOrErase() == 0; + } else { + file->Erase(); + } if (!okay) { std::string msg(StringPrintf("Couldn't dump heap; writing \"%s\" failed: %s", filename_.c_str(), strerror(errno))); diff --git a/runtime/signal_catcher.cc b/runtime/signal_catcher.cc index d448460dc3..e377542e45 100644 --- a/runtime/signal_catcher.cc +++ b/runtime/signal_catcher.cc @@ -110,11 +110,17 @@ void SignalCatcher::Output(const std::string& s) { PLOG(ERROR) << "Unable to open stack trace file '" << stack_trace_file_ << "'"; return; } - std::unique_ptr<File> file(new File(fd, stack_trace_file_)); - if (!file->WriteFully(s.data(), s.size())) { - PLOG(ERROR) << "Failed to write stack traces to '" << stack_trace_file_ << "'"; + std::unique_ptr<File> file(new File(fd, stack_trace_file_, true)); + bool success = file->WriteFully(s.data(), s.size()); + if (success) { + success = file->FlushCloseOrErase() == 0; } else { + file->Erase(); + } + if (success) { LOG(INFO) << "Wrote stack traces to '" << stack_trace_file_ << "'"; + } else { + PLOG(ERROR) << "Failed to write stack traces to '" << stack_trace_file_ << "'"; } } diff --git a/runtime/trace.cc b/runtime/trace.cc index 29c01e4d47..2cc50b3732 100644 --- a/runtime/trace.cc +++ b/runtime/trace.cc @@ -431,6 +431,15 @@ void Trace::Stop() { instrumentation::Instrumentation::kMethodExited | instrumentation::Instrumentation::kMethodUnwind); } + if (the_trace->trace_file_.get() != nullptr) { + // Do not try to erase, so flush and close explicitly. + if (the_trace->trace_file_->Flush() != 0) { + PLOG(ERROR) << "Could not flush trace file."; + } + if (the_trace->trace_file_->Close() != 0) { + PLOG(ERROR) << "Could not close trace file."; + } + } delete the_trace; } runtime->GetThreadList()->ResumeAll(); diff --git a/runtime/zip_archive_test.cc b/runtime/zip_archive_test.cc index 96abee2dc3..70a4ddaabf 100644 --- a/runtime/zip_archive_test.cc +++ b/runtime/zip_archive_test.cc @@ -41,7 +41,7 @@ TEST_F(ZipArchiveTest, FindAndExtract) { ScratchFile tmp; ASSERT_NE(-1, tmp.GetFd()); - std::unique_ptr<File> file(new File(tmp.GetFd(), tmp.GetFilename())); + std::unique_ptr<File> file(new File(tmp.GetFd(), tmp.GetFilename(), false)); ASSERT_TRUE(file.get() != NULL); bool success = zip_entry->ExtractToFile(*file, &error_msg); ASSERT_TRUE(success) << error_msg; |