From: Joel Rosdahl Date: Mon, 5 Dec 2022 19:50:58 +0000 (+0100) Subject: enhance: Extract lock keep-alive thread to a manager class X-Git-Tag: v4.8~80 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1b71d20b4ff0eebd6d0abc965d22d887203b364;p=thirdparty%2Fccache.git enhance: Extract lock keep-alive thread to a manager class Instead of running one keep-alive thread per lock, a long-lived LockFile now lets a separate LongLivedLockFileManager object handle keep-alive for several locks in a single thread. --- diff --git a/src/storage/local/StatsFile.cpp b/src/storage/local/StatsFile.cpp index 42987becc..af5854a7c 100644 --- a/src/storage/local/StatsFile.cpp +++ b/src/storage/local/StatsFile.cpp @@ -62,7 +62,7 @@ std::optional StatsFile::update( std::function function) const { - util::ShortLivedLockFile lock(m_path); + util::LockFile lock(m_path); if (!lock.acquire()) { LOG("Failed to acquire lock for {}", m_path); return std::nullopt; diff --git a/src/test_lockfile.cpp b/src/test_lockfile.cpp index 2e48fbb00..25dc72808 100644 --- a/src/test_lockfile.cpp +++ b/src/test_lockfile.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -41,16 +42,19 @@ main(int argc, char** argv) const std::string path(argv[1]); const auto seconds = util::parse_signed(argv[2]); - const auto lock_type = std::string(argv[3]) == "long" - ? util::LockFile::Type::long_lived - : util::LockFile::Type::short_lived; + const bool long_lived = std::string(argv[3]) == "long"; const bool blocking = std::string(argv[4]) == "blocking"; if (!seconds) { PRINT_RAW(stderr, "Error: Failed to parse seconds\n"); return 1; } - util::LockFile lock(path, lock_type); + std::unique_ptr lock_manager; + util::LockFile lock(path); + if (long_lived) { + lock_manager = std::make_unique(); + lock.make_long_lived(*lock_manager); + } if (blocking) { PRINT_RAW(stdout, "Acquiring\n"); lock.acquire(); diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 560f79f91..d08916bbc 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -2,6 +2,7 @@ set( sources Bytes.cpp LockFile.cpp + LongLivedLockFileManager.cpp TextTable.cpp TimePoint.cpp Tokenizer.cpp diff --git a/src/util/LockFile.cpp b/src/util/LockFile.cpp index 90c3020b1..26b38e6b9 100644 --- a/src/util/LockFile.cpp +++ b/src/util/LockFile.cpp @@ -42,7 +42,6 @@ const double k_min_sleep_time = 0.010; const double k_max_sleep_time = 0.050; #ifndef _WIN32 const util::Duration k_staleness_limit(2); -const util::Duration k_keep_alive_interval(k_staleness_limit / 4); #endif namespace { @@ -72,10 +71,9 @@ private: namespace util { -LockFile::LockFile(const std::string& path, [[maybe_unused]] Type type) +LockFile::LockFile(const std::string& path) : m_lock_file(path + ".lock"), #ifndef _WIN32 - m_type(type), m_alive_file(path + ".alive"), m_acquired(false) #else @@ -84,6 +82,15 @@ LockFile::LockFile(const std::string& path, [[maybe_unused]] Type type) { } +void +LockFile::make_long_lived( + [[maybe_unused]] LongLivedLockFileManager& lock_manager) +{ +#ifndef _WIN32 + m_lock_manager = &lock_manager; +#endif +} + bool LockFile::acquire() { @@ -107,14 +114,8 @@ LockFile::release() LOG("Releasing {}", m_lock_file); #ifndef _WIN32 - if (m_type == Type::long_lived && m_keep_alive_thread.joinable()) { - { - std::unique_lock lock(m_stop_keep_alive_mutex); - m_stop_keep_alive = true; - } - m_stop_keep_alive_condition.notify_one(); - m_keep_alive_thread.join(); - + if (m_lock_manager) { + m_lock_manager->deregister_alive_file(m_alive_file); Util::unlink_tmp(m_alive_file); } Util::unlink_tmp(m_lock_file); @@ -153,28 +154,12 @@ LockFile::acquire(const bool blocking) if (acquired()) { LOG("Acquired {}", m_lock_file); #ifndef _WIN32 - if (m_type == Type::long_lived) { + if (m_lock_manager) { const auto result = util::write_file(m_alive_file, ""); if (!result) { LOG("Failed to write {}: {}", m_alive_file, result.error()); } - - LOG_RAW("Starting keep-alive thread"); - m_keep_alive_thread = std::thread([&] { - while (true) { - std::unique_lock lock(m_stop_keep_alive_mutex); - m_stop_keep_alive_condition.wait_for( - lock, - std::chrono::seconds(k_keep_alive_interval.sec()) - + std::chrono::nanoseconds(k_keep_alive_interval.nsec()), - [this] { return m_stop_keep_alive; }); - if (m_stop_keep_alive) { - return; - } - util::set_timestamps(m_alive_file); - } - }); - LOG_RAW("Started keep-alive thread"); + m_lock_manager->register_alive_file(m_alive_file); } #endif } else { diff --git a/src/util/LockFile.hpp b/src/util/LockFile.hpp index b6cbae7ba..c1f336323 100644 --- a/src/util/LockFile.hpp +++ b/src/util/LockFile.hpp @@ -19,26 +19,29 @@ #pragma once #include +#include #include -#include -#include #include #include -#include namespace util { +// Unless make_long_lived is called, the lock is expected to be released shortly +// after being acquired - if it is held for more than two seconds it risks being +// considered stale by another client. class LockFile : NonCopyable { public: - enum class Type { long_lived, short_lived }; - - LockFile(const std::string& path, Type type); + LockFile(const std::string& path); // Release the lock if previously acquired. ~LockFile(); + // Make this lock long-lived. Depending on implementation, it will be kept + // alive by a helper thread. + void make_long_lived(LongLivedLockFileManager& lock_manager); + // Acquire lock, blocking. Returns true if acquired, otherwise false. bool acquire(); @@ -54,13 +57,9 @@ public: private: std::string m_lock_file; #ifndef _WIN32 - Type m_type; + LongLivedLockFileManager* m_lock_manager = nullptr; std::string m_alive_file; bool m_acquired; - std::thread m_keep_alive_thread; - std::mutex m_stop_keep_alive_mutex; - bool m_stop_keep_alive = false; - std::condition_variable m_stop_keep_alive_condition; #else void* m_handle; #endif @@ -74,39 +73,9 @@ private: #endif }; -// A short-lived lock. -// -// The lock is expected to be released shortly after being acquired - if it is -// held for more than two seconds it risks being considered stale by another -// client. -class ShortLivedLockFile : public LockFile -{ -public: - ShortLivedLockFile(const std::string& path); -}; - -// A long-lived lock. -// -// The lock will (depending on implementation) be kept alive by a helper thread. -class LongLivedLockFile : public LockFile -{ -public: - LongLivedLockFile(const std::string& path); -}; - inline LockFile::~LockFile() { release(); } -inline ShortLivedLockFile::ShortLivedLockFile(const std::string& path) - : LockFile(path, Type::short_lived) -{ -} - -inline LongLivedLockFile::LongLivedLockFile(const std::string& path) - : LockFile(path, Type::long_lived) -{ -} - } // namespace util diff --git a/src/util/LongLivedLockFileManager.cpp b/src/util/LongLivedLockFileManager.cpp new file mode 100644 index 000000000..e0bdf1e94 --- /dev/null +++ b/src/util/LongLivedLockFileManager.cpp @@ -0,0 +1,89 @@ +// Copyright (C) 2022 Joel Rosdahl and other contributors +// +// See doc/AUTHORS.adoc for a complete list of contributors. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along with +// this program; if not, write to the Free Software Foundation, Inc., 51 +// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +#include "LongLivedLockFileManager.hpp" + +#include +#include +#include + +#include + +#ifndef _WIN32 +std::chrono::milliseconds k_keep_alive_interval{500}; +#endif + +namespace util { + +LongLivedLockFileManager::LongLivedLockFileManager() +{ +#ifndef _WIN32 + LOG_RAW("Starting keep-alive thread"); + m_thread = std::thread([&] { + auto awake_time = std::chrono::steady_clock::now(); + while (true) { + std::unique_lock lock(m_mutex); + m_stop_condition.wait_until(lock, awake_time, [this] { return m_stop; }); + if (m_stop) { + return; + } + for (const auto& alive_file : m_alive_files) { + util::set_timestamps(alive_file); + } + awake_time += k_keep_alive_interval; + } + }); + LOG_RAW("Started keep-alive thread"); +#endif +} + +LongLivedLockFileManager::~LongLivedLockFileManager() +{ +#ifndef _WIN32 + LOG_RAW("Stopping keep-alive thread"); + { + std::unique_lock lock(m_mutex); + m_stop = true; + } + m_stop_condition.notify_one(); + m_thread.join(); + LOG_RAW("Stopped keep-alive thread"); +#endif +} + +void +LongLivedLockFileManager::register_alive_file( + [[maybe_unused]] const std::string& path) +{ +#ifndef _WIN32 + std::unique_lock lock(m_mutex); + m_alive_files.insert(path); +#endif +} + +void +LongLivedLockFileManager::deregister_alive_file( + [[maybe_unused]] const std::string& path) +{ +#ifndef _WIN32 + std::unique_lock lock(m_mutex); + m_alive_files.erase(path); +#endif +} + +} // namespace util diff --git a/src/util/LongLivedLockFileManager.hpp b/src/util/LongLivedLockFileManager.hpp new file mode 100644 index 000000000..72fdc5b47 --- /dev/null +++ b/src/util/LongLivedLockFileManager.hpp @@ -0,0 +1,50 @@ +// Copyright (C) 2022 Joel Rosdahl and other contributors +// +// See doc/AUTHORS.adoc for a complete list of contributors. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along with +// this program; if not, write to the Free Software Foundation, Inc., 51 +// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +#pragma once + +#include + +#include +#include +#include +#include +#include + +namespace util { + +class LongLivedLockFileManager : NonCopyable +{ +public: + LongLivedLockFileManager(); + ~LongLivedLockFileManager(); + + void register_alive_file(const std::string& path); + void deregister_alive_file(const std::string& path); + +private: +#ifndef _WIN32 + std::thread m_thread; + std::mutex m_mutex; + std::condition_variable m_stop_condition; + bool m_stop = false; + std::set m_alive_files; +#endif +}; + +} // namespace util diff --git a/unittest/test_util_LockFile.cpp b/unittest/test_util_LockFile.cpp index 7bed76629..3086bb4d9 100644 --- a/unittest/test_util_LockFile.cpp +++ b/unittest/test_util_LockFile.cpp @@ -40,7 +40,7 @@ TEST_CASE("Acquire and release short-lived lock file") { TestContext test_context; - util::ShortLivedLockFile lock("test"); + util::LockFile lock("test"); { CHECK(!lock.acquired()); CHECK(!Stat::lstat("test.lock")); @@ -69,10 +69,10 @@ TEST_CASE("Non-blocking short-lived lock") { TestContext test_context; - util::ShortLivedLockFile lock_file_1("test"); + util::LockFile lock_file_1("test"); CHECK(!lock_file_1.acquired()); - util::ShortLivedLockFile lock_file_2("test"); + util::LockFile lock_file_2("test"); CHECK(!lock_file_2.acquired()); CHECK(lock_file_1.try_acquire()); @@ -95,7 +95,9 @@ TEST_CASE("Acquire and release long-lived lock file") { TestContext test_context; - util::LongLivedLockFile lock("test"); + util::LongLivedLockFileManager lock_manager; + util::LockFile lock("test"); + lock.make_long_lived(lock_manager); { CHECK(!lock.acquired()); CHECK(!Stat::lstat("test.lock")); @@ -126,7 +128,9 @@ TEST_CASE("LockFile creates missing directories") { TestContext test_context; - util::ShortLivedLockFile lock("a/b/c/test"); + util::LongLivedLockFileManager lock_manager; + util::LockFile lock("a/b/c/test"); + lock.make_long_lived(lock_manager); CHECK(lock.acquire()); CHECK(Stat::lstat("a/b/c/test.lock")); } @@ -141,7 +145,9 @@ TEST_CASE("Break stale lock, blocking") util::set_timestamps("test.alive", long_time_ago); CHECK(symlink("foo", "test.lock") == 0); - util::LongLivedLockFile lock("test"); + util::LongLivedLockFileManager lock_manager; + util::LockFile lock("test"); + lock.make_long_lived(lock_manager); CHECK(lock.acquire()); } @@ -154,7 +160,9 @@ TEST_CASE("Break stale lock, non-blocking") util::set_timestamps("test.alive", long_time_ago); CHECK(symlink("foo", "test.lock") == 0); - util::LongLivedLockFile lock("test"); + util::LongLivedLockFileManager lock_manager; + util::LockFile lock("test"); + lock.make_long_lived(lock_manager); CHECK(lock.try_acquire()); CHECK(lock.acquired()); }