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

Jeremy Day via llvm-commits llvm-commits at lists.llvm.org
Wed May 15 09:59:27 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:

I haven't worked on the symbol index generation code directly, so can't speak with conviction to the considerations taken in its design. My understanding is that explicit synchronization was anticipated to be more expensive than potentially duplicating work regenerating portions of the index.

At a high level, a compile job for a particular module will generate the symbol index for that module as well as all input (downstream) modules. There's a good chance that a downstream module is included in multiple upstream modules, so a different compile job may have already generated that symbol index file. There's a quick check (via `fs::status`) to see if that file has already been generated and is not stale, which avoids the duplicate work, but since this work is so inexpensive, it's not a big deal if some raciness messes up the underlying check and the index is regenerated unnecessarily.

My initial solution here was just to ignore the `permission_denied` errors and treat them as an indication that the index needed to generated, but I wanted to avoid leaking this Windows idiosyncrasy into platform agnostic code. Ideally this is accomplished by unifying the interface across platforms (i.e. the range of potential errors), but given the fundamental differences in filesystem implementation, it seems like there isn't a good way to do this without resorting to behavior like sleeping in a loop.

With all that said, I think returning just returning `errc::delete_pending` and handling this case downstream is reasonable. This avoids the suppression of legitimate permission errors, and is in theory not Windows-specific (though of course in practice it is).

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


More information about the llvm-commits mailing list