diff --git a/3rdParty/iresearch/core/utils/file_utils.cpp b/3rdParty/iresearch/core/utils/file_utils.cpp index e61c15b341..f68d52953c 100644 --- a/3rdParty/iresearch/core/utils/file_utils.cpp +++ b/3rdParty/iresearch/core/utils/file_utils.cpp @@ -732,7 +732,7 @@ handle_t open(FILE* file, const file_path_t mode) NOEXCEPT { // --SECTION-- path utils // ----------------------------------------------------------------------------- -bool mkdir(const file_path_t path) NOEXCEPT { +bool mkdir(const file_path_t path, bool createNew) NOEXCEPT { bool result; if (!exists_directory(result, path)) { @@ -740,31 +740,18 @@ bool mkdir(const file_path_t path) NOEXCEPT { } if (result) { - return true; // already exists + return !createNew; // directory already exists. } - if (!exists(result, path) || result) { - return false; // failure checking existence or something else exists with the same name - } + // we do not check existence of anything other than directory, as race conditions will arise on second check. Just rely on OS file manager to deal with all name conflicts. auto parts = path_parts(path); if (!parts.dirname.empty()) { // need a null terminated string for use with ::mkdir()/::CreateDirectoryW() std::basic_string::type> parent(parts.dirname); - - if (!mkdir(parent.c_str())) { -#ifdef _WIN32 - if (::GetLastError() != ERROR_ALREADY_EXISTS) { - // failed to create parent - return false; - } -#else - if (errno != EEXIST) { - // failed to create parent - return false; - } -#endif + if (!mkdir(parent.c_str(), false)) { // intermediate path parts can exist, this is ok anyway + return false; } } @@ -778,16 +765,18 @@ bool mkdir(const file_path_t path) NOEXCEPT { // '\\?\' cannot be used with relative paths if (!abs) { if (0 == ::CreateDirectoryW(path, nullptr)) { - typedef std::remove_pointer::type char_t; - auto locale = irs::locale_utils::locale(irs::string_ref::NIL, "utf8", true); // utf8 internal and external - std::string utf8path; + if (::GetLastError() != ERROR_ALREADY_EXISTS || createNew) { + // failed to create directory or directory exist, but we are asked to perform creation + typedef std::remove_pointer::type char_t; + auto locale = irs::locale_utils::locale(irs::string_ref::NIL, "utf8", true); // utf8 internal and external + std::string utf8path; - irs::locale_utils::append_external(utf8path, path, locale); - IR_FRMT_ERROR("Failed to create path: '%s', error %d", utf8path.c_str(), GetLastError()); - - return false; + irs::locale_utils::append_external(utf8path, path, locale); + IR_FRMT_ERROR("Failed to create path: '%s', error %d", + utf8path.c_str(), GetLastError()); + return false; + } } - return true; } @@ -800,20 +789,25 @@ bool mkdir(const file_path_t path) NOEXCEPT { ); if (0 == ::CreateDirectoryW(dirname.c_str(), nullptr)) { - typedef std::remove_pointer::type char_t; - auto locale = irs::locale_utils::locale(irs::string_ref::NIL, "utf8", true); // utf8 internal and external - std::string utf8path; + if (::GetLastError() != ERROR_ALREADY_EXISTS || createNew) { + // failed to create directory or directory exist, but we are asked to perform creation + typedef std::remove_pointer::type char_t; + auto locale = irs::locale_utils::locale(irs::string_ref::NIL, "utf8", true); // utf8 internal and external + std::string utf8path; - irs::locale_utils::append_external(utf8path, path, locale); - IR_FRMT_ERROR("Failed to create path: '%s', error %d", utf8path.c_str(), GetLastError()); + irs::locale_utils::append_external(utf8path, path, locale); + IR_FRMT_ERROR("Failed to create path: '%s', error %d", utf8path.c_str(), GetLastError()); - return false; + return false; + } } #else if (0 != ::mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO)) { - IR_FRMT_ERROR("Failed to create path: '%s', error %d", path, errno); - - return false; + if (errno != EEXIST || createNew) { + // failed to create directory or directory exist, but we are asked to perform creation + IR_FRMT_ERROR("Failed to create path: '%s', error %d", path, errno); + return false; + } } #endif diff --git a/3rdParty/iresearch/core/utils/file_utils.hpp b/3rdParty/iresearch/core/utils/file_utils.hpp index 70f16be91c..c2e6bd8806 100644 --- a/3rdParty/iresearch/core/utils/file_utils.hpp +++ b/3rdParty/iresearch/core/utils/file_utils.hpp @@ -139,7 +139,7 @@ handle_t open(FILE* file, const file_path_t mode) NOEXCEPT; // --SECTION-- path utils // ----------------------------------------------------------------------------- -bool mkdir(const file_path_t path) NOEXCEPT; // recursive directory creation +bool mkdir(const file_path_t path, bool createNew) NOEXCEPT; // recursive directory creation bool move(const file_path_t src_path, const file_path_t dst_path) NOEXCEPT; diff --git a/3rdParty/iresearch/core/utils/utf8_path.cpp b/3rdParty/iresearch/core/utils/utf8_path.cpp index d3d8b6ef3a..6bad1a20ba 100644 --- a/3rdParty/iresearch/core/utils/utf8_path.cpp +++ b/3rdParty/iresearch/core/utils/utf8_path.cpp @@ -338,8 +338,9 @@ bool utf8_path::mtime(time_t& result) const NOEXCEPT { return irs::file_utils::mtime(result, c_str()); } -bool utf8_path::mkdir() const NOEXCEPT { - return irs::file_utils::mkdir(c_str()); + +bool utf8_path::mkdir(bool createNew /* = true*/) const NOEXCEPT { + return irs::file_utils::mkdir(c_str(), createNew); } bool utf8_path::remove() const NOEXCEPT { diff --git a/3rdParty/iresearch/core/utils/utf8_path.hpp b/3rdParty/iresearch/core/utils/utf8_path.hpp index 715374d874..a553eeac62 100644 --- a/3rdParty/iresearch/core/utils/utf8_path.hpp +++ b/3rdParty/iresearch/core/utils/utf8_path.hpp @@ -67,7 +67,9 @@ class IRESEARCH_API utf8_path { bool exists_directory(bool& result) const NOEXCEPT; bool exists_file(bool& result) const NOEXCEPT; bool file_size(uint64_t& result) const NOEXCEPT; - bool mkdir() const NOEXCEPT; + /// @brief creates path + /// @param createNew requires at least last segment of path to be actually created or error will be detected + bool mkdir(bool createNew = true) const NOEXCEPT; bool mtime(time_t& result) const NOEXCEPT; bool remove() const NOEXCEPT; bool rename(const utf8_path& destination) const NOEXCEPT; @@ -96,4 +98,4 @@ class IRESEARCH_API utf8_path { NS_END -#endif \ No newline at end of file +#endif diff --git a/3rdParty/iresearch/tests/store/directory_test_case.cpp b/3rdParty/iresearch/tests/store/directory_test_case.cpp index 5fa71bb4dc..757cf4c304 100644 --- a/3rdParty/iresearch/tests/store/directory_test_case.cpp +++ b/3rdParty/iresearch/tests/store/directory_test_case.cpp @@ -1106,7 +1106,7 @@ class fs_directory_test : public test_base { path_ = test_case_dir(); path_ /= name_; - ASSERT_TRUE(path_.mkdir()); + ASSERT_TRUE(path_.mkdir(false)); dir_ = std::make_shared(path_.utf8()); } diff --git a/3rdParty/iresearch/tests/tests_main.cpp b/3rdParty/iresearch/tests/tests_main.cpp index 1171d95917..d8b7c52399 100644 --- a/3rdParty/iresearch/tests/tests_main.cpp +++ b/3rdParty/iresearch/tests/tests_main.cpp @@ -311,7 +311,7 @@ void test_base::SetUp() { test_case_dir_ /= ti->test_case_name(); (test_dir_ = test_case_dir_) /= ti->name(); - test_dir_.mkdir(); + test_dir_.mkdir(false); } // ----------------------------------------------------------------------------- diff --git a/3rdParty/iresearch/tests/tests_param.cpp b/3rdParty/iresearch/tests/tests_param.cpp index c597cf2564..01133fa7ad 100644 --- a/3rdParty/iresearch/tests/tests_param.cpp +++ b/3rdParty/iresearch/tests/tests_param.cpp @@ -43,7 +43,7 @@ std::pair, std::string> fs_directory(const test_ auto dir = test->test_dir(); dir /= "index"; - dir.mkdir(); + dir.mkdir(false); impl = std::shared_ptr( new irs::fs_directory(dir.utf8()), @@ -63,7 +63,7 @@ std::pair, std::string> mmap_directory(const tes auto dir = test->test_dir(); dir /= "index"; - dir.mkdir(); + dir.mkdir(false); impl = std::shared_ptr( new irs::mmap_directory(dir.utf8()), diff --git a/3rdParty/iresearch/tests/utils/utf8_path_tests.cpp b/3rdParty/iresearch/tests/utils/utf8_path_tests.cpp index e6dab150b2..fa13ec3893 100644 --- a/3rdParty/iresearch/tests/utils/utf8_path_tests.cpp +++ b/3rdParty/iresearch/tests/utils/utf8_path_tests.cpp @@ -22,6 +22,8 @@ //////////////////////////////////////////////////////////////////////////////// #include +#include +#include #include "tests_shared.hpp" #include "utils/file_utils.hpp" @@ -782,6 +784,105 @@ TEST_F(utf8_path_tests, directory) { ASSERT_FALSE(path2.remove()); // path already removed } + + // recursive path creation with concurrency (full path exists) + { + std::string directory1("deleteme1/deleteme2/deleteme3"); + irs::utf8_path path1; + irs::utf8_path path2; + + path1 /= directory1; + path2 /= directory1; + + EXPECT_TRUE(path1.mkdir()); + EXPECT_FALSE(path2.mkdir()); // directory already exists + EXPECT_TRUE(path2.mkdir(false)); // directory exists, but creation is not mandatory + + ASSERT_TRUE(path1.remove()); + ASSERT_FALSE(path2.remove()); // path already removed + } + // recursive path creation with concurrency (only last sement added) + { + std::string directory1("deleteme1/deleteme2/deleteme3"); + std::string directory2("deleteme4"); + irs::utf8_path path1; + irs::utf8_path path2; + + path1 /= directory1; + path2 /= directory1; + path2 /= directory2; + + ASSERT_TRUE(path1.mkdir()); + ASSERT_TRUE(path2.mkdir()); // last segment created + + ASSERT_TRUE(path1.remove()); + ASSERT_FALSE(path2.remove()); // path already removed + } + // race condition test inside path tree building + { + std::string directory1("deleteme1"); + std::string directory2("deleteme2/deleteme3/deleteme_thread"); + irs::utf8_path pathRoot; + pathRoot /= directory1; + + // threads sync for start + std::mutex mutex; + std::condition_variable ready_cv; + + for (size_t j = 0; j < 3; ++j) { + pathRoot.remove(); // make sure full path tree building always needed + + const auto thread_count = 20; + std::vector results(thread_count, false); + std::vector pool; + // We need all threads to be in same position to maximize test validity (not just ready to run!). + // So we count ready threads + size_t readyCount = 0; + bool ready = false; // flag to indicate all is ready (needed for spurious wakeup check) + + for (size_t i = 0; i < thread_count; ++i) { + auto& result = results[i]; + pool.emplace_back(std::thread([ &result, &directory1, &directory2, i, &mutex, &ready_cv, &readyCount, &ready]() { + irs::utf8_path path; + path /= directory1; + std::ostringstream ss; + ss << directory2 << i; + path /= ss.str(); + + std::unique_lock lk(mutex); + ++readyCount; + while (!ready) { + ready_cv.wait(lk); + } + lk.unlock(); + result = path.mkdir(true) ? 1 : 0; + })); + } + + while(true) { + { + std::lock_guard lock(mutex); + if (readyCount >= thread_count) { + // all threads on positions... go, go, go... + ready = true; + ready_cv.notify_all(); + break; + } + } + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + } + for (auto& thread : pool) { + thread.join(); + } + + ASSERT_TRUE(std::all_of( + results.begin(), results.end(), [](bool res) { return res != 0; } + )); + pathRoot.remove(); // cleanup + } + } + + } void validate_move(bool src_abs, bool dst_abs) {