[Support] fix TempFile infinite loop and permission denied errors
Summary: On Windows, TempFile::create() was prone to failing with permission denied errors when a process created many tempfiles without providing a model large enough to accommodate them. There was also a problem with createUniqueEntity getting into an infinite loop when all names permitted by the model are in use. This change fixes both of these problems and adds a unit test for them. Reviewers: pcc, rnk, zturner Reviewed By: zturner Subscribers: inglorion, hiraditya, llvm-commits Differential Revision: https://reviews.llvm.org/D50126 llvm-svn: 338745
This commit is contained in:
parent
41d7047de5
commit
9b36f51ae7
|
@ -190,48 +190,55 @@ createUniqueEntity(const Twine &Model, int &ResultFD,
|
|||
ResultPath.push_back(0);
|
||||
ResultPath.pop_back();
|
||||
|
||||
retry_random_path:
|
||||
// Replace '%' with random chars.
|
||||
for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
|
||||
if (ModelStorage[i] == '%')
|
||||
ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
|
||||
}
|
||||
|
||||
// Try to open + create the file.
|
||||
switch (Type) {
|
||||
case FS_File: {
|
||||
if (std::error_code EC =
|
||||
sys::fs::openFileForReadWrite(Twine(ResultPath.begin()), ResultFD,
|
||||
sys::fs::CD_CreateNew, Flags, Mode)) {
|
||||
if (EC == errc::file_exists)
|
||||
goto retry_random_path;
|
||||
return EC;
|
||||
// Limit the number of attempts we make, so that we don't infinite loop when
|
||||
// we run out of filenames that fit the model.
|
||||
std::error_code EC;
|
||||
for (int Retries = 128; Retries > 0; --Retries) {
|
||||
// Replace '%' with random chars.
|
||||
for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
|
||||
if (ModelStorage[i] == '%')
|
||||
ResultPath[i] =
|
||||
"0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
|
||||
}
|
||||
|
||||
return std::error_code();
|
||||
}
|
||||
// Try to open + create the file.
|
||||
switch (Type) {
|
||||
case FS_File: {
|
||||
EC = sys::fs::openFileForReadWrite(Twine(ResultPath.begin()), ResultFD,
|
||||
sys::fs::CD_CreateNew, Flags, Mode);
|
||||
if (EC) {
|
||||
// errc::permission_denied happens on Windows when we try to open a file
|
||||
// that has been marked for deletion.
|
||||
if (EC == errc::file_exists || EC == errc::permission_denied)
|
||||
continue;
|
||||
return EC;
|
||||
}
|
||||
|
||||
case FS_Name: {
|
||||
std::error_code EC =
|
||||
sys::fs::access(ResultPath.begin(), sys::fs::AccessMode::Exist);
|
||||
if (EC == errc::no_such_file_or_directory)
|
||||
return std::error_code();
|
||||
if (EC)
|
||||
return EC;
|
||||
goto retry_random_path;
|
||||
}
|
||||
|
||||
case FS_Dir: {
|
||||
if (std::error_code EC =
|
||||
sys::fs::create_directory(ResultPath.begin(), false)) {
|
||||
if (EC == errc::file_exists)
|
||||
goto retry_random_path;
|
||||
return EC;
|
||||
}
|
||||
return std::error_code();
|
||||
|
||||
case FS_Name: {
|
||||
EC = sys::fs::access(ResultPath.begin(), sys::fs::AccessMode::Exist);
|
||||
if (EC == errc::no_such_file_or_directory)
|
||||
return std::error_code();
|
||||
if (EC)
|
||||
return EC;
|
||||
continue;
|
||||
}
|
||||
|
||||
case FS_Dir: {
|
||||
EC = sys::fs::create_directory(ResultPath.begin(), false);
|
||||
if (EC) {
|
||||
if (EC == errc::file_exists)
|
||||
continue;
|
||||
return EC;
|
||||
}
|
||||
return std::error_code();
|
||||
}
|
||||
}
|
||||
llvm_unreachable("Invalid Type");
|
||||
}
|
||||
}
|
||||
llvm_unreachable("Invalid Type");
|
||||
return EC;
|
||||
}
|
||||
|
||||
namespace llvm {
|
||||
|
|
|
@ -681,6 +681,37 @@ TEST_F(FileSystemTest, TempFiles) {
|
|||
#endif
|
||||
}
|
||||
|
||||
TEST_F(FileSystemTest, TempFileCollisions) {
|
||||
SmallString<128> TestDirectory;
|
||||
ASSERT_NO_ERROR(
|
||||
fs::createUniqueDirectory("CreateUniqueFileTest", TestDirectory));
|
||||
FileRemover Cleanup(TestDirectory);
|
||||
SmallString<128> Model = TestDirectory;
|
||||
path::append(Model, "%.tmp");
|
||||
SmallString<128> Path;
|
||||
std::vector<fs::TempFile> TempFiles;
|
||||
|
||||
auto TryCreateTempFile = [&]() {
|
||||
Expected<fs::TempFile> T = fs::TempFile::create(Model);
|
||||
if (T) {
|
||||
TempFiles.push_back(std::move(*T));
|
||||
return true;
|
||||
} else {
|
||||
logAllUnhandledErrors(T.takeError(), errs(),
|
||||
"Failed to create temporary file: ");
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
// We should be able to create exactly 16 temporary files.
|
||||
for (int i = 0; i < 16; ++i)
|
||||
EXPECT_TRUE(TryCreateTempFile());
|
||||
EXPECT_FALSE(TryCreateTempFile());
|
||||
|
||||
for (fs::TempFile &T : TempFiles)
|
||||
cantFail(T.discard());
|
||||
}
|
||||
|
||||
TEST_F(FileSystemTest, CreateDir) {
|
||||
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo"));
|
||||
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory) + "foo"));
|
||||
|
|
Loading…
Reference in New Issue