llvm::sys::fs::access can spuriously fail on Windows when called with AccessMode::Exist

[z2oh]

We are regularly seeing this issue surface as a compiler crash on Apple's LLVM fork: swiftlang#8224

llvm-project/llvm/lib/Support/Windows/Path.inc

Line 617 in f1bb88b

std::error_code access(const Twine &Path, AccessMode Mode) {

I've been unable to reproduce this in isolation, but there seems to be some kind of race condition when calling GetFileAttributesW such that the call fails on a valid path with ERROR_ACCESS_DENIED. I've managed to work around this locally by avoiding the call to GetFileAttributesW when the requested access mode is AccessMode::Exist, and instead calling the shell function PathFileExistsW:

if (Mode == AccessMode::Exist) {
  if (::PathFileExistsW(PathUtf16.begin())) {
    return std::error_code();
  } else {
    return errc::no_such_file_or_directory;
  }
}

I've added some logging here and verified that this function returns correctly when GetFileAttributesW would have failed with ERROR_ACCESS_DENIED. This function requires pulling in shlwapi.h.

I'm happy to make a PR with this patch, but I wanted to make an issue first to seek alternate solutions, or to see if anyone had insight as to why GetFileAttributesW is failing in this way.

[z2oh]

@tristanlabelle managed to reproduce this:

#include <Windows.h>
#include <thread>
#include <cassert>

int main() {
    static LPCWSTR filePath = L"%TEMP%";
    std::thread createFileThread([]()
    {
        while (true)
        {
            HANDLE h = CreateFileW(filePath, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
            assert(h != NULL);
            Sleep(1000);
            assert(CloseHandle(h));
            Sleep(1000);
            assert(DeleteFileW(filePath));
            Sleep(1000);
        }
    });
    std::thread getFileAttributesThread([]()
    {
        while (true)
        {
            auto attributes = GetFileAttributesW(filePath);
            if (attributes == INVALID_FILE_ATTRIBUTES)
            {
                auto lastError = GetLastError();
                assert(lastError != ERROR_ACCESS_DENIED);
            }
        }
    });

    getFileAttributesThread.join();
}

Which fails after a few seconds on assert(lastError != ERROR_ACCESS_DENIED);. When I tested this, it occurred when the file was deleted, in which the expected error would be ERROR_FILE_NOT_FOUND, thus I don't think that GetFileAttributesW can be reliably used to check for existence of a file.

[aganea]

The reason this happens is because DeleteFileW does a deferred delete if the handle is already open, and any attempt to query the handle afterwards returns ERROR_ACCESS_DENIED. Most likely GetFileAttributesW opens a handle internally.

[efriedma-quic]

See also https://stackoverflow.com/questions/52706859/how-to-tell-if-a-path-is-a-file-or-a-directory-without-elevated-privileges