Skip to content

Commit

Permalink
Fix cleanup race condition with exclusive and shared lock files (#5319)
Browse files Browse the repository at this point in the history
* Prevent race condition with concurrent cleanup operation in RobustExclusiveLock

Signed-off-by: Matthias Schneider <ma30002000@yahoo.de>

* Prevent race condition with concurrent cleanup operation in RobustSharedLock

Signed-off-by: Matthias Schneider <ma30002000@yahoo.de>

* Adapted coding style as suggested by MiguelCompany

Signed-off-by: Matthias Schneider <ma30002000@yahoo.de>

---------

Signed-off-by: Matthias Schneider <ma30002000@yahoo.de>
  • Loading branch information
ma30002000 authored Mar 7, 2025
1 parent 791012c commit 53bf6ab
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 49 deletions.
58 changes: 35 additions & 23 deletions src/cpp/utils/shared_memory/RobustExclusiveLock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,30 +212,42 @@ 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)
int fd = -1;
do
{
return -1;
}

// Lock the file
if (0 != flock(fd, LOCK_EX | LOCK_NB))
{
close(fd);
return -1;
}

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;
}

Expand Down
66 changes: 40 additions & 26 deletions src/cpp/utils/shared_memory/RobustSharedLock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,40 +351,54 @@ class RobustSharedLock
bool* was_lock_created,
bool* was_lock_released)
{
auto fd = open(file_path.c_str(), O_RDONLY, 0);

if (fd != -1)
{
*was_lock_created = false;
}
else
int fd = -1;
do
{
*was_lock_created = true;
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
}
fd = open(file_path.c_str(), O_RDONLY, 0);

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;
}
Expand Down

0 comments on commit 53bf6ab

Please sign in to comment.