diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h --- a/llvm/include/llvm/Support/LockFileManager.h +++ b/llvm/include/llvm/Support/LockFileManager.h @@ -10,6 +10,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallString.h" +#include "llvm/Support/FileSystem.h" #include #include // for std::pair @@ -52,7 +53,7 @@ private: SmallString<128> FileName; SmallString<128> LockFileName; - SmallString<128> UniqueLockFileName; + Optional UniqueLockFile; Optional > Owner; std::error_code ErrorCode; diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -8,8 +8,10 @@ #include "llvm/Support/LockFileManager.h" #include "llvm/ADT/None.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Config/llvm-config.h" #include "llvm/Support/Errc.h" #include "llvm/Support/ErrorOr.h" #include "llvm/Support/FileSystem.h" @@ -125,39 +127,6 @@ return true; } -namespace { - -/// An RAII helper object ensure that the unique lock file is removed. -/// -/// Ensures that if there is an error or a signal before we finish acquiring the -/// lock, the unique file will be removed. And if we successfully take the lock, -/// the signal handler is left in place so that signals while the lock is held -/// will remove the unique lock file. The caller should ensure there is a -/// matching call to sys::DontRemoveFileOnSignal when the lock is released. -class RemoveUniqueLockFileOnSignal { - StringRef Filename; - bool RemoveImmediately; -public: - RemoveUniqueLockFileOnSignal(StringRef Name) - : Filename(Name), RemoveImmediately(true) { - sys::RemoveFileOnSignal(Filename, nullptr); - } - - ~RemoveUniqueLockFileOnSignal() { - if (!RemoveImmediately) { - // Leave the signal handler enabled. It will be removed when the lock is - // released. - return; - } - sys::fs::remove(Filename); - sys::DontRemoveFileOnSignal(Filename); - } - - void lockAcquired() { RemoveImmediately = false; } -}; - -} // end anonymous namespace - LockFileManager::LockFileManager(StringRef FileName) { this->FileName = FileName; @@ -176,16 +145,27 @@ return; // Create a lock file that is unique to this instance. - UniqueLockFileName = LockFileName; - UniqueLockFileName += "-%%%%%%%%"; - int UniqueLockFileID; - if (std::error_code EC = sys::fs::createUniqueFile( - UniqueLockFileName, UniqueLockFileID, UniqueLockFileName)) { - std::string S("failed to create unique file "); - S.append(std::string(UniqueLockFileName.str())); + // UniqueLockFile = LockFileName; + // UniqueLockFile += "-%%%%%%%%"; + int UniqueLockFileID = 0; + Expected Temp = + sys::fs::TempFile::create(LockFileName + "-%%%%%%%%"); + // if (std::error_code EC = sys::fs::createUniqueFile( + // UniqueLockFile, UniqueLockFileID, UniqueLockFile)) { + if (!Temp) { + std::error_code EC = errorToErrorCode(Temp.takeError()); + std::string S("failed to create unique file with prefix "); + S.append(std::string(LockFileName.str())); setError(EC, S); return; } + UniqueLockFile = std::move(*Temp); + + // Make sure we discard the temporary file on exit. + auto RemoveTempFile = llvm::make_scope_exit([&]() { + if (Error E = UniqueLockFile->discard()) + setError(errorToErrorCode(std::move(E))); + }); // Write our process ID to our unique lock file. { @@ -203,41 +183,33 @@ // We failed to write out PID, so report the error, remove the // unique lock file, and fail. std::string S("failed to write to "); - S.append(std::string(UniqueLockFileName.str())); + S.append(UniqueLockFile->TmpName); setError(Out.error(), S); - sys::fs::remove(UniqueLockFileName); return; } } - // Clean up the unique file on signal, which also releases the lock if it is - // held since the .lock symlink will point to a nonexistent file. - RemoveUniqueLockFileOnSignal RemoveUniqueFile(UniqueLockFileName); - while (true) { // Create a link from the lock file name. If this succeeds, we're done. std::error_code EC = - sys::fs::create_link(UniqueLockFileName, LockFileName); + sys::fs::create_link(UniqueLockFile->TmpName, LockFileName); if (!EC) { - RemoveUniqueFile.lockAcquired(); + RemoveTempFile.release(); return; } if (EC != errc::file_exists) { std::string S("failed to create link "); raw_string_ostream OSS(S); - OSS << LockFileName.str() << " to " << UniqueLockFileName.str(); + OSS << LockFileName.str() << " to " << UniqueLockFile->TmpName; setError(EC, OSS.str()); return; } // Someone else managed to create the lock file first. Read the process ID // from the lock file. - if ((Owner = readLockFile(LockFileName))) { - // Wipe out our unique lock file (it's useless now) - sys::fs::remove(UniqueLockFileName); - return; - } + if ((Owner = readLockFile(LockFileName))) + return; // RemoveTempFile will delete out our unique lock file. if (!sys::fs::exists(LockFileName)) { // The previous owner released the lock file before we could read it. @@ -249,7 +221,7 @@ // ownership. if ((EC = sys::fs::remove(LockFileName))) { std::string S("failed to remove lockfile "); - S.append(std::string(UniqueLockFileName.str())); + S.append(UniqueLockFile->TmpName); setError(EC, S); return; } @@ -284,10 +256,7 @@ // Since we own the lock, remove the lock file and our own unique lock file. sys::fs::remove(LockFileName); - sys::fs::remove(UniqueLockFileName); - // The unique file is now gone, so remove it from the signal handler. This - // matches a sys::RemoveFileOnSignal() in LockFileManager(). - sys::DontRemoveFileOnSignal(UniqueLockFileName); + consumeError(UniqueLockFile->discard()); } LockFileManager::WaitForUnlockResult diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp --- a/llvm/lib/Support/Path.cpp +++ b/llvm/lib/Support/Path.cpp @@ -1240,7 +1240,7 @@ // If we can't rename, discard the temporary file. if (RenameEC) - setDeleteDisposition(H, true); + removeFD(FD); #else std::error_code RenameEC = fs::rename(TmpName, Name); if (RenameEC) { @@ -1270,8 +1270,7 @@ Done = true; #ifdef _WIN32 - auto H = reinterpret_cast(_get_osfhandle(FD)); - if (std::error_code EC = setDeleteDisposition(H, false)) + if (std::error_code EC = cancelDeleteOnClose(FD)) return errorCodeToError(EC); #else sys::DontRemoveFileOnSignal(TmpName); diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -438,6 +438,36 @@ return std::error_code(); } +std::error_code removeFD(int FD) { + HANDLE Handle = reinterpret_cast(_get_osfhandle(FD)); + return setDeleteDisposition(Handle, true); +} + +static std::error_code cancelDeleteOnClose(int &FD) { + HANDLE Handle = reinterpret_cast(_get_osfhandle(FD)); + SmallVector Name; + if (std::error_code EC = realPathFromHandle(Handle, Name)) + return EC; + HANDLE NewHandle = + ::CreateFileW(Name.data(), GENERIC_READ | GENERIC_WRITE | DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (NewHandle == INVALID_HANDLE_VALUE) + return mapWindowsError(::GetLastError()); + if (close(FD)) + return mapWindowsError(::GetLastError()); + + if (std::error_code EC = setDeleteDisposition(NewHandle, false)) + return EC; + + FD = ::_open_osfhandle(intptr_t(NewHandle), 0); + if (FD == -1) { + ::CloseHandle(NewHandle); + return mapWindowsError(ERROR_INVALID_HANDLE); + } + return std::error_code(); +} + static std::error_code rename_internal(HANDLE FromHandle, const Twine &To, bool ReplaceIfExists) { SmallVector ToWide;