From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Sun, 1 Mar 2015 19:36:00 +0000 Subject: cache issue In-Reply-To: References: <20140323140642.GF1223@lemonhead.scabb> <20150228123719.GL890@serenity.lan> Message-ID: <20150301193600.GM890@serenity.lan> On Sun, Mar 01, 2015 at 06:43:17PM +0000, Bertrand Jacquin wrote: > On 28/02/2015 12:37, John Keeping wrote: > > On Sat, Feb 28, 2015 at 12:06:41PM +0000, Bertrand Jacquin wrote: > >> We are still experiencing the issue. Is there any fixes with newer > >> releases ? > > > > I have just tried to reproduce this with the latest version and have > > not > > been able to do so, but I'm not aware of any changes that should have > > an > > effect on this (there is one cache change, 6ceba45 Skip cache slot when > > time-to-live is zero, but that only applies if you set one of the *-ttl > > values to zero). > > > > The cache timeout logic relies on the mtime of the cache file, so this > > could be affected by your filesystem, but it sounds like the problem is > > that the .lock files are not being cleaned up. > > The filesystem here is a ext4 with no specific option except noatime > which quiet common. > > > When CGit finds a .lock > > file for a cache slot it is trying to use it will just serve the stale > > content, on the assumption that is has only just been replaced. > > So there is so assumption the .lock can be obsolete ? > > > I can't see many ways that you can end up with stale lock files; the > > only options are: > > > > 1) CGit crashes, in which case there should be some evidence in the > > system log. > > That might happend, the cgi can in this case be killed after 60 seconds. I wonder if we should be doing something like this (which is probably 3 patches if cleaned up, but shows the idea): -- >8 -- diff --git a/cache.c b/cache.c index e0bb47a..e7649ad 100644 --- a/cache.c +++ b/cache.c @@ -195,6 +195,7 @@ static int unlock_slot(struct cache_slot *slot, int replace_old_slot) else err = unlink(slot->lock_name); + slot->lock_name = NULL; if (err) return errno; @@ -343,6 +344,14 @@ static int process_slot(struct cache_slot *slot) return err; } +static struct cache_slot the_slot; + +void cache_cleanup_locks(void) +{ + if (the_slot.lock_name) + unlock_slot(&the_slot, 0); +} + /* Print cached content to stdout, generate the content if necessary. */ int cache_process(int size, const char *path, const char *key, int ttl, cache_fill_fn fn) @@ -351,7 +360,6 @@ int cache_process(int size, const char *path, const char *key, int ttl, int i; struct strbuf filename = STRBUF_INIT; struct strbuf lockname = STRBUF_INIT; - struct cache_slot slot; int result; /* If the cache is disabled, just generate the content */ @@ -377,13 +385,13 @@ int cache_process(int size, const char *path, const char *key, int ttl, } strbuf_addbuf(&lockname, &filename); strbuf_addstr(&lockname, ".lock"); - slot.fn = fn; - slot.ttl = ttl; - slot.cache_name = filename.buf; - slot.lock_name = lockname.buf; - slot.key = key; - slot.keylen = strlen(key); - result = process_slot(&slot); + the_slot.fn = fn; + the_slot.ttl = ttl; + the_slot.cache_name = filename.buf; + the_slot.lock_name = lockname.buf; + the_slot.key = key; + the_slot.keylen = strlen(key); + result = process_slot(&the_slot); strbuf_release(&filename); strbuf_release(&lockname); diff --git a/cache.h b/cache.h index 9392836..f0d1c75 100644 --- a/cache.h +++ b/cache.h @@ -28,6 +28,9 @@ extern int cache_process(int size, const char *path, const char *key, int ttl, /* List info about all cache entries on stdout */ extern int cache_ls(const char *path); +/* Cleanup open cache lock files on abnormal exit */ +extern void cache_cleanup_locks(void); + /* Print a message to stdout */ __attribute__((format (printf,1,2))) extern void cache_log(const char *format, ...); diff --git a/cgit.c b/cgit.c index d9a8b1f..0912a3d 100644 --- a/cgit.c +++ b/cgit.c @@ -1031,6 +1031,26 @@ static int calc_ttl() return ctx.cfg.cache_repo_ttl; } +static void cleanup_handler(int signum) +{ + cache_cleanup_locks(); +} + +static void register_signal_handlers(void) +{ + struct sigaction sa = { + .sa_handler = cleanup_handler, + .sa_flags = SA_RESETHAND, + }; + sigemptyset(&sa.sa_mask); + + sigaction(SIGHUP, &sa, NULL); + sigaction(SIGINT, &sa, NULL); + sigaction(SIGQUIT, &sa, NULL); + sigaction(SIGPIPE, &sa, NULL); + sigaction(SIGTERM, &sa, NULL); +} + int main(int argc, const char **argv) { const char *path; @@ -1039,6 +1059,8 @@ int main(int argc, const char **argv) cgit_init_filters(); atexit(cgit_cleanup_filters); + register_signal_handlers(); + prepare_context(); cgit_repolist.length = 0; cgit_repolist.count = 0;