Remove FdFile::DisableAutoClose.
Remove a footgun that also makes it impossible to use fdsan, since we
don't know whether the FdFile actually owns the fd or not.
Bug: http://b/113558485
Test: mma
Change-Id: I6a7767c33925db631852579f75d2def9ff6a44d5
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 5655b3c..9406c62 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1348,12 +1348,12 @@
}
}
} else {
- std::unique_ptr<File> oat_file(new File(oat_fd_, oat_location_, /* check_usage */ true));
- if (oat_file == nullptr) {
+ std::unique_ptr<File> oat_file(
+ new File(DupCloexec(oat_fd_), oat_location_, /* check_usage */ true));
+ if (!oat_file->IsOpened()) {
PLOG(ERROR) << "Failed to create oat file: " << oat_location_;
return false;
}
- oat_file->DisableAutoClose();
if (oat_file->SetLength(0) != 0) {
PLOG(WARNING) << "Truncating oat file " << oat_location_ << " failed.";
oat_file->Erase();
@@ -1385,12 +1385,12 @@
DCHECK_NE(output_vdex_fd_, -1);
std::string vdex_location = ReplaceFileExtension(oat_location_, "vdex");
- std::unique_ptr<File> vdex_file(new File(output_vdex_fd_, vdex_location, /* check_usage */ true));
- if (vdex_file == nullptr) {
+ std::unique_ptr<File> vdex_file(new File(
+ DupCloexec(output_vdex_fd_), vdex_location, /* check_usage */ true));
+ if (!vdex_file->IsOpened()) {
PLOG(ERROR) << "Failed to create vdex file: " << vdex_location;
return false;
}
- vdex_file->DisableAutoClose();
if (input_vdex_file_ != nullptr && output_vdex_fd_ == input_vdex_fd_) {
update_input_vdex_ = true;
} else {
@@ -1472,10 +1472,7 @@
PLOG(ERROR) << "Failed to create swap file: " << swap_file_name_;
return false;
}
- swap_fd_ = swap_file->Fd();
- swap_file->MarkUnchecked(); // We don't we to track this, it will be unlinked immediately.
- swap_file->DisableAutoClose(); // We'll handle it ourselves, the File object will be
- // released immediately.
+ swap_fd_ = swap_file->Release();
unlink(swap_file_name_.c_str());
}
diff --git a/libartbase/base/unix_file/fd_file.cc b/libartbase/base/unix_file/fd_file.cc
index c5313e9..2166cc7 100644
--- a/libartbase/base/unix_file/fd_file.cc
+++ b/libartbase/base/unix_file/fd_file.cc
@@ -37,25 +37,26 @@
namespace unix_file {
FdFile::FdFile()
- : guard_state_(GuardState::kClosed), fd_(-1), auto_close_(true), read_only_mode_(false) {
-}
+ : guard_state_(GuardState::kClosed), fd_(-1), read_only_mode_(false) {}
FdFile::FdFile(int fd, bool check_usage)
: guard_state_(check_usage ? GuardState::kBase : GuardState::kNoCheck),
- fd_(fd), auto_close_(true), read_only_mode_(false) {
-}
+ fd_(fd),
+ read_only_mode_(false) {}
FdFile::FdFile(int fd, const std::string& path, bool check_usage)
- : FdFile(fd, path, check_usage, false) {
-}
+ : FdFile(fd, path, check_usage, false) {}
-FdFile::FdFile(int fd, const std::string& path, bool check_usage, bool read_only_mode)
+FdFile::FdFile(int fd, const std::string& path, bool check_usage,
+ bool read_only_mode)
: guard_state_(check_usage ? GuardState::kBase : GuardState::kNoCheck),
- fd_(fd), file_path_(path), auto_close_(true), read_only_mode_(read_only_mode) {
-}
+ fd_(fd),
+ file_path_(path),
+ read_only_mode_(read_only_mode) {}
-FdFile::FdFile(const std::string& path, int flags, mode_t mode, bool check_usage)
- : fd_(-1), auto_close_(true) {
+FdFile::FdFile(const std::string& path, int flags, mode_t mode,
+ bool check_usage)
+ : fd_(-1) {
Open(path, flags, mode);
if (!check_usage || !IsOpened()) {
guard_state_ = GuardState::kNoCheck;
@@ -72,7 +73,7 @@
}
DCHECK_GE(guard_state_, GuardState::kClosed);
}
- if (auto_close_ && fd_ != -1) {
+ if (fd_ != -1) {
if (Close() != 0) {
PLOG(WARNING) << "Failed to close file with fd=" << fd_ << " path=" << file_path_;
}
@@ -91,7 +92,6 @@
guard_state_ = other.guard_state_;
fd_ = other.fd_;
file_path_ = std::move(other.file_path_);
- auto_close_ = other.auto_close_;
read_only_mode_ = other.read_only_mode_;
other.Release(); // Release other.
@@ -125,10 +125,6 @@
}
}
-void FdFile::DisableAutoClose() {
- auto_close_ = false;
-}
-
bool FdFile::Open(const std::string& path, int flags) {
return Open(path, flags, 0640);
}
diff --git a/libartbase/base/unix_file/fd_file.h b/libartbase/base/unix_file/fd_file.h
index 19be3ef..c365ff7 100644
--- a/libartbase/base/unix_file/fd_file.h
+++ b/libartbase/base/unix_file/fd_file.h
@@ -35,8 +35,8 @@
class FdFile : public RandomAccessFile {
public:
FdFile();
- // Creates an FdFile using the given file descriptor. Takes ownership of the
- // file descriptor. (Use DisableAutoClose to retain ownership.)
+ // Creates an FdFile using the given file descriptor.
+ // Takes ownership of the file descriptor.
FdFile(int fd, bool checkUsage);
FdFile(int fd, const std::string& path, bool checkUsage);
FdFile(int fd, const std::string& path, bool checkUsage, bool read_only_mode);
@@ -50,7 +50,6 @@
: guard_state_(other.guard_state_),
fd_(other.fd_),
file_path_(std::move(other.file_path_)),
- auto_close_(other.auto_close_),
read_only_mode_(other.read_only_mode_) {
other.Release(); // Release the src.
}
@@ -64,7 +63,6 @@
int tmp_fd = fd_;
fd_ = -1;
guard_state_ = GuardState::kNoCheck;
- auto_close_ = false;
return tmp_fd;
}
@@ -78,7 +76,6 @@
} else {
guard_state_ = GuardState::kNoCheck;
}
- // Keep the auto_close_ state.
}
// Destroys an FdFile, closing the file descriptor if Close hasn't already
@@ -121,7 +118,6 @@
const std::string& GetPath() const {
return file_path_;
}
- void DisableAutoClose();
bool ReadFully(void* buffer, size_t byte_count) WARN_UNUSED;
bool PreadFully(void* buffer, size_t byte_count, size_t offset) WARN_UNUSED;
bool WriteFully(const void* buffer, size_t byte_count) WARN_UNUSED;
@@ -182,7 +178,6 @@
int fd_;
std::string file_path_;
- bool auto_close_;
bool read_only_mode_;
DISALLOW_COPY_AND_ASSIGN(FdFile);
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 4780aea..9b0ea3f 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -1306,8 +1306,8 @@
std::string* error_msg) {
ScopedTrace trace(__PRETTY_FUNCTION__);
if (oat_fd != -1) {
- std::unique_ptr<File> file = std::make_unique<File>(oat_fd, false);
- file->DisableAutoClose();
+ int duped_fd = DupCloexec(oat_fd);
+ std::unique_ptr<File> file = std::make_unique<File>(duped_fd, false);
if (file == nullptr) {
*error_msg = StringPrintf("Failed to open oat filename for reading: %s",
strerror(errno));
diff --git a/runtime/trace.cc b/runtime/trace.cc
index 1986eec..e3641b6 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -383,9 +383,6 @@
}
};
std::unique_ptr<File, decltype(deleter)> trace_file(trace_file_in.release(), deleter);
- if (trace_file != nullptr) {
- trace_file->DisableAutoClose();
- }
Thread* self = Thread::Current();
{
