From 847a77ffb38dd1ec3ea101dca85afad7efc208fd Mon Sep 17 00:00:00 2001 From: Bob Haarman Date: Thu, 16 Nov 2017 01:16:52 +0000 Subject: [PATCH] LTO: clarify why we need to gracefully handle sys::fs::rename failures Reviewers: pcc, rafael Reviewed By: pcc Subscribers: mehdi_amini, llvm-commits, hiraditya Differential Revision: https://reviews.llvm.org/D40094 llvm-svn: 318362 --- llvm/lib/LTO/Caching.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/llvm/lib/LTO/Caching.cpp b/llvm/lib/LTO/Caching.cpp index e6338b2..dd47eb5 100644 --- a/llvm/lib/LTO/Caching.cpp +++ b/llvm/lib/LTO/Caching.cpp @@ -73,13 +73,15 @@ Expected lto::localCache(StringRef CacheDirectoryPath, TempFile.TmpName + ": " + MBOrErr.getError().message() + "\n"); - // This is atomic on POSIX systems. - // On Windows, it can fail with permission denied if the destination - // file already exists. Since the existing file should be semantically - // equivalent to the one we are trying to write, we give AddBuffer - // a copy of the bytes we wrote in that case. We do this instead of - // just using the existing file, because the pruner might delete the - // file before we get a chance to use it. + // On POSIX systems, this will atomically replace the destination if + // it already exists. We try to emulate this on Windows, but this may + // fail with a permission denied error (for example, if the destination + // is currently opened by another process that does not give us the + // sharing permissions we need). Since the existing file should be + // semantically equivalent to the one we are trying to write, we give + // AddBuffer a copy of the bytes we wrote in that case. We do this + // instead of just using the existing file, because the pruner might + // delete the file before we get a chance to use it. Error E = TempFile.keep(EntryPath); E = handleErrors(std::move(E), [&](const ECError &E) -> Error { std::error_code EC = E.convertToErrorCode(); -- 2.7.4