cache issue

[john at keeping.me.uk (John Keeping)]

From eb741581ec16d3249e7d207c3dbc4a433a8f329b Mon Sep 17 00:00:00 2001
Message-Id: <eb741581ec16d3249e7d207c3dbc4a433a8f329b.1425410489.git.john at keeping.me.uk>
From: John Keeping <john at keeping.me.uk>
Date: Tue, 3 Mar 2015 19:01:24 +0000
Subject: [PATCH] cache: use F_SETLK to avoid stale lock files

If CGit is killed while it holds a lock on a cache slot (for example
because it is taking too long to generate a page), the lock file will be
left in place.  This prevents any future attempt to use the same slot
since it will fail to exclusively create the lock file.

Since CGit is the only program that should be manipulating lock files,
we can use advisory locking to detect whether another process is
actually using the lock file or if it is now stale.

I have confirmed that this works on Linux by setting a short TTL in a
custom cgitrc and running the following with CGit patched to print a
message to stderr if the fcntl(2) fails:

	$ export CGIT_CONFIG=$PWD/cgitrc
	$ export QUERY_STRING=url=cgit/tree/ui-shared.c
	$ ./cgit |
		grep -v -e '^<div class=.footer.>' \
			-e '^Last-Modified: ' \
			-e ^'Expires: ' >expect
	$ seq 50000 | dd bs=8192 |
		parallel -j200 "diff -u expect <(./cgit |
			grep -v -e '^<div class=.footer.>' \
				-e '^Last-Modified: ' \
				-e ^'Expires: ') || echo BAD"

This printed the fail message several times without ever printing "BAD".

Signed-off-by: John Keeping <john at keeping.me.uk>
---
On Tue, Mar 03, 2015 at 04:40:01PM +0100, Jason A. Donenfeld wrote:
> Feel free to send a patch, John.

Here it is!

 cache.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/cache.c b/cache.c
index 801e63f..900b161 100644
--- a/cache.c
+++ b/cache.c
@@ -161,10 +161,23 @@ 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;
+		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;
-- 
2.3.1.308.g754cd77