[llvm] [Support] Handle delete_pending case for Windows fs::status (PR #90655)

Jeremy Day via llvm-commits llvm-commits at lists.llvm.org
Tue May 14 09:59:21 PDT 2024


================
@@ -785,9 +785,20 @@ std::error_code status(const Twine &path, file_status &result, bool Follow) {
 
   DWORD Flags = FILE_FLAG_BACKUP_SEMANTICS;
   if (!Follow) {
-    DWORD attr = ::GetFileAttributesW(path_utf16.begin());
-    if (attr == INVALID_FILE_ATTRIBUTES)
-      return getStatus(INVALID_HANDLE_VALUE, result);
+    DWORD attr;
+
+    // If getting file attributes fails due to a pending deletion, try
+    // again in a loop to avoid returning a misleading permission denied
+    // error.
+    for (int Retry = 200; Retry >= 0; --Retry) {
+      attr = ::GetFileAttributesW(path_utf16.begin());
+      if (attr != INVALID_FILE_ATTRIBUTES)
+        break;
+      std::error_code code = getStatus(INVALID_HANDLE_VALUE, result);
+      if (code != llvm::errc::delete_pending || !Retry)
+        return code;
+      ::Sleep(15);
----------------
z2oh wrote:

To report the results of my test: in a lengthy compile with many (100k+) calls to the `status()` function, the loop only required extra iteration 3 times, with total number of iterations needed being 2, 3, and 3.

As for renaming the file before deleting it: the particular instance of this race I'm addressing is one where the file being `stat`'d is the destination file of another rename.

(for reference)
The call to `fs::rename`: https://github.com/apple/llvm-project/blob/32aa837fd7b392532b1180c63dace8ce10d0a5e6/clang/lib/Index/IndexUnitWriter.cpp#L401C3-L401C87

The call to `fs::status`: https://github.com/apple/llvm-project/blob/32aa837fd7b392532b1180c63dace8ce10d0a5e6/clang/lib/Index/IndexUnitWriter.cpp#L247

Several processes all race to rename temporary files to the same destination location, while one or more other processes race to `stat` this same destination location. So the rename-before-deleting change would be in `fs::rename` on the destination file, and it looks like this is already opportunistically applied: https://github.com/llvm/llvm-project/blob/0f17d9a28c40eebd42c83956e2a7b5186c1814d7/llvm/lib/Support/Windows/Path.inc#L513-L523

https://github.com/llvm/llvm-project/pull/90655


More information about the llvm-commits mailing list