diff options
| -rw-r--r-- | runtime/base/unix_file/fd_file.cc | 53 | ||||
| -rw-r--r-- | runtime/base/unix_file/fd_file.h | 9 | ||||
| -rw-r--r-- | runtime/base/unix_file/fd_file_test.cc | 20 |
3 files changed, 73 insertions, 9 deletions
diff --git a/runtime/base/unix_file/fd_file.cc b/runtime/base/unix_file/fd_file.cc index 4498198b34..ff2dd1b399 100644 --- a/runtime/base/unix_file/fd_file.cc +++ b/runtime/base/unix_file/fd_file.cc @@ -339,22 +339,59 @@ bool FdFile::Copy(FdFile* input_file, int64_t offset, int64_t size) { return true; } -void FdFile::Erase() { +bool FdFile::Unlink() { + if (file_path_.empty()) { + return false; + } + + // Try to figure out whether this file is still referring to the one on disk. + bool is_current = false; + { + struct stat this_stat, current_stat; + int cur_fd = TEMP_FAILURE_RETRY(open(file_path_.c_str(), O_RDONLY)); + if (cur_fd > 0) { + // File still exists. + if (fstat(fd_, &this_stat) == 0 && fstat(cur_fd, ¤t_stat) == 0) { + is_current = (this_stat.st_dev == current_stat.st_dev) && + (this_stat.st_ino == current_stat.st_ino); + } + close(cur_fd); + } + } + + if (is_current) { + unlink(file_path_.c_str()); + } + + return is_current; +} + +bool FdFile::Erase(bool unlink) { DCHECK(!read_only_mode_); - TEMP_FAILURE_RETRY(SetLength(0)); - TEMP_FAILURE_RETRY(Flush()); - TEMP_FAILURE_RETRY(Close()); + + bool ret_result = true; + if (unlink) { + ret_result = Unlink(); + } + + int result; + result = SetLength(0); + result = Flush(); + result = Close(); + // Ignore the errors. + + return ret_result; } int FdFile::FlushCloseOrErase() { DCHECK(!read_only_mode_); - int flush_result = TEMP_FAILURE_RETRY(Flush()); + int flush_result = Flush(); if (flush_result != 0) { LOG(ERROR) << "CloseOrErase failed while flushing a file."; Erase(); return flush_result; } - int close_result = TEMP_FAILURE_RETRY(Close()); + int close_result = Close(); if (close_result != 0) { LOG(ERROR) << "CloseOrErase failed while closing a file."; Erase(); @@ -365,11 +402,11 @@ int FdFile::FlushCloseOrErase() { int FdFile::FlushClose() { DCHECK(!read_only_mode_); - int flush_result = TEMP_FAILURE_RETRY(Flush()); + int flush_result = Flush(); if (flush_result != 0) { LOG(ERROR) << "FlushClose failed while flushing a file."; } - int close_result = TEMP_FAILURE_RETRY(Close()); + int close_result = Close(); if (close_result != 0) { LOG(ERROR) << "FlushClose failed while closing a file."; } diff --git a/runtime/base/unix_file/fd_file.h b/runtime/base/unix_file/fd_file.h index d896ee9ecb..eb85c4f097 100644 --- a/runtime/base/unix_file/fd_file.h +++ b/runtime/base/unix_file/fd_file.h @@ -97,7 +97,14 @@ class FdFile : public RandomAccessFile { int Flush() OVERRIDE WARN_UNUSED; // Short for SetLength(0); Flush(); Close(); - void Erase(); + // If the file was opened with a path name and unlink = true, also calls Unlink() on the path. + // Note that it is the the caller's responsibility to avoid races. + bool Erase(bool unlink = false); + + // Call unlink() if the file was opened with a path, and if open() with the name shows that + // the file descriptor of this file is still up-to-date. This is still racy, though, and it + // is up to the caller to ensure correctness in a multi-process setup. + bool Unlink(); // Try to Flush(), then try to Close(); If either fails, call Erase(). int FlushCloseOrErase() WARN_UNUSED; diff --git a/runtime/base/unix_file/fd_file_test.cc b/runtime/base/unix_file/fd_file_test.cc index 99ef6f73ba..7657a38cec 100644 --- a/runtime/base/unix_file/fd_file_test.cc +++ b/runtime/base/unix_file/fd_file_test.cc @@ -186,4 +186,24 @@ TEST_F(FdFileTest, MoveConstructor) { ASSERT_EQ(file2.Close(), 0); } +TEST_F(FdFileTest, EraseWithPathUnlinks) { + // New scratch file, zero-length. + art::ScratchFile tmp; + std::string filename = tmp.GetFilename(); + tmp.Close(); // This is required because of the unlink race between the scratch file and the + // FdFile, which leads to close-guard breakage. + FdFile file(filename, O_RDWR, false); + ASSERT_TRUE(file.IsOpened()); + EXPECT_GE(file.Fd(), 0); + uint8_t buffer[16] = { 0 }; + EXPECT_TRUE(file.WriteFully(&buffer, sizeof(buffer))); + EXPECT_EQ(file.Flush(), 0); + + EXPECT_TRUE(file.Erase(true)); + + EXPECT_FALSE(file.IsOpened()); + + EXPECT_FALSE(art::OS::FileExists(filename.c_str())) << filename; +} + } // namespace unix_file |