[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 13:52:37 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 share your apprehension about sleeping in a loop like this. This wasn't the initial solution I was hoping for :/

Some alternate solutions considered that avoid the loop are:
1. in the case where we encounter `STATUS_DELETE_PENDING`, go ahead and return `no_such_file_or_directory`. The downside here is a stuck process could be holding on to the pending-deletion file handle, which would mean the return of `status` does not at all accurately reflect the state of the file. There is perhaps some precedent for this though, [Wine's implementation of `PathFileExistsW`](https://github.com/wine-mirror/wine/blob/1d3b312f225f79bec74d3d265128ead455467e2a/dlls/kernelbase/path.c#L2636) will return `false` if the queried path is pending deletion (indicated via `GetFileAttributesW(path) != INVALID_FILE_ATTRIBUTES`).
2. in the case where we encounter `STATUS_DELETE_PENDING`, return the new `errc::delete_pending` and require callers to handle this (potentially via looping). This avoids embedding a potentially expensive loop here, but for callers to handle this properly, they will have to do more or less the same thing (and I suspect this Windows-specific error condition just won't get handled most of the time; maybe that's okay).

Do these seem more palatable? There's some more discussion in the issue about alternate solutions: https://github.com/llvm/llvm-project/issues/89137

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


More information about the llvm-commits mailing list