]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
enhance: Extract lock keep-alive thread to a manager class
authorJoel Rosdahl <joel@rosdahl.net>
Mon, 5 Dec 2022 19:50:58 +0000 (20:50 +0100)
committerJoel Rosdahl <joel@rosdahl.net>
Tue, 13 Dec 2022 19:38:37 +0000 (20:38 +0100)
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.

src/storage/local/StatsFile.cpp
src/test_lockfile.cpp
src/util/CMakeLists.txt
src/util/LockFile.cpp
src/util/LockFile.hpp
src/util/LongLivedLockFileManager.cpp [new file with mode: 0644]
src/util/LongLivedLockFileManager.hpp [new file with mode: 0644]
unittest/test_util_LockFile.cpp

index 42987becc64df3bde0e5b732a73d24dc4b016008..af5854a7c7e897b9216f80bd8926d55cd92a13c8 100644 (file)
@@ -62,7 +62,7 @@ std::optional<core::StatisticsCounters>
 StatsFile::update(
   std::function<void(core::StatisticsCounters& counters)> 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;
index 2e48fbb00d1cb8b6cb9f623d6e60d9bd1196b096..25dc7280807cbaf1ef8cc98143332bae73566ad7 100644 (file)
@@ -20,6 +20,7 @@
 #include <Logging.hpp>
 #include <fmtmacros.hpp>
 #include <util/LockFile.hpp>
+#include <util/LongLivedLockFileManager.hpp>
 #include <util/string.hpp>
 
 #include <memory>
@@ -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<util::LongLivedLockFileManager> lock_manager;
+  util::LockFile lock(path);
+  if (long_lived) {
+    lock_manager = std::make_unique<util::LongLivedLockFileManager>();
+    lock.make_long_lived(*lock_manager);
+  }
   if (blocking) {
     PRINT_RAW(stdout, "Acquiring\n");
     lock.acquire();
index 560f79f91f8de8ca7f5db82f03f6522ae39e6456..d08916bbc12b2f442c94adba836e58478c2fd805 100644 (file)
@@ -2,6 +2,7 @@ set(
   sources
   Bytes.cpp
   LockFile.cpp
+  LongLivedLockFileManager.cpp
   TextTable.cpp
   TimePoint.cpp
   Tokenizer.cpp
index 90c3020b145a8c09d64020e560680e19c672220d..26b38e6b978eb5f8ca367fae72eb89038a3b373b 100644 (file)
@@ -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<std::mutex> 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<std::mutex> 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 {
index b6cbae7baa764e33de2228a17368e717c2372a03..c1f336323141687d1533b6c89b1a08f3e9d911a6 100644 (file)
 #pragma once
 
 #include <NonCopyable.hpp>
+#include <util/LongLivedLockFileManager.hpp>
 #include <util/TimePoint.hpp>
 
-#include <condition_variable>
-#include <cstdint>
 #include <optional>
 #include <string>
-#include <thread>
 
 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 (file)
index 0000000..e0bdf1e
--- /dev/null
@@ -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 <Logging.hpp>
+#include <Util.hpp>
+#include <util/file.hpp>
+
+#include <chrono>
+
+#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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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 (file)
index 0000000..72fdc5b
--- /dev/null
@@ -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 <NonCopyable.hpp>
+
+#include <condition_variable>
+#include <mutex>
+#include <set>
+#include <string>
+#include <thread>
+
+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<std::string> m_alive_files;
+#endif
+};
+
+} // namespace util
index 7bed76629298b13f11ef4cfc2aeff24faa06c3dc..3086bb4d96a4af8a4b7138fe4c00e1428a2d54ef 100644 (file)
@@ -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());
 }