Author: Frank Ch. Eigler
Date: Mon Apr 26 09:58:45 2021 -0400
PR26125: debuginfod client cache - rmdir harder
With PR25365, we accidentally lost the ability to rmdir client-cache
directories corresponding to buildids. Bring this back, with some
attention to a possible race between a client doing cleanup and
another client doing lookups at the same time.
Signed-off-by: Frank Ch. Eigler
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index ad9dbc0a8a74..9af641ec0e13 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,11 @@
+2021-04-26 Frank Ch. Eigler
+
+ PR26125
+ * debuginfod-client.c (debuginfod_clean_cache): For directory
+ rmdir, check mtime first.
+ (debuginfod_query_server): Try mkdir / mkstemp up to twice,
+ in case of race.
+
2021-04-23 Frank Ch. Eigler
PR27701
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 7fdc6c9f30ec..0170500faaa9 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -307,12 +307,13 @@ debuginfod_clean_cache(debuginfod_client *c,
return -errno;
regex_t re;
- const char * pattern = ".*/[a-f0-9]+/(debuginfo|executable|source.*)$";
+ const char * pattern = ".*/[a-f0-9]+(/debuginfo|/executable|/source.*|)$";
/* include dirs */
if (regcomp (&re, pattern, REG_EXTENDED | REG_NOSUB) != 0)
return -ENOMEM;
FTSENT *f;
long files = 0;
+ time_t now = time(NULL);
while ((f = fts_read(fts)) != NULL)
{
/* ignore any files that do not match the pattern. */
@@ -327,15 +328,16 @@ debuginfod_clean_cache(debuginfod_client *c,
switch (f->fts_info)
{
case FTS_F:
- /* delete file if max_unused_age has been met or exceeded. */
- /* XXX consider extra effort to clean up old tmp files */
- if (time(NULL) - f->fts_statp->st_atime >= max_unused_age)
-unlink (f->fts_path);
+ /* delete file if max_unused_age has been met or exceeded w.r.t.
atime. */
+ if (now - f->fts_statp->st_atime >= max_unused_age)
+(void) unlink (f->fts_path);
break;
case FTS_DP:
- /* Remove if empty. */
- (void) rmdir (f->fts_path);
+ /* Remove if old & empty. Weaken race against concurrent creation
by
+ checking mtime. */
+ if (now - f->fts_statp->st_mtime >= max_unused_age)
+(void) rmdir (f->fts_path);
break;
default:
@@ -715,19 +717,18 @@ debuginfod_query_server (debuginfod_client *c,
}
/* thereafter, goto out0 on error*/
- /* create target directory in cache if not found. */
- struct stat st;
- if (stat(target_cache_dir, &st) == -1 && mkdir(target_cache_dir, 0700) < 0)
-{
- rc = -errno;
- goto out0;
-}
-
- /* NB: write to a temporary file first, to avoid race condition of
- multiple clients checking the cache, while a partially-written or empty
- file is in there, being written from libcurl. */
- fd = mkstemp (target_cache_tmppath);
- if (fd < 0)
+ /* Because of a race with cache cleanup / rmdir, try to mkdir/mkstemp up to
twice. */
+ for(int i=0; i<2; i++) {
+/* (re)create target directory in cache */
+(void) mkdir(target_cache_dir, 0700);
+
+/* NB: write to a temporary file first, to avoid race condition of
+ multiple clients checking the cache, while a partially-written or empty
+ file is in there, being written from libcurl. */
+fd = mkstemp (target_cache_tmppath);
+if (fd >= 0) break;
+ }
+ if (fd < 0) /* Still failed after two iterations. */
{
rc = -errno;
goto out0;
diff --git a/tests/ChangeLog b/tests/ChangeLog
index ac1963650693..4995ba609b29 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-26 Frank Ch. Eigler
+
+ PR26125
+ * run-debuginfod-find.sh: Add test case for cache cleanup rmdir.
+
2021-04-23 Frank Ch. Eigler
* run-debuginfod-find.sh: Add a tiny test for client object reuse.
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index b1911a92f909..2ba31d8266f6 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -570,6 +570,10 @@ mkdir $DEBUGINFOD_CACHE_PATH/malformed
touch $DEBUGINFOD_CACHE_PATH/malformed0
touch $DEBUGINFOD_CACHE_PATH/malformed/malformed1
+# A valid format for an empty buildid subdirectory
+mkdir $DEBUGINFOD_CACHE_PATH/
+touch -d '1970-01-01' $DEBUGINFOD_CACHE_PATH/ # old enough to
guarantee nukage
+
# Trigger a cache clean and run the tests again. The clients should be unable
to
# find the target.
echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s
@@ -585,6 +589,11 @@ if [ ! -f $DEBUGINFOD_CACHE_PATH/malformed0 ] \
exit 1
fi
+if [ -d $DEBUGINFOD_CACHE_PATH/ ]; then
+echo "failed to rmdir old cache dir"
+exit 1
+fi
+
# Test deb
