Retry ReplaceFile in case of failure
The Win32 ReplaceFile function performs several IO operations under the
covers. Among these is creating a temporary file to which the original
file will be moved. Aggressive filesystem scanners, such as A/V software
or git fsmontor--daemon, may inspect this temporary file. When such
software wins the race with the actual movement of the file, ReplaceFile
will fail with ERROR_UNABLE_TO_REMOVE_REPLACED. This is unfortunate.
This CL makes base::ReplaceFile a bit more robust against these scanners
by retrying a call to ReplaceFile that fails in this way for up to half
a second. In local experimentation, the second or third attempt tends to
succeed when the first fails.
This is a band-aid over failures of the "ERROR Failed to write
build.ninja." variety.
Bug: 384523565
Change-Id: Ia77e596687fd91daa888bcc6c07604b845093295
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/18020
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: David Turner <digit@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
diff --git a/src/base/files/file_util_win.cc b/src/base/files/file_util_win.cc
index 774d1e3..0bc07bf 100644
--- a/src/base/files/file_util_win.cc
+++ b/src/base/files/file_util_win.cc
@@ -228,13 +228,26 @@
return true;
File::Error move_error = File::OSErrorToFileError(GetLastError());
- // Try the full-blown replace if the move fails, as ReplaceFile will only
- // succeed when |to_path| does exist. When writing to a network share, we may
- // not be able to change the ACLs. Ignore ACL errors then
- // (REPLACEFILE_IGNORE_MERGE_ERRORS).
- if (::ReplaceFile(ToWCharT(&to_path.value()), ToWCharT(&from_path.value()),
- NULL, REPLACEFILE_IGNORE_MERGE_ERRORS, NULL, NULL)) {
- return true;
+ // ReplaceFile will fail with ERROR_UNABLE_TO_REMOVE_REPLACED when it is
+ // racing with another process (e.g., git fsmonitor--deamon). Repeatedly retry
+ // after a short delay for up to half a second in an attempt to win the race.
+ for (int i = 0; i < 11; ++i) {
+ if (i != 0) {
+ ::Sleep(/*dwMilliseconds=*/50);
+ }
+ // Try the full-blown replace if the move fails, as ReplaceFile will only
+ // succeed when |to_path| does exist. When writing to a network share, we
+ // may not be able to change the ACLs. Ignore ACL errors then
+ // (REPLACEFILE_IGNORE_MERGE_ERRORS).
+ if (::ReplaceFile(ToWCharT(&to_path.value()), ToWCharT(&from_path.value()),
+ NULL, REPLACEFILE_IGNORE_MERGE_ERRORS, NULL, NULL)) {
+ return true;
+ }
+ if (::GetLastError() != ERROR_UNABLE_TO_REMOVE_REPLACED) {
+ // Do not retry if replacement failed for any reason other than this one,
+ // as it's possible that one or the other file has been modified.
+ break;
+ }
}
// In the case of FILE_ERROR_NOT_FOUND from ReplaceFile, it is likely that
// |to_path| does not exist. In this case, the more relevant error comes
