lock_file_t: Change to POSIX record locks on non-Windows systems

This commit is contained in:
m
2021-06-28 13:20:50 +02:00
parent 063de34924
commit 74bb8b7760
5 changed files with 29 additions and 140 deletions
+14 -127
View File
@@ -68,23 +68,6 @@ std::wstring construct_mutex_name(const std::string& path) {
return NAME_PREFIX + name;
}
#endif
#if !defined(_WIN32)
// The max file lock age before it is considered *definitely* stale, in seconds.
// Note: We set this high to accomodate for time zone differences, clock errors, etc.
const time::seconds_t MAX_FILE_LOCK_AGE = 24 * 3600;
bool file_is_too_old(const std::string& path) {
try {
const auto age = time::seconds_since_epoch() - get_file_info(path).modify_time();
return age > MAX_FILE_LOCK_AGE;
} catch (const std::exception& e) {
// Note: This is not necessarily an error, since the lock file may no longer exist.
debug::log(debug::DEBUG) << "Unable to determine file age for " << path << ": " << e.what();
return false;
}
}
#endif // !_WIN32
} // namespace
file_lock_t::file_lock_t(const std::string& path, const bool remote_lock) : m_path(path) {
@@ -157,115 +140,19 @@ file_lock_t::file_lock_t(const std::string& path, const bool remote_lock) : m_pa
// benefit is negligable, and there are many inherent problems with that solution.
(void)remote_lock;
// Time values are in microseconds.
const int64_t MAX_WAIT_TIME = 10000000; // We'll fail if the lock can't be acquired in 10s.
const int64_t TIME_BETWEEN_LOCK_BREAKS = 100000; // We try to break the lock every 100ms.
const int64_t MIN_SLEEP_TIME = 10;
const int64_t MAX_SLEEP_TIME = 50000;
int64_t total_wait_time = 0;
int64_t sleep_time = MIN_SLEEP_TIME;
int64_t time_until_lock_break = TIME_BETWEEN_LOCK_BREAKS;
while (total_wait_time < MAX_WAIT_TIME) {
// Try to create the lock file in exclusive mode.
m_file_handle = ::open(path.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0666);
if (m_file_handle != invalid_file_handle()) {
// We got the lock! Write our PID to the file.
const auto pid_str = std::to_string(getpid());
const auto bytes_written = ::write(m_file_handle, pid_str.data(), pid_str.size());
if (bytes_written != static_cast<ssize_t>(pid_str.size())) {
::unlink(path.c_str());
::close(m_file_handle);
m_file_handle = invalid_file_handle();
debug::log(debug::ERROR) << "Failed to write our PID to the lock file " << path;
}
break;
// Open the lock file (create if necessary).
m_file_handle = ::open(path.c_str(), O_WRONLY | O_CREAT, 0666);
if (m_file_handle != invalid_file_handle()) {
// Use POSIX record locks to lock the entire file in exclusive mode. We acquire the lock in
// blocking mode (i.e. wait for the lock to be available).
struct flock fl = {};
fl.l_type = F_WRLCK; // Exclusive mode.
fl.l_whence = SEEK_SET; // Lock entire file (l_start = 0, l_len = 0).
if (::fcntl(m_file_handle, F_SETLKW, &fl) == -1) {
debug::log(debug::ERROR) << "Failed to acquire a lock on the lock file " << m_path;
::close(m_file_handle);
m_file_handle = invalid_file_handle();
}
if (errno != EEXIST) {
debug::log(debug::ERROR) << "Failed to open the lock file (\"" << std::strerror(errno)
<< "\")" << path;
break;
}
// The file was already taken (errno == EEXIST), so try again soon.
// Time to check if we can break the lock if it's stale.
if (time_until_lock_break < 0) {
auto fd = ::open(path.c_str(), O_RDONLY);
if (fd < 0) {
if (errno != ENOENT) {
debug::log(debug::ERROR) << "Unable to open possibly stale lock for reading: " << path;
break;
}
// File was removed since we last tried to open it. Try again...
} else {
// Read the PID from the file.
std::string owner_pid_str;
{
char buf[100];
size_t bytes_left = sizeof(buf) - 1;
ssize_t bytes_read = 0;
bool finished_reading = false;
while (!finished_reading) {
const auto bytes = ::read(fd, &buf[bytes_read], bytes_left);
if (bytes == 0) {
finished_reading = true;
} else if (bytes < 0) {
debug::log(debug::INFO) << "Unable to read PID from possibly stale lock " << path;
break;
} else {
bytes_left -= static_cast<size_t>(bytes);
bytes_read += static_cast<size_t>(bytes);
}
}
close(fd);
if (finished_reading) {
owner_pid_str = std::string(buf, static_cast<std::string::size_type>(bytes_read));
}
}
// Convert the file string to a pid_t.
pid_t owner_pid = -1;
try {
owner_pid = static_cast<pid_t>(std::stol(owner_pid_str));
} catch (...) {
debug::log(debug::INFO) << "Invalid PID for possibly stale lock " << path << ": "
<< owner_pid_str;
}
// If we have the PID of the owner process, we can check if it's still alive. We also check
// if the file is too old to reasonably be held at the moment (this should also work across
// different nodes sharing locks on a network drive, provided that their clocks are not
// wildly out of sync).
const auto owner_process_is_dead =
(owner_pid >= 0 && ::kill(owner_pid, 0) == -1) || file_is_too_old(path);
if (owner_process_is_dead) {
// Ok, the process is dead. Let's remove the lock file and hope for better luck during
// the next iteration.
const auto file_removed = (::unlink(m_path.c_str()) != -1);
if (file_removed) {
debug::log(debug::INFO) << "Removed stale lock " << path << " for PID " << owner_pid;
} else {
debug::log(debug::INFO)
<< "Unable to remove stale lock " << path << " for PID " << owner_pid;
}
// Restart the timer until we try to break the lock again.
time_until_lock_break = TIME_BETWEEN_LOCK_BREAKS;
sleep_time = MIN_SLEEP_TIME;
}
}
}
// Wait for a small period of time to give other processes a chance to release the lock.
// Note: Handle both short and long blocks by increasing the sleep time for every new try.
std::this_thread::sleep_for(std::chrono::microseconds(sleep_time));
total_wait_time += sleep_time;
time_until_lock_break -= sleep_time;
sleep_time = std::min(sleep_time * 2, MAX_SLEEP_TIME);
}
#endif
@@ -314,8 +201,8 @@ file_lock_t::~file_lock_t() {
}
#else
if (m_file_handle != invalid_file_handle()) {
// Delete and close the lock file.
::unlink(m_path.c_str());
// Close (and thus unlock) the lock file. Note that the lock file will stay around on the file
// system (this is intentional, and any cleanup needs to be handled elsewhere).
::close(m_file_handle);
}
#endif
+6 -4
View File
@@ -32,8 +32,11 @@ namespace file {
/// during operations such as file renames or writes.
///
/// When the lock is created, a global named system object is created (if necessary) and acquired.
/// Once the lock goes out of scope, the system object is released. If this is the last process that
/// releases the lock, the system object is deleted.
/// Once the lock goes out of scope, the system object is released.
///
/// After a lock is no longer used by any process, the named system object may or may not be left
/// on the system (depending on the underlying implementation). It is thus up to the user of the
/// lock to guarantee that the necessary cleanup is performed.
///
/// Two system object types are supported:
///
@@ -76,8 +79,7 @@ public:
/// @brief Release the lock.
///
/// This releases the lock that was held. The sycnhronization object (if any) is deleted if this
/// is the last process that holds a lock.
/// This releases the lock that was held.
~file_lock_t();
/// @returns true if the lock was acquired successfully.
+3
View File
@@ -84,6 +84,9 @@ int main(int argc, const char** argv) {
const auto file_lockname = filename + ".lock";
const bool local_locks = (std::string(argv[2]) == "true");
// Enable error logging.
debug::set_log_level(debug::ERROR);
long last_count = -1;
for (int i = 0; i < NUM_LOOPS; ++i) {
{
+3 -9
View File
@@ -33,7 +33,7 @@ TEST_CASE("file_lock_t constructors are behaving as expected") {
}
TEST_CASE("Remote locks") {
SUBCASE("Acquiring a lock creates and removes a file") {
SUBCASE("Acquiring a lock creates a file") {
// Get a temporary file name.
file::tmp_file_t tmp_file(file::get_temp_dir(), ".lock");
REQUIRE_GT(tmp_file.path().size(), 0);
@@ -49,9 +49,6 @@ TEST_CASE("Remote locks") {
CHECK_EQ(lock.has_lock(), true);
CHECK_EQ(file::file_exists(tmp_file.path()), true);
}
// The lock file should no longer exist after the lock has gone out of scope.
CHECK_EQ(file::file_exists(tmp_file.path()), false);
}
SUBCASE("Transfering lock ownership works as expected") {
@@ -76,7 +73,7 @@ TEST_CASE("Remote locks") {
// Move the lock object to the parent scope.
lock = std::move(child_lock);
// TThe ownership of the lock should now have moved, and the file should still exist.
// The ownership of the lock should now have moved, and the file should still exist.
CHECK_EQ(child_lock.has_lock(), false);
CHECK_EQ(lock.has_lock(), true);
CHECK_EQ(file::file_exists(tmp_file.path()), true);
@@ -86,14 +83,11 @@ TEST_CASE("Remote locks") {
CHECK_EQ(lock.has_lock(), true);
CHECK_EQ(file::file_exists(tmp_file.path()), true);
}
// The lock file should no longer exist after the lock has gone out of scope.
CHECK_EQ(file::file_exists(tmp_file.path()), false);
}
}
TEST_CASE("Local locks") {
SUBCASE("Acquiring a lock creates and removes a file") {
SUBCASE("Acquiring a lock works as expected") {
// Get a temporary file name.
file::tmp_file_t tmp_file(file::get_temp_dir(), ".lock");
REQUIRE_GT(tmp_file.path().size(), 0);
+3
View File
@@ -34,6 +34,9 @@ function run_test {
DATA=$(cat "$TESTFILE")
rm -f "$TESTFILE"
# Delete the lock file (if any).
rm -f "${TESTFILE}.lock"
# Did we have an error exit status from any of the processes?
if $got_error ; then
echo "*** FAIL: At least one of the processes failed."