From 95edde45e53fc84ce30449663d9f2145328bb877 Mon Sep 17 00:00:00 2001 From: "Frank Ch. Eigler" Date: Mon, 26 Apr 2021 09:58:45 -0400 Subject: [PATCH] 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 --- debuginfod/ChangeLog | 8 ++++++++ debuginfod/debuginfod-client.c | 41 +++++++++++++++++++++-------------------- tests/ChangeLog | 5 +++++ tests/run-debuginfod-find.sh | 9 +++++++++ 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index ad9dbc0..9af641e 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 7fdc6c9..0170500 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 eac006d..0d2c5ed 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -8,6 +8,11 @@ (EXTRA_DIST): Add run-low_high_pc-dw-form-indirect.sh, run-readelf-dw-form-indirect.sh, and testfile-dw-form-indirect.bz2. +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 11e8498..3b9a5a6 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -571,6 +571,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/00000000 +touch -d '1970-01-01' $DEBUGINFOD_CACHE_PATH/00000000 # 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 @@ -586,6 +590,11 @@ if [ ! -f $DEBUGINFOD_CACHE_PATH/malformed0 ] \ exit 1 fi +if [ -d $DEBUGINFOD_CACHE_PATH/00000000 ]; then + echo "failed to rmdir old cache dir" + exit 1 +fi + # Test debuginfod without a path list; reuse $PORT1 env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -F -U -d :memory: -p $PORT1 -L -F & PID3=$! -- 2.7.4