Fix rename() sometimes failing if another process uses openFileForRead()
On Windows, fs::rename() could fail is another process was reading the
file at the same time using fs::openFileForRead(). In most cases the user
wouldn't notice as fs::rename() will continue to retry for 2000ms. Typically
this is enough for the read to complete and a retry to succeed, but if the
disk is being it too hard then the response time might be longer than the
retry time and the rename would fail with a permission error.
Add FILE_SHARE_DELETE to the sharing flags for CreateFileW() in
fs::openFileForRead() and try ReplaceFileW() prior to MoveFileExW()
in fs::rename().
Based on an initial patch by Edd Dawson!
Differential Revision: http://reviews.llvm.org/D13647
llvm-svn: 250046
diff --git a/llvm/unittests/Support/ReplaceFileTest.cpp b/llvm/unittests/Support/ReplaceFileTest.cpp
new file mode 100644
index 0000000..8b16daf
--- /dev/null
+++ b/llvm/unittests/Support/ReplaceFileTest.cpp
@@ -0,0 +1,113 @@
+//===- llvm/unittest/Support/ReplaceFileTest.cpp - unit tests -------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::sys;
+
+#define ASSERT_NO_ERROR(x) \
+ do { \
+ if (std::error_code ASSERT_NO_ERROR_ec = x) { \
+ errs() << #x ": did not return errc::success.\n" \
+ << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \
+ << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \
+ } \
+ } while (false)
+
+namespace {
+std::error_code CreateFileWithContent(const SmallString<128> &FilePath,
+ const StringRef &content) {
+ int FD = 0;
+ if (std::error_code ec = fs::openFileForWrite(FilePath, FD, fs::F_None))
+ return ec;
+
+ const bool ShouldClose = true;
+ raw_fd_ostream OS(FD, ShouldClose);
+ OS << content;
+
+ return std::error_code();
+}
+
+class ScopedFD {
+ int FD;
+
+ ScopedFD(const ScopedFD &) = delete;
+ ScopedFD &operator=(const ScopedFD &) = delete;
+
+ public:
+ explicit ScopedFD(int Descriptor) : FD(Descriptor) {}
+ ~ScopedFD() { Process::SafelyCloseFileDescriptor(FD); }
+};
+
+TEST(rename, FileOpenedForReadingCanBeReplaced) {
+ // Create unique temporary directory for this test.
+ SmallString<128> TestDirectory;
+ ASSERT_NO_ERROR(fs::createUniqueDirectory(
+ "FileOpenedForReadingCanBeReplaced-test", TestDirectory));
+
+ // Add a couple of files to the test directory.
+ SmallString<128> SourceFileName(TestDirectory);
+ path::append(SourceFileName, "source");
+
+ SmallString<128> TargetFileName(TestDirectory);
+ path::append(TargetFileName, "target");
+
+ ASSERT_NO_ERROR(CreateFileWithContent(SourceFileName, "!!source!!"));
+ ASSERT_NO_ERROR(CreateFileWithContent(TargetFileName, "!!target!!"));
+
+ {
+ // Open the target file for reading.
+ int ReadFD = 0;
+ ASSERT_NO_ERROR(fs::openFileForRead(TargetFileName, ReadFD));
+ ScopedFD EventuallyCloseIt(ReadFD);
+
+ // Confirm we can replace the file while it is open.
+ EXPECT_TRUE(!fs::rename(SourceFileName, TargetFileName));
+
+ // We should still be able to read the old data through the existing
+ // descriptor.
+ auto Buffer = MemoryBuffer::getOpenFile(ReadFD, TargetFileName, -1);
+ ASSERT_TRUE(static_cast<bool>(Buffer));
+ EXPECT_EQ(Buffer.get()->getBuffer(), "!!target!!");
+
+ // The source file should no longer exist
+ EXPECT_FALSE(fs::exists(SourceFileName));
+ }
+
+ {
+ // If we obtain a new descriptor for the target file, we should find that it
+ // contains the content that was in the source file.
+ int ReadFD = 0;
+ ASSERT_NO_ERROR(fs::openFileForRead(TargetFileName, ReadFD));
+ ScopedFD EventuallyCloseIt(ReadFD);
+ auto Buffer = MemoryBuffer::getOpenFile(ReadFD, TargetFileName, -1);
+ ASSERT_TRUE(static_cast<bool>(Buffer));
+
+ EXPECT_EQ(Buffer.get()->getBuffer(), "!!source!!");
+ }
+
+ // Rename the target file back to the source file name to confirm that rename
+ // still works if the destination does not already exist.
+ EXPECT_TRUE(!fs::rename(TargetFileName, SourceFileName));
+ EXPECT_FALSE(fs::exists(TargetFileName));
+ ASSERT_TRUE(fs::exists(SourceFileName));
+
+ // Clean up.
+ ASSERT_NO_ERROR(fs::remove(SourceFileName));
+ ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
+}
+
+} // anonymous namespace