From d4bbcdb652ffcdb9de569960ee626ef4fa716670 Mon Sep 17 00:00:00 2001 From: Matthias Schneider Date: Mon, 14 Oct 2024 10:35:26 +0200 Subject: [PATCH 1/2] Prevent race condition with concurrent cleanup operation in RobustExclusiveLock Signed-off-by: Matthias Schneider --- .../shared_memory/RobustExclusiveLock.hpp | 59 +++++++++++-------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/src/cpp/utils/shared_memory/RobustExclusiveLock.hpp b/src/cpp/utils/shared_memory/RobustExclusiveLock.hpp index c7e67bf8b08..6e5021ce7ac 100644 --- a/src/cpp/utils/shared_memory/RobustExclusiveLock.hpp +++ b/src/cpp/utils/shared_memory/RobustExclusiveLock.hpp @@ -171,30 +171,41 @@ class RobustExclusiveLock const std::string& file_path, bool* was_lock_created) { - auto fd = open(file_path.c_str(), O_RDONLY, 0); - - if (fd != -1) - { - *was_lock_created = false; - } - else - { - *was_lock_created = true; - fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666); - } - - if (fd == -1) - { - return -1; - } - - // Lock the file - if (0 != flock(fd, LOCK_EX | LOCK_NB)) - { - close(fd); - return -1; - } - + int fd = -1; + do { + fd = open(file_path.c_str(), O_RDONLY, 0); + + if (fd != -1) + { + *was_lock_created = false; + } + else + { + *was_lock_created = true; + fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666); + } + + if (fd == -1) + { + return -1; + } + + // Lock the file + if (0 != flock(fd, LOCK_EX | LOCK_NB)) + { + close(fd); + return -1; + } + + // Check if file was deleted by clean up script between open and lock + // if yes, repeat file creation + struct stat buffer = {}; + if (stat(file_path.c_str(), &buffer) != 0 && errno == ENOENT) + { + close(fd); + fd = -1; + } + } while (fd == -1); return fd; } From 109deba7c44950274f072662511c9b5f678e4858 Mon Sep 17 00:00:00 2001 From: Matthias Schneider Date: Mon, 14 Oct 2024 11:37:34 +0200 Subject: [PATCH 2/2] Prevent race condition with concurrent cleanup operation in RobustSharedLock Signed-off-by: Matthias Schneider --- .../utils/shared_memory/RobustSharedLock.hpp | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/src/cpp/utils/shared_memory/RobustSharedLock.hpp b/src/cpp/utils/shared_memory/RobustSharedLock.hpp index 68f18316ebb..3b7918a6bd0 100644 --- a/src/cpp/utils/shared_memory/RobustSharedLock.hpp +++ b/src/cpp/utils/shared_memory/RobustSharedLock.hpp @@ -237,40 +237,53 @@ class RobustSharedLock bool* was_lock_created, bool* was_lock_released) { - auto fd = open(file_path.c_str(), O_RDONLY, 0); + int fd = -1; + do { + fd = open(file_path.c_str(), O_RDONLY, 0); - if (fd != -1) - { - *was_lock_created = false; - } - else - { - *was_lock_created = true; - fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666); - } - - if (was_lock_released != nullptr) - { - // Lock exclusive - if (0 == flock(fd, LOCK_EX | LOCK_NB)) + if (fd != -1) { - // Exclusive => shared - flock(fd, LOCK_SH | LOCK_NB); - *was_lock_released = true; - return fd; + *was_lock_created = false; } else { - *was_lock_released = false; + *was_lock_created = true; + fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666); } - } - // Lock shared - if (0 != flock(fd, LOCK_SH | LOCK_NB)) - { - close(fd); - throw std::runtime_error(("failed to lock " + file_path).c_str()); - } + if (was_lock_released != nullptr) + { + // Lock exclusive + if (0 == flock(fd, LOCK_EX | LOCK_NB)) + { + // Check if file was deleted by clean up script between open and lock + // if yes, repeat file creation + struct stat buffer = {}; + if (stat(file_path.c_str(), &buffer) != 0 && errno == ENOENT) + { + close(fd); + fd = -1; + continue; + } + + // Exclusive => shared + flock(fd, LOCK_SH | LOCK_NB); + *was_lock_released = true; + return fd; + } + else + { + *was_lock_released = false; + } + } + + // Lock shared + if (0 != flock(fd, LOCK_SH | LOCK_NB)) + { + close(fd); + throw std::runtime_error(("failed to lock " + file_path).c_str()); + } + } while (fd == -1); return fd; }