Skip to content

Commit 82d55e7

Browse files
ma30002000mergify[bot]
authored andcommitted
Fix cleanup race condition with exclusive and shared lock files (#5319)
* Prevent race condition with concurrent cleanup operation in RobustExclusiveLock Signed-off-by: Matthias Schneider <[email protected]> * Prevent race condition with concurrent cleanup operation in RobustSharedLock Signed-off-by: Matthias Schneider <[email protected]> * Adapted coding style as suggested by MiguelCompany Signed-off-by: Matthias Schneider <[email protected]> --------- Signed-off-by: Matthias Schneider <[email protected]> (cherry picked from commit 53bf6ab)
1 parent 735a643 commit 82d55e7

File tree

2 files changed

+75
-49
lines changed

2 files changed

+75
-49
lines changed

src/cpp/utils/shared_memory/RobustExclusiveLock.hpp

+35-23
Original file line numberDiff line numberDiff line change
@@ -212,30 +212,42 @@ class RobustExclusiveLock
212212
const std::string& file_path,
213213
bool* was_lock_created)
214214
{
215-
auto fd = open(file_path.c_str(), O_RDONLY, 0);
216-
217-
if (fd != -1)
218-
{
219-
*was_lock_created = false;
220-
}
221-
else
222-
{
223-
*was_lock_created = true;
224-
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
225-
}
226-
227-
if (fd == -1)
215+
int fd = -1;
216+
do
228217
{
229-
return -1;
230-
}
231-
232-
// Lock the file
233-
if (0 != flock(fd, LOCK_EX | LOCK_NB))
234-
{
235-
close(fd);
236-
return -1;
237-
}
238-
218+
fd = open(file_path.c_str(), O_RDONLY, 0);
219+
220+
if (fd != -1)
221+
{
222+
*was_lock_created = false;
223+
}
224+
else
225+
{
226+
*was_lock_created = true;
227+
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
228+
}
229+
230+
if (fd == -1)
231+
{
232+
return -1;
233+
}
234+
235+
// Lock the file
236+
if (0 != flock(fd, LOCK_EX | LOCK_NB))
237+
{
238+
close(fd);
239+
return -1;
240+
}
241+
242+
// Check if file was deleted by clean up script between open and lock
243+
// if yes, repeat file creation
244+
struct stat buffer = {};
245+
if (stat(file_path.c_str(), &buffer) != 0 && errno == ENOENT)
246+
{
247+
close(fd);
248+
fd = -1;
249+
}
250+
} while (fd == -1);
239251
return fd;
240252
}
241253

src/cpp/utils/shared_memory/RobustSharedLock.hpp

+40-26
Original file line numberDiff line numberDiff line change
@@ -351,40 +351,54 @@ class RobustSharedLock
351351
bool* was_lock_created,
352352
bool* was_lock_released)
353353
{
354-
auto fd = open(file_path.c_str(), O_RDONLY, 0);
355-
356-
if (fd != -1)
357-
{
358-
*was_lock_created = false;
359-
}
360-
else
354+
int fd = -1;
355+
do
361356
{
362-
*was_lock_created = true;
363-
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
364-
}
357+
fd = open(file_path.c_str(), O_RDONLY, 0);
365358

366-
if (was_lock_released != nullptr)
367-
{
368-
// Lock exclusive
369-
if (0 == flock(fd, LOCK_EX | LOCK_NB))
359+
if (fd != -1)
370360
{
371-
// Exclusive => shared
372-
flock(fd, LOCK_SH | LOCK_NB);
373-
*was_lock_released = true;
374-
return fd;
361+
*was_lock_created = false;
375362
}
376363
else
377364
{
378-
*was_lock_released = false;
365+
*was_lock_created = true;
366+
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
379367
}
380-
}
381368

382-
// Lock shared
383-
if (0 != flock(fd, LOCK_SH | LOCK_NB))
384-
{
385-
close(fd);
386-
throw std::runtime_error(("failed to lock " + file_path).c_str());
387-
}
369+
if (was_lock_released != nullptr)
370+
{
371+
// Lock exclusive
372+
if (0 == flock(fd, LOCK_EX | LOCK_NB))
373+
{
374+
// Check if file was deleted by clean up script between open and lock
375+
// if yes, repeat file creation
376+
struct stat buffer = {};
377+
if (stat(file_path.c_str(), &buffer) != 0 && errno == ENOENT)
378+
{
379+
close(fd);
380+
fd = -1;
381+
continue;
382+
}
383+
384+
// Exclusive => shared
385+
flock(fd, LOCK_SH | LOCK_NB);
386+
*was_lock_released = true;
387+
return fd;
388+
}
389+
else
390+
{
391+
*was_lock_released = false;
392+
}
393+
}
394+
395+
// Lock shared
396+
if (0 != flock(fd, LOCK_SH | LOCK_NB))
397+
{
398+
close(fd);
399+
throw std::runtime_error(("failed to lock " + file_path).c_str());
400+
}
401+
} while (fd == -1);
388402

389403
return fd;
390404
}

0 commit comments

Comments
 (0)