Skip to content

Commit d7cbab5

Browse files
ma30002000MiguelCompany
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 d5fb295 commit d7cbab5

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
@@ -171,30 +171,42 @@ class RobustExclusiveLock
171171
const std::string& file_path,
172172
bool* was_lock_created)
173173
{
174-
auto fd = open(file_path.c_str(), O_RDONLY, 0);
175-
176-
if (fd != -1)
177-
{
178-
*was_lock_created = false;
179-
}
180-
else
181-
{
182-
*was_lock_created = true;
183-
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
184-
}
185-
186-
if (fd == -1)
174+
int fd = -1;
175+
do
187176
{
188-
return -1;
189-
}
190-
191-
// Lock the file
192-
if (0 != flock(fd, LOCK_EX | LOCK_NB))
193-
{
194-
close(fd);
195-
return -1;
196-
}
197-
177+
fd = open(file_path.c_str(), O_RDONLY, 0);
178+
179+
if (fd != -1)
180+
{
181+
*was_lock_created = false;
182+
}
183+
else
184+
{
185+
*was_lock_created = true;
186+
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
187+
}
188+
189+
if (fd == -1)
190+
{
191+
return -1;
192+
}
193+
194+
// Lock the file
195+
if (0 != flock(fd, LOCK_EX | LOCK_NB))
196+
{
197+
close(fd);
198+
return -1;
199+
}
200+
201+
// Check if file was deleted by clean up script between open and lock
202+
// if yes, repeat file creation
203+
struct stat buffer = {};
204+
if (stat(file_path.c_str(), &buffer) != 0 && errno == ENOENT)
205+
{
206+
close(fd);
207+
fd = -1;
208+
}
209+
} while (fd == -1);
198210
return fd;
199211
}
200212

src/cpp/utils/shared_memory/RobustSharedLock.hpp

+40-26
Original file line numberDiff line numberDiff line change
@@ -237,40 +237,54 @@ class RobustSharedLock
237237
bool* was_lock_created,
238238
bool* was_lock_released)
239239
{
240-
auto fd = open(file_path.c_str(), O_RDONLY, 0);
241-
242-
if (fd != -1)
243-
{
244-
*was_lock_created = false;
245-
}
246-
else
240+
int fd = -1;
241+
do
247242
{
248-
*was_lock_created = true;
249-
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
250-
}
243+
fd = open(file_path.c_str(), O_RDONLY, 0);
251244

252-
if (was_lock_released != nullptr)
253-
{
254-
// Lock exclusive
255-
if (0 == flock(fd, LOCK_EX | LOCK_NB))
245+
if (fd != -1)
256246
{
257-
// Exclusive => shared
258-
flock(fd, LOCK_SH | LOCK_NB);
259-
*was_lock_released = true;
260-
return fd;
247+
*was_lock_created = false;
261248
}
262249
else
263250
{
264-
*was_lock_released = false;
251+
*was_lock_created = true;
252+
fd = open(file_path.c_str(), O_CREAT | O_RDONLY, 0666);
265253
}
266-
}
267254

268-
// Lock shared
269-
if (0 != flock(fd, LOCK_SH | LOCK_NB))
270-
{
271-
close(fd);
272-
throw std::runtime_error(("failed to lock " + file_path).c_str());
273-
}
255+
if (was_lock_released != nullptr)
256+
{
257+
// Lock exclusive
258+
if (0 == flock(fd, LOCK_EX | LOCK_NB))
259+
{
260+
// Check if file was deleted by clean up script between open and lock
261+
// if yes, repeat file creation
262+
struct stat buffer = {};
263+
if (stat(file_path.c_str(), &buffer) != 0 && errno == ENOENT)
264+
{
265+
close(fd);
266+
fd = -1;
267+
continue;
268+
}
269+
270+
// Exclusive => shared
271+
flock(fd, LOCK_SH | LOCK_NB);
272+
*was_lock_released = true;
273+
return fd;
274+
}
275+
else
276+
{
277+
*was_lock_released = false;
278+
}
279+
}
280+
281+
// Lock shared
282+
if (0 != flock(fd, LOCK_SH | LOCK_NB))
283+
{
284+
close(fd);
285+
throw std::runtime_error(("failed to lock " + file_path).c_str());
286+
}
287+
} while (fd == -1);
274288

275289
return fd;
276290
}

0 commit comments

Comments
 (0)