[PATCH] D17903: Improve reliability of file renaming in Windows

[Douglas Yung via llvm-commits]

dyung created this revision.
dyung added a reviewer: llvm-commits.

Improve the reliability of file renaming in Windows by having the compiler retry the rename operation on 3 error conditions of ReplaceFileW() that it was previously bailing out on.

http://reviews.llvm.org/D17903

Files:
  lib/Support/Windows/Path.inc

Index: lib/Support/Windows/Path.inc
===================================================================
--- lib/Support/Windows/Path.inc
+++ lib/Support/Windows/Path.inc
@@ -255,34 +255,52 @@
 
   std::error_code ec = std::error_code();
 
-  // Retry while we see ERROR_ACCESS_DENIED.
+  // Retry while we see recoverable errors.
   // System scanners (eg. indexer) might open the source file when it is written
   // and closed.
 
+  bool TryReplace = true;
+
   for (int i = 0; i < 2000; i++) {
-    // Try ReplaceFile first, as it is able to associate a new data stream with
-    // the destination even if the destination file is currently open.
-    if (::ReplaceFileW(wide_to.begin(), wide_from.begin(), NULL, 0, NULL, NULL))
-      return std::error_code();
+    if (i > 0)
+      ::Sleep(1);
 
-    // We get ERROR_FILE_NOT_FOUND if the destination file is missing.
-    // MoveFileEx can handle this case.
-    DWORD ReplaceError = ::GetLastError();
-    ec = mapWindowsError(ReplaceError);
-    if (ReplaceError != ERROR_ACCESS_DENIED &&
-        ReplaceError != ERROR_FILE_NOT_FOUND &&
-        ReplaceError != ERROR_SHARING_VIOLATION)
-      break;
+    if (TryReplace) {
+      // Try ReplaceFile first, as it is able to associate a new data stream
+      // with the destination even if the destination file is currently open.
+      if (::ReplaceFileW(wide_to.begin(), wide_from.begin(), NULL, 0, NULL,
+                         NULL))
+        return std::error_code();
+
+      ReplaceError = ::GetLastError();
+      ec = mapWindowsError(ReplaceError);
+
+      // If ReplaceFileW returned ERROR_UNABLE_TO_MOVE_REPLACEMENT or
+      // ERROR_UNABLE_TO_MOVE_REPLACEMENT_2, retry but only use MoveFileExW().
+      if (ReplaceError == ERROR_UNABLE_TO_MOVE_REPLACEMENT ||
+          ReplaceError == ERROR_UNABLE_TO_MOVE_REPLACEMENT_2) {
+        TryReplace = false;
+        continue;
+      }
+      // If ReplaceFileW returned ERROR_UNABLE_TO_REMOVE_REPLACED, retry
+      // using ReplaceFileW().
+      if (ReplaceError == ERROR_UNABLE_TO_REMOVE_REPLACED)
+        continue;
+      // We get ERROR_FILE_NOT_FOUND if the destination file is missing.
+      // MoveFileEx can handle this case.
+      if (ReplaceError != ERROR_ACCESS_DENIED &&
+          ReplaceError != ERROR_FILE_NOT_FOUND &&
+          ReplaceError != ERROR_SHARING_VIOLATION)
+        break;
+    }
 
     if (::MoveFileExW(wide_from.begin(), wide_to.begin(),
                       MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING))
       return std::error_code();
 
     DWORD MoveError = ::GetLastError();
     ec = mapWindowsError(MoveError);
     if (MoveError != ERROR_ACCESS_DENIED) break;
-
-    ::Sleep(1);
   }
 
   return ec;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17903.49863.patch
Type: text/x-patch
Size: 2727 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160305/671597b6/attachment.bin>