From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Tue, 3 Mar 2015 09:31:24 +0000 Subject: cache issue In-Reply-To: <20150301193600.GM890@serenity.lan> References: <20140323140642.GF1223@lemonhead.scabb> <20150228123719.GL890@serenity.lan> <20150301193600.GM890@serenity.lan> Message-ID: <20150303093124.GN890@serenity.lan> On Sun, Mar 01, 2015 at 07:36:00PM +0000, John Keeping wrote: > 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): Having thought about this a bit more, maybe this is the safer way to do it, since this will detect stale .lock files no matter how they're caused. -- >8 -- diff --git a/cache.c b/cache.c index 801e63f..dbfa6a9 100644 --- a/cache.c +++ b/cache.c @@ -161,10 +161,24 @@ static int close_lock(struct cache_slot *slot) */ static int lock_slot(struct cache_slot *slot) { - slot->lock_fd = open(slot->lock_name, O_RDWR | O_CREAT | O_EXCL, + struct flock lock = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 0, + }; + + slot->lock_fd = open(slot->lock_name, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR); if (slot->lock_fd == -1) return errno; + if (fcntl(slot->lock_fd, F_SETLK, &lock) < 0) { + int saved_errno = errno; + fprintf(stderr, "failed to lock slot!"); + close(slot->lock_fd); + slot->lock_fd = -1; + return saved_errno; + } if (xwrite(slot->lock_fd, slot->key, slot->keylen + 1) < 0) return errno; return 0;