diff options
author | Joel Rosdahl <joel@rosdahl.net> | 2023-01-05 11:07:06 +0100 |
---|---|---|
committer | Joel Rosdahl <joel@rosdahl.net> | 2023-01-15 21:33:41 +0100 |
commit | 00b9a854bc20f5c27937682d266bf0b23142163a (patch) | |
tree | 2f9a31714776226bd2e4f6aded152d201ade5d8c | |
parent | 26f8e94b86df915a33152f35697b5446442efb1b (diff) | |
download | ccache-00b9a854bc20f5c27937682d266bf0b23142163a.tar.gz |
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.
-rw-r--r-- | src/util/LockFile.cpp | 23 | ||||
-rw-r--r-- | unittest/test_util_LockFile.cpp | 30 |
2 files changed, 14 insertions, 39 deletions
diff --git a/src/util/LockFile.cpp b/src/util/LockFile.cpp index 2e5bef32..a9deab30 100644 --- 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 3086bb4d..6a91841c 100644 --- 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; |