Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(345)

Unified Diff: base/files/file_util_win.cc

Issue 2545283002: A robust base::DeleteFile.
Patch Set: clang fix; simplification Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/files/file_util_unittest.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/files/file_util_win.cc
diff --git a/base/files/file_util_win.cc b/base/files/file_util_win.cc
index d70454df3836a7067f71eecf5a0ae8673376f474..cbf2f255e7d7d6f01ba1ded985cda53cbdaf167c 100644
--- a/base/files/file_util_win.cc
+++ b/base/files/file_util_win.cc
@@ -11,13 +11,16 @@
#include <shlobj.h>
#include <stddef.h>
#include <stdint.h>
+#include <string.h>
#include <time.h>
+#include <winioctl.h>
#include <winsock2.h>
#include <algorithm>
#include <limits>
#include <string>
+#include "base/base32.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/logging.h"
@@ -40,32 +43,378 @@ namespace {
const DWORD kFileShareAll =
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
-// Deletes all files and directories in a path.
-// Returns false on the first failure it encounters.
-bool DeleteFileRecursive(const FilePath& path,
- const FilePath::StringType& pattern,
- bool recursive) {
- FileEnumerator traversal(path, false,
- FileEnumerator::FILES | FileEnumerator::DIRECTORIES,
- pattern);
- for (FilePath current = traversal.Next(); !current.empty();
- current = traversal.Next()) {
- // Try to clear the read-only bit if we find it.
- FileEnumerator::FileInfo info = traversal.GetInfo();
- if ((info.find_data().dwFileAttributes & FILE_ATTRIBUTE_READONLY) &&
- (recursive || !info.IsDirectory())) {
- SetFileAttributes(
- current.value().c_str(),
- info.find_data().dwFileAttributes & ~FILE_ATTRIBUTE_READONLY);
- }
+// The workhorse of base::DeleteFile. This class implements a robust file and
+// directory deletion strategy to work with Windows' pecurlarity around file
+// deletion -- on Windows, one does not simply delete a file. The Win32
+// DeleteFile function is deceptively named (though properly documented) as it
+// marks a file for deletion at some unspecified time in the future when all
+// handles are closed. This here workhorse accomplishes two goals: recursive
+// deletes can succeed in the face of other software operating therein (so long
+// as they open files with SHARE_DELETE) and it's much more likely that callers
+// can delete a file or directory and then immediately create another with the
+// same name. This class implementes DeleteFile as follows:
+// - Open the item to be deleted.
+// - Rob the item of its read-only status, if applicable.
+// - In the case of a directory that is not a mount point or a symlink,
+// recursively delete everything contained therein using the strategy
+// described here.
+// - Paint the item with delete-on-close to make sure it is deletable.
+// - Hide the item.
+// - Clear the delete-on-close bit so that it can...
+// - Move the item up one directory (with a random name).
+// - Paint the item with delete-on-close for good.
+//
+// Failure possibilities are:
+// - The item could not be opened.
+// - The item's attributes could not be read.
+// - The item was read-only and this attribute could not be cleared.
+// - The item could not be deleted for various legitimate reasons such as it
+// being open without SHARE_DELETE access, or the current user not having
+// permission to delete it.
+// - The file was moved out to the parent directory, could not be deleted, and
+// could not be moved back to its original location. This will DCHECK in a
+// debug build, as it is highly undesirable and so far outside of expectation.
+//
+// There also exists a partial success case where the item could not be moved
+// prior to deletion but was deleted in situ. This is essentially the Win32
+// DeleteFile behavior, also known as "better than nothing".
+class TreeDeleter {
+ public:
+ // Deletes a single item (a file, directory, mount point, symlink, etc).
+ // Returns false if the item could not be deleted.
+ static bool DeleteItem(const FilePath& path);
+
+ // Recursively deletes |path| and its contents if |path| is a directory and
+ // is not a mount point or a symlink.
+ static bool DeleteRecursive(const FilePath& path);
+
+ private:
+ // Constructs an instance to delete |path| recursively or not.
+ explicit TreeDeleter(const FilePath& path);
+
+ // Main entrypoint to delete |path|. |recurse|, when true, indicates that an
+ // instance created for recursion should recurse into directories.
+ bool Delete(const FilePath& path, bool recurse);
+
+ // Deletes as much of the contents of |path| as possible, returning |true|
+ // only if everything within was deleted.
+ bool DeleteContents(const FilePath& path);
+
+ // Deletes |to_delete|, using |basic_info| to adjust its attributes in the
+ // process.
+ bool DoDelete(const FilePath& path,
+ const win::ScopedHandle& to_delete,
+ FILE_BASIC_INFO* basic_info);
+
+ // Moves |to_delete| out into the parent directory of the operation.
+ // |to_delete| is expected to already have its delete-on-close attribute set.
+ // This function will clear it, move the file, and re-establish
+ // delete-on-close. Returns false if the delete-on-close attribute is not set
+ // on exit.
+ bool MoveToParent(const FilePath& path, const win::ScopedHandle& to_delete);
+
+ // Returns a buffer used to determine if a directory is a mount point or a
+ // symlink.
+ REPARSE_GUID_DATA_BUFFER* GetReparseDataBuffer();
+
+ // Returns |true| if |to_delete| (whose attributes are represented in
+ // |basic_info|) is a mount point (a.k.a. a junction point) or a symlink.
+ bool IsMountPointOrSymlink(const win::ScopedHandle& to_delete,
+ const FILE_BASIC_INFO& basic_info);
+
+ // Renames the item |to_delete| to |target|.
+ bool MoveTo(const win::ScopedHandle& to_delete, const FilePath& target);
+
+ // The parent directory into which items are to be moved immediately prior to
+ // deletion.
+ FilePath parent_directory_;
+
+ // The size, in bytes, of the FILE_RENAME_INFO block.
+ size_t rename_info_buffer_size_ = 0;
+
+ // Holds a FILE_RENAME_INFO when the instance will be used to move files out
+ // of their current directory prior to deletion.
+ std::unique_ptr<uint8_t> rename_info_buffer_;
+
+ // A buffer of size MAXIMUM_REPARSE_DATA_BUFFER_SIZE used to determine if a
+ // directory with a reparse point is either a mount point or a symlink. Only
+ // used for recursive deletes.
+ std::unique_ptr<uint8_t> reparse_data_buffer_;
+
+ DISALLOW_COPY_AND_ASSIGN(TreeDeleter);
+};
+
+// static
+bool TreeDeleter::DeleteItem(const FilePath& path) {
+ return TreeDeleter(path).Delete(path, false /* !recurse */);
+}
+
+// static
+bool TreeDeleter::DeleteRecursive(const FilePath& path) {
+ return TreeDeleter(path).Delete(path, true /* recurse */);
+}
+
+TreeDeleter::TreeDeleter(const FilePath& path) {
+ const FilePath directory = path.DirName();
+ const FilePath parent_directory = directory.DirName();
+
+ // Prepare to move files up to the parent directory while deleting them if the
+ // path to be deleted is not at the root of a volume.
+ if (parent_directory != directory)
+ parent_directory_ = parent_directory;
+}
+
+bool TreeDeleter::Delete(const FilePath& path, bool recurse) {
+ // Open the item to be deleted with permission to hide and delete it with full
+ // sharing. Note that backup semantics are required to open a directory.
+ // Operate on a link or a mount point (implemented via reparse points) itself
+ // rather than on its target.
+ win::ScopedHandle to_delete(::CreateFile(
+ path.value().c_str(),
+ DELETE | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr,
+ OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
+ nullptr));
+ if (!to_delete.IsValid()) {
+ DWORD error = ::GetLastError();
+ if (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND)
+ return true;
+ // Try a straight-up delete as a fallback.
+ if (::DeleteFile(path.value().c_str()))
+ return true;
+ ::SetLastError(error);
+ PLOG(ERROR) << "Failed to open " << path.value() << " for deletion";
+ return false;
+ }
- if (info.IsDirectory()) {
- if (recursive && (!DeleteFileRecursive(current, pattern, true) ||
- !RemoveDirectory(current.value().c_str())))
- return false;
- } else if (!::DeleteFile(current.value().c_str())) {
+ // Remove the item's read-only bit if it is set. This is required for
+ // deletion.
+ bool read_only_set = false;
+ FILE_BASIC_INFO basic_info = {};
+ if (!::GetFileInformationByHandleEx(to_delete.Get(), FileBasicInfo,
+ &basic_info, sizeof(basic_info))) {
+ PLOG(ERROR) << "Failed to read attributes of " << path.value();
+ return false;
+ }
+ if ((basic_info.FileAttributes & FILE_ATTRIBUTE_READONLY) != 0) {
+ basic_info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY;
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+ &basic_info, sizeof(basic_info))) {
+ PLOG(ERROR) << "Failed to strip read-only attribute of " << path.value();
return false;
}
+ read_only_set = true;
+ }
+
+ // Recurse into this item if desired, this item is a directory, and this item
+ // is neither a mount point nor a symlink.
+ if (recurse && (basic_info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 &&
+ !IsMountPointOrSymlink(to_delete, basic_info) && !DeleteContents(path)) {
grt (UTC plus 2) 2016/12/04 08:52:06 i wonder if this is overkill. a directory with a r
jschuh 2016/12/04 14:10:46 For IsMountPointOrSymlink()? Doesn't seem like ove
grt (UTC plus 2) 2016/12/04 21:31:48 I'm not too familiar with the other uses for repar
jschuh 2016/12/06 22:16:34 Ah, I see. I think it would probably be safe enoug
grt (UTC plus 2) 2016/12/07 08:34:39 Done.
+ return false;
+ }
+
+ if (DoDelete(path, to_delete, &basic_info))
+ return true;
+
+ // Restore the item's read-only state on failure.
+ if (read_only_set) {
Scott Hess - ex-Googler 2016/12/06 08:15:56 Maybe split this function into a helper, so that y
grt (UTC plus 2) 2016/12/07 08:34:39 Nice catch! Indeed, the block above belongs in DoD
+ basic_info.FileAttributes |= FILE_ATTRIBUTE_READONLY;
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+ &basic_info, sizeof(basic_info))) {
+ PLOG(ERROR) << "Failed to restore read-only attribute of "
+ << path.value();
+ }
+ }
+ return false;
+}
+
+bool TreeDeleter::DeleteContents(const FilePath& path) {
+ // Recurse through all subdirectories to clear them out. Ignore errors during
+ // this pass -- they will be reported if the directories themselves cannot be
+ // deleted below.
+ {
+ FileEnumerator enumer(path, false, FileEnumerator::DIRECTORIES);
+ for (FilePath item = enumer.Next(); !item.empty(); item = enumer.Next())
+ Delete(item, true /* recurse */);
+ }
+
+ // Now that the contents of all subdirectories have been cleared, repeatedly
+ // pass over the contents of this directory until there is nothing left that
+ // can be deleted. Bail out after five fruitless iterations. This is a
+ // mitigation for the failure mode where the item could not be moved out to
+ // the parent directory before deletion.
+ bool items_failed;
+ int retries = 0;
+ do {
+ items_failed = false;
+ FileEnumerator enumer(
+ path, false, (FileEnumerator::DIRECTORIES | FileEnumerator::FILES));
+ for (FilePath item = enumer.Next(); !item.empty(); item = enumer.Next()) {
+ if (Delete(item, false /* !recurse */))
+ retries = 0; // Reset the retry count on any success.
+ else
+ items_failed = true;
+ }
+ } while (items_failed && ++retries < 5);
grt (UTC plus 2) 2016/12/07 08:34:39 wdyt of a short sleep here to give the system time
+
+ // Report success if the last pass encountered no errors.
+ return !items_failed;
+}
+
+bool TreeDeleter::DoDelete(const FilePath& path,
+ const win::ScopedHandle& to_delete,
+ FILE_BASIC_INFO* basic_info) {
+ // Check to see if it's possible to delete this item. A non-empty directory,
+ // for example, cannot be deleted.
+ FILE_DISPOSITION_INFO disposition = {TRUE}; // DeleteFile
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+ &disposition, sizeof(disposition))) {
+ PLOG(ERROR) << "Unable to delete " << path.value();
+ return false;
+ }
+
+ // The item will now be deleted when all handles are closed. Hide it so that
+ // it appears to vanish right away and so that any other procs (e.g.,
+ // explorer.exe) observing the parent directory have a greater liklihood of
+ // not reacting when a new file suddenly appears and disappears.
+ bool hidden_set = false;
+ if ((basic_info->FileAttributes & FILE_ATTRIBUTE_HIDDEN) == 0) {
+ basic_info->FileAttributes |= FILE_ATTRIBUTE_HIDDEN;
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+ basic_info, sizeof(*basic_info))) {
+ PLOG(WARNING) << "Failed to hide " << path.value();
+ }
+ hidden_set = true;
+ }
+
+ if (MoveToParent(path, to_delete))
+ return true;
+
+ // The item couldn't be deleted. Restore its visibility.
+ if (hidden_set) {
+ basic_info->FileAttributes &= ~FILE_ATTRIBUTE_HIDDEN;
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileBasicInfo,
+ basic_info, sizeof(*basic_info))) {
+ PLOG(WARNING) << "Failed to unhide " << path.value();
+ }
+ }
+ return false;
+}
+
+bool TreeDeleter::MoveToParent(const FilePath& path,
jschuh 2016/12/04 14:10:46 Is there a better place to move to than the parent
grt (UTC plus 2) 2016/12/04 21:35:46 I had similar misgivings about doing this, and I'm
Scott Hess - ex-Googler 2016/12/06 08:15:56 In most cases in our code, there probably is going
jschuh 2016/12/06 22:16:34 I just worry about accumulating a bunch of files i
grt (UTC plus 2) 2016/12/07 08:34:39 The next two things on my plate are: - Survey curr
grt (UTC plus 2) 2016/12/07 15:03:10 I've added this. PTAL.
+ const win::ScopedHandle& to_delete) {
+ // Do the shuffle to move the item out to the parent directory so that a new
+ // item with the same name can be created immediately in this directory, or
+ // so that the containing directory can be deleted immediately.
+
+ // There is nothing to do when deleting at the root of a volume.
+ if (parent_directory_.empty())
+ return true;
+
+ // Revoke deletion so that the item can be moved.
+ FILE_DISPOSITION_INFO disposition = {FALSE}; // DeleteFile
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+ &disposition, sizeof(disposition))) {
+ PLOG(WARNING) << "Unable to revoke deletion of " << path.value()
+ << "; skipping move to parent";
+ return true;
+ }
+
+ // Move the file out to the parent directory. Failure to move is not a fatal
+ // failure, though it will lead to a race when deleting the containing
+ // directory. This is mitigated by retries in directory deletion.
+ bool file_moved = false;
+ int attempts = 0;
+ do {
+ char random[5];
+ base::RandBytes(&random[0], sizeof(random));
+ file_moved = MoveTo(
+ to_delete,
+ parent_directory_.AppendASCII(
+ base::Base32Encode(base::StringPiece(&random[0], sizeof(random)))
+ .append(".tmp")));
Scott Hess - ex-Googler 2016/12/06 08:15:56 IMHO adding .tmp is just putting lipstick on the p
jschuh 2016/12/06 22:16:34 I disagree. I think this is a good indicator if so
grt (UTC plus 2) 2016/12/07 08:34:38 This was my thinking exactly.
+ } while (!file_moved && ++attempts < 5);
+
+ // Mark the file to be deleted on close once again.
+ disposition.DeleteFile = TRUE;
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+ &disposition, sizeof(disposition))) {
+ // This is highly unexpected since the file was to be deleted moments ago.
+ PLOG(ERROR) << "Failed to mark file to be deleted on close";
+
+ if (file_moved) {
+ // Try to move the file back to where it was. If this fails, we're in the
+ // town of cats and leaving the file in the wrong place. Regardless, make
+ // a final attempt to mark the file for deletion.
+ bool file_moved_back = MoveTo(to_delete, path);
+ LONG error = ::GetLastError();
+ if (::SetFileInformationByHandle(to_delete.Get(), FileDispositionInfo,
+ &disposition, sizeof(disposition))) {
+ // Whew! The file will eventually be deleted in situ.
+ return true;
+ }
+ if (!file_moved_back) {
+ ::SetLastError(error);
+ PLOG(DFATAL) << "Failed to move item back to original location";
+ }
+ }
+ return false;
+ }
+
+ return true;
+}
+
+REPARSE_GUID_DATA_BUFFER* TreeDeleter::GetReparseDataBuffer() {
+ if (!reparse_data_buffer_)
+ reparse_data_buffer_.reset(new uint8_t[MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
+ return reinterpret_cast<REPARSE_GUID_DATA_BUFFER*>(
+ reparse_data_buffer_.get());
+}
+
+bool TreeDeleter::IsMountPointOrSymlink(const win::ScopedHandle& to_delete,
+ const FILE_BASIC_INFO& basic_info) {
+ if ((basic_info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0)
+ return false;
+
+ REPARSE_GUID_DATA_BUFFER* buffer = GetReparseDataBuffer();
+ DWORD bytes_used = 0;
+ if (!::DeviceIoControl(to_delete.Get(), FSCTL_GET_REPARSE_POINT, nullptr, 0,
+ buffer, MAXIMUM_REPARSE_DATA_BUFFER_SIZE, &bytes_used,
+ nullptr) ||
+ bytes_used < sizeof(DWORD)) {
+ PLOG(ERROR) << "Failed getting reparse data";
+ return true; // Err on the safe side.
+ }
+
+ return buffer->ReparseTag == IO_REPARSE_TAG_MOUNT_POINT ||
+ buffer->ReparseTag == IO_REPARSE_TAG_SYMLINK;
+}
+
+bool TreeDeleter::MoveTo(const win::ScopedHandle& to_delete,
+ const FilePath& target) {
+ // Compute the proper size of the FILE_RENAME_INFO including the target path.
+ size_t path_bytes = (target.value().length() + 1) * sizeof(wchar_t);
+ size_t struct_size = offsetof(FILE_RENAME_INFO, FileName) + path_bytes;
+
+ // Allocate a new buffer if needed.
+ if (rename_info_buffer_size_ < struct_size) {
+ rename_info_buffer_.reset();
+ rename_info_buffer_.reset(new uint8_t[struct_size]);
+ rename_info_buffer_size_ = struct_size;
+ }
+
+ // Formulate a FILE_RENAME_INFO struct in the buffer.
+ FILE_RENAME_INFO* file_rename_info =
+ reinterpret_cast<FILE_RENAME_INFO*>(rename_info_buffer_.get());
+ file_rename_info->ReplaceIfExists = FALSE;
+ file_rename_info->RootDirectory = nullptr;
+ file_rename_info->FileNameLength = target.value().length();
+ ::memcpy(&file_rename_info->FileName[0], target.value().c_str(), path_bytes);
+
+ if (!::SetFileInformationByHandle(to_delete.Get(), FileRenameInfo,
+ file_rename_info, struct_size)) {
+ PLOG(ERROR) << "Failed to move item to " << target.value();
+ return false;
}
return true;
}
@@ -83,36 +432,18 @@ FilePath MakeAbsoluteFilePath(const FilePath& input) {
bool DeleteFile(const FilePath& path, bool recursive) {
ThreadRestrictions::AssertIOAllowed();
+ // Wildcards are not supported.
+ DCHECK_EQ(path.BaseName().value().find_first_of(L"*?"),
+ FilePath::StringType::npos);
+
if (path.empty())
return true;
if (path.value().length() >= MAX_PATH)
return false;
- // Handle any path with wildcards.
- if (path.BaseName().value().find_first_of(L"*?") !=
- FilePath::StringType::npos) {
- return DeleteFileRecursive(path.DirName(), path.BaseName().value(),
- recursive);
- }
- DWORD attr = GetFileAttributes(path.value().c_str());
- // We're done if we can't find the path.
- if (attr == INVALID_FILE_ATTRIBUTES)
- return true;
- // We may need to clear the read-only bit.
- if ((attr & FILE_ATTRIBUTE_READONLY) &&
- !SetFileAttributes(path.value().c_str(),
- attr & ~FILE_ATTRIBUTE_READONLY)) {
- return false;
- }
- // Directories are handled differently if they're recursive.
- if (!(attr & FILE_ATTRIBUTE_DIRECTORY))
- return !!::DeleteFile(path.value().c_str());
- // Handle a simple, single file delete.
- if (!recursive || DeleteFileRecursive(path, L"*", true))
- return !!RemoveDirectory(path.value().c_str());
-
- return false;
+ return recursive ? TreeDeleter::DeleteRecursive(path)
+ : TreeDeleter::DeleteItem(path);
}
bool DeleteFileAfterReboot(const FilePath& path) {
« no previous file with comments | « base/files/file_util_unittest.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698