enhance: Make it possible for LockFile::try_acquire to break the lock

If a long-lived lock is stale and has no alive file,
LockFile::try_acquire will never succeed to acquire the lock. Fix this
by creating the alive file for all lock types and making
LockFile::try_acquire exit when lock activity is seen instead of
immediately after failing to acquire the lock.

Another advantage is that a stale lock can now always be broken right
away if the alive file exists.

diff --git a/src/util/LockFile.cpp b/src/util/LockFile.cpp
index 2e5bef320e7da6483636f19ca80961c3a0cb2a65..a9deab30c1f8b12fb834a5e826bdb31d04e21984 100644 (file)
--- a/src/util/LockFile.cpp
+++ b/src/util/LockFile.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2020-2022 Joel Rosdahl and other contributors
+// Copyright (C) 2020-2023 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -153,8 +153,8 @@ LockFile::release()
 #ifndef _WIN32
   if (m_lock_manager) {
     m_lock_manager->deregister_alive_file(m_alive_file);
-    Util::unlink_tmp(m_alive_file);
   }
+  Util::unlink_tmp(m_alive_file);
   Util::unlink_tmp(m_lock_file);
 #else
   CloseHandle(m_handle);
@@ -191,11 +191,12 @@ LockFile::acquire(const bool blocking)
   if (acquired()) {
     LOG("Acquired {}", m_lock_file);
 #ifndef _WIN32
+    LOG("Creating {}", m_alive_file);
+    const auto result = util::write_file(m_alive_file, "");
+    if (!result) {
+      LOG("Failed to write {}: {}", m_alive_file, result.error());
+    }
     if (m_lock_manager) {
-      const auto result = util::write_file(m_alive_file, "");
-      if (!result) {
-        LOG("Failed to write {}: {}", m_alive_file, result.error());
-      }
       m_lock_manager->register_alive_file(m_alive_file);
     }
 #endif
@@ -281,8 +282,11 @@ LockFile::do_acquire(const bool blocking)
     }
 
     const auto last_lock_update = get_last_lock_update();
-    if (last_lock_update) {
-      last_seen_activity = std::max(last_seen_activity, *last_lock_update);
+    if (last_lock_update && *last_lock_update > last_seen_activity) {
+      if (!blocking) {
+        return false;
+      }
+      last_seen_activity = *last_lock_update;
     }
 
     const util::Duration inactive_duration =
@@ -293,9 +297,6 @@ LockFile::do_acquire(const bool blocking)
           m_lock_file,
           inactive_duration.sec(),
           inactive_duration.nsec() / 1'000'000);
-      if (!blocking) {
-        return false;
-      }
     } else if (content == initial_content) {
       // The lock seems to be stale -- break it and try again.
       LOG("Breaking {} since it has been inactive for {}.{:03} seconds",

diff --git a/unittest/test_util_LockFile.cpp b/unittest/test_util_LockFile.cpp
index 3086bb4d96a4af8a4b7138fe4c00e1428a2d54ef..6a91841c7bf133061bbbb9816c83221fcd29d034 100644 (file)
--- a/unittest/test_util_LockFile.cpp
+++ b/unittest/test_util_LockFile.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2020-2022 Joel Rosdahl and other contributors
+// Copyright (C) 2020-2023 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -48,7 +48,7 @@ TEST_CASE("Acquire and release short-lived lock file")
 
     CHECK(lock.acquire());
     CHECK(lock.acquired());
-    CHECK(!Stat::lstat("test.alive"));
+    CHECK(Stat::lstat("test.alive"));
     const auto st = Stat::lstat("test.lock");
     CHECK(st);
 #ifndef _WIN32
@@ -65,32 +65,6 @@ TEST_CASE("Acquire and release short-lived lock file")
   CHECK(!Stat::lstat("test.alive"));
 }
 
-TEST_CASE("Non-blocking short-lived lock")
-{
-  TestContext test_context;
-
-  util::LockFile lock_file_1("test");
-  CHECK(!lock_file_1.acquired());
-
-  util::LockFile lock_file_2("test");
-  CHECK(!lock_file_2.acquired());
-
-  CHECK(lock_file_1.try_acquire());
-  CHECK(lock_file_1.acquired());
-
-  CHECK(!lock_file_2.try_acquire());
-  CHECK(lock_file_1.acquired());
-  CHECK(!lock_file_2.acquired());
-
-  lock_file_2.release();
-  CHECK(lock_file_1.acquired());
-  CHECK(!lock_file_2.acquired());
-
-  lock_file_1.release();
-  CHECK(!lock_file_1.acquired());
-  CHECK(!lock_file_2.acquired());
-}
-
 TEST_CASE("Acquire and release long-lived lock file")
 {
   TestContext test_context;