ART: Track Flush & Close in FdFile

Implement a check that aborts when a file hasn't been explicitly
flushed and closed when it is destructed.

Add WARN_UNUSED to FdFile methods.

Update dex2oat, patchoat, scoped_flock and some gtests to pass with
this.

(cherry picked from commit 9433ec60b325b708b9fa87e699ab4a6565741494)

Change-Id: I9ab03b1653e69f44cc98946dc89d764c3e045dd4

19 files changed, 299 insertions, 45 deletions

diff --git a/compiler/image_test.cc b/compiler/image_test.cc
index 7e2be3e..dac1ef4 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 64d2de1..ef1bf81 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 2d2a82e..927c5f5 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 d6309f7..ea71996 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 281649e..b15c712 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 25fe45f..58f7940 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 bf091d0..0e93eee 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 f29a7ec..6e5e7a1 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 01f4ca2..8db2ee4 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 3481f2f..a7e5b96 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 0002433..e7ace4c 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 6317922..9457d22 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 6e3ebc2..03b33e9 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 134e284..b304779 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 addd948..ec1e5f0 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 14d7432..3069581 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 d448460..e377542 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 29c01e4..2cc50b3 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 96abee2..70a4dda 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;