cache issue

[John Keeping]

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;