diff options
author | Reid Kleckner <rnk@google.com> | 2016-09-02 01:10:53 +0000 |
---|---|---|
committer | Reid Kleckner <rnk@google.com> | 2016-09-02 01:10:53 +0000 |
commit | 1a4398a1986bd7c2fc34048cd07e6b221ebbd882 (patch) | |
tree | 559de5b30cd25d7d82da0bb90d836ccff9f8c38c | |
parent | d4e80a9615639942f62ac808cdb195105218f813 (diff) | |
download | bcm5719-llvm-1a4398a1986bd7c2fc34048cd07e6b221ebbd882.tar.gz bcm5719-llvm-1a4398a1986bd7c2fc34048cd07e6b221ebbd882.zip |
Fix a real temp file leak in FileOutputBuffer
If we failed to commit the buffer but did not die to a signal, the temp
file would remain on disk on Windows. Having an open file mapping and
file handle prevents the file from being deleted. I am choosing not to
add an assertion of success on the temp file removal, since virus
scanners and other environmental things can often cause removal to fail
in real world tools.
Also fix more temp file leaks in unit tests.
llvm-svn: 280445
-rw-r--r-- | llvm/lib/Support/FileOutputBuffer.cpp | 3 | ||||
-rw-r--r-- | llvm/unittests/Support/FileOutputBufferTest.cpp | 13 | ||||
-rw-r--r-- | llvm/unittests/Support/Path.cpp | 2 | ||||
-rw-r--r-- | llvm/unittests/Support/raw_pwrite_stream_test.cpp | 10 |
4 files changed, 19 insertions, 9 deletions
diff --git a/llvm/lib/Support/FileOutputBuffer.cpp b/llvm/lib/Support/FileOutputBuffer.cpp index 651e679f2cb..2c7bf0435d8 100644 --- a/llvm/lib/Support/FileOutputBuffer.cpp +++ b/llvm/lib/Support/FileOutputBuffer.cpp @@ -32,6 +32,9 @@ FileOutputBuffer::FileOutputBuffer(std::unique_ptr<mapped_file_region> R, : Region(std::move(R)), FinalPath(Path), TempPath(TmpPath) {} FileOutputBuffer::~FileOutputBuffer() { + // Close the mapping before deleting the temp file, so that the removal + // succeeds. + Region.reset(); sys::fs::remove(Twine(TempPath)); } diff --git a/llvm/unittests/Support/FileOutputBufferTest.cpp b/llvm/unittests/Support/FileOutputBufferTest.cpp index 090c476e35c..53a2ae0aadd 100644 --- a/llvm/unittests/Support/FileOutputBufferTest.cpp +++ b/llvm/unittests/Support/FileOutputBufferTest.cpp @@ -20,9 +20,12 @@ using namespace llvm::sys; #define ASSERT_NO_ERROR(x) \ 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"; \ + SmallString<128> MessageStorage; \ + raw_svector_ostream Message(MessageStorage); \ + Message << #x ": did not return errc::success.\n" \ + << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \ + << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \ + GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \ } else { \ } @@ -57,9 +60,9 @@ TEST(FileOutputBuffer, Test) { ASSERT_EQ(File1Size, 8192ULL); ASSERT_NO_ERROR(fs::remove(File1.str())); - // TEST 2: Verify abort case. + // TEST 2: Verify abort case. SmallString<128> File2(TestDirectory); - File2.append("/file2"); + File2.append("/file2"); { ErrorOr<std::unique_ptr<FileOutputBuffer>> Buffer2OrErr = FileOutputBuffer::create(File2, 8192); diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 0c612293a96..bd12b3b8c84 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -488,6 +488,8 @@ TEST_F(FileSystemTest, Unique) { fs::createUniqueDirectory("dir2", Dir2)); ASSERT_NO_ERROR(fs::getUniqueID(Dir2.c_str(), F2)); ASSERT_NE(F1, F2); + ASSERT_NO_ERROR(fs::remove(Dir1)); + ASSERT_NO_ERROR(fs::remove(Dir2)); ASSERT_NO_ERROR(fs::remove(TempPath2)); ASSERT_NO_ERROR(fs::remove(TempPath)); } diff --git a/llvm/unittests/Support/raw_pwrite_stream_test.cpp b/llvm/unittests/Support/raw_pwrite_stream_test.cpp index 88855675253..65625fcddd4 100644 --- a/llvm/unittests/Support/raw_pwrite_stream_test.cpp +++ b/llvm/unittests/Support/raw_pwrite_stream_test.cpp @@ -17,13 +17,15 @@ using namespace llvm; #define ASSERT_NO_ERROR(x) \ 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"; \ + SmallString<128> MessageStorage; \ + raw_svector_ostream Message(MessageStorage); \ + Message << #x ": did not return errc::success.\n" \ + << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \ + << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \ + GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \ } else { \ } - namespace { TEST(raw_pwrite_ostreamTest, TestSVector) { |