Skip to content

Commit a20dcd9

Browse files
committed
Support: use Windows 10 RS1+ API if possible to make moves more atomic
This addresses a small window of in-atomicity in the rename support for Windows. If the rename API is stressed sufficiently, it can run afoul of the small window of opportunity where the destination file will not exist. In Swift, this causes the `LockFileManager` to improperly handle file locking for module compilation resulting in spurious failures as the file file is failed to be read after writing due to multiple processes overwriting the file and exposing the window constantly to the scheduled process when building with `-num-threads` > 1 for the Swift driver. The implementation of `llvm::sys::fs::rename` was changed in SVN r315079 (80e31f1) which even explicitly identifies the issue: ~~~ This implementation is still not fully POSIX. Specifically in the case where the destination file is open at the point when rename is called, there will be a short interval of time during which the destination file will not exist. It isn't clear whether it is possible to avoid this using the Windows API. ~~~ Special thanks to Ben Barham for the discussions and help with analyzing logging to track down this issue! This API was introduced with Windows 10 RS1, and although there are LTSC contracts that allow older versions to be supported still, the mainstream support system has declared this release to be obsoleted. We attempt to use the new API, and if we are unable to do so, fallback to the old API. This allows us to maintain compatibility with older releases.
1 parent 4f5d866 commit a20dcd9

File tree

2 files changed

+41
-11
lines changed

2 files changed

+41
-11
lines changed

llvm/include/llvm/Support/Windows/WindowsSupport.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,17 @@
2121
#ifndef LLVM_SUPPORT_WINDOWSSUPPORT_H
2222
#define LLVM_SUPPORT_WINDOWSSUPPORT_H
2323

24+
#if defined(__MINGW32__)
2425
// mingw-w64 tends to define it as 0x0502 in its headers.
2526
#undef _WIN32_WINNT
27+
#undef NTDDI_VERSION
28+
// Expose APIs from Windows 10 RS1. We only require Windows 7 at runtime, so
29+
// make sure that we only use such features optionally, with fallbacks where
30+
// relevant.
31+
#define _WIN32_WINNT 0x0a00
32+
#define NTDDI_VERSION 0x0a000002 // expose APIs from WIN10_RS1
33+
#endif
2634

27-
// Require at least Windows 7 API.
28-
#define _WIN32_WINNT 0x0601
2935
#define WIN32_LEAN_AND_MEAN
3036
#ifndef NOMINMAX
3137
#define NOMINMAX

llvm/lib/Support/Windows/Path.inc

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -466,21 +466,45 @@ static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
466466
(ToWide.size() * sizeof(wchar_t)));
467467
FILE_RENAME_INFO &RenameInfo =
468468
*reinterpret_cast<FILE_RENAME_INFO *>(RenameInfoBuf.data());
469-
RenameInfo.ReplaceIfExists = ReplaceIfExists;
470469
RenameInfo.RootDirectory = 0;
471470
RenameInfo.FileNameLength = ToWide.size() * sizeof(wchar_t);
472471
std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
473472

474-
SetLastError(ERROR_SUCCESS);
475-
if (!SetFileInformationByHandle(FromHandle, FileRenameInfo, &RenameInfo,
476-
RenameInfoBuf.size())) {
477-
unsigned Error = GetLastError();
478-
if (Error == ERROR_SUCCESS)
479-
Error = ERROR_CALL_NOT_IMPLEMENTED; // Wine doesn't always set error code.
480-
return mapWindowsError(Error);
473+
static const struct {
474+
DWORD dwFlags;
475+
FILE_INFO_BY_HANDLE_CLASS Class;
476+
} kRenameStyles[] = {
477+
{ static_cast<DWORD>(FILE_RENAME_FLAG_POSIX_SEMANTICS | (ReplaceIfExists ? FILE_RENAME_FLAG_REPLACE_IF_EXISTS : 0)), FileRenameInfoEx },
478+
{ 0, FileRenameInfo },
479+
};
480+
481+
DWORD dwError;
482+
for (const auto &style : kRenameStyles) {
483+
if (style.dwFlags)
484+
RenameInfo.Flags = style.dwFlags;
485+
else
486+
RenameInfo.ReplaceIfExists = ReplaceIfExists;
487+
488+
SetLastError(ERROR_SUCCESS);
489+
if (SetFileInformationByHandle(FromHandle, style.Class, &RenameInfo,
490+
RenameInfoBuf.size()))
491+
return std::error_code();
492+
493+
dwError = GetLastError();
494+
495+
// Wine doesn't always set error code.
496+
if (dwError == ERROR_SUCCESS)
497+
dwError = ERROR_CALL_NOT_IMPLEMENTED;
498+
499+
// Retry with a different style if we do not support this style.
500+
if (dwError == ERROR_INVALID_PARAMETER ||
501+
dwError == ERROR_CALL_NOT_IMPLEMENTED)
502+
continue;
503+
504+
return mapWindowsError(dwError);
481505
}
482506

483-
return std::error_code();
507+
return mapWindowsError(dwError);
484508
}
485509

486510
static std::error_code rename_handle(HANDLE FromHandle, const Twine &To) {

0 commit comments

Comments
 (0)