From 9f01ec9157da30843df7866625350fcd9d9b3534 Mon Sep 17 00:00:00 2001
From: Bryce Kwon <bryce@brycekwon.com>
Date: Sat, 31 Oct 2026 14:09:59 -0700
Subject: Lock the cached repolist with flock

The slot lock has to be an flock, since an exec filter drops a record
lock when it closes the stdout a fill writes through. The scan only got
by with fcntl because nothing reopens its file while it runs. One kind
of lock lets the two share their code, and on Linux, where the kinds
never see each other, an flock on the scan's file now holds a scan off
like any other.
---
 source/cgit.c        | 13 ++++---------
 tests/t0003-cache.sh | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/source/cgit.c b/source/cgit.c
index f5f3138..e98210f 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -27,6 +27,7 @@
 #include "ui-shared.h"
 #include "ui-snapshot.h"
 #include "ui-stats.h"
+#include <sys/file.h>
 
 // Deliberately small. The whole body is read at once and handed to the auth
 // filter, so a filter never has to defend itself against a huge or a
@@ -317,19 +318,13 @@ static void parse_args(int argc, const char **argv)
 }
 
 /*
- * The lock is a fcntl lock rather than the mere existence of the lock file,
+ * The lock is an flock rather than the mere existence of the lock file,
  * because a lock the kernel drops with its process cannot outlive a scan
  * killed mid-run. A leftover file would count as a scan forever in progress.
  */
 static int generate_cached_repolist(const char *path, const char *cached_rc)
 {
 	struct strbuf locked_rc = STRBUF_INIT;
-	struct flock lock = {
-		.l_type = F_WRLCK,
-		.l_whence = SEEK_SET,
-		.l_start = 0,
-		.l_len = 0,
-	};
 	struct stat held, named;
 	int err = 0;
 	int fd;
@@ -343,13 +338,13 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		goto out;
 	}
-	if (fcntl(fd, F_SETLK, &lock) < 0) {
+	if (flock(fd, LOCK_EX | LOCK_NB) < 0) {
 		// A lock held elsewhere only means concurrent requests, which
 		// is not worth a line in the server log. Any other failure, a
 		// filesystem without lock support say, silently costs a scan
 		// on every request and does deserve one.
 		err = errno;
-		if (err != EACCES && err != EAGAIN)
+		if (err != EWOULDBLOCK)
 			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		close(fd);
 		goto out;
diff --git a/tests/t0003-cache.sh b/tests/t0003-cache.sh
index 945ac63..8b3f2f9 100755
--- a/tests/t0003-cache.sh
+++ b/tests/t0003-cache.sh
@@ -294,4 +294,42 @@ test_expect_success PERL 'a filter holds the lock file only as its stdout' '
 	grep -F "lock file on fd 1." fd.out
 '
 
+# A scan is cached in a file named after the hash of its path, so a first
+# scan finds out what that name is. The index page is left uncached, which
+# makes each request read the list afresh.
+test_expect_success 'set up a cached scan of two repositories' '
+	mkrepo scanned/one 1 &&
+	mkrepo scanned/two 1 &&
+	{
+		echo "virtual-root=/" &&
+		echo "cache-root=$PWD/cache5" &&
+		echo "cache-size=64" &&
+		echo "cache-index-ttl=0" &&
+		echo "scan-path=$PWD/scanned"
+	} >scanrc &&
+	rm -rf cache5 && mkdir cache5 &&
+	CGIT_CONFIG="$PWD/scanrc" QUERY_STRING="url=" cgit >/dev/null &&
+	scanned_rc=$(ls cache5) &&
+	test_path_is_file "cache5/$scanned_rc"
+'
+
+# A request that finds the scan's lock held and has no list to go on scans
+# in its own process, leaving the file to the holder. That is only another
+# request at work and no cause for a line in the log.
+test_expect_success PERL 'a scan finding its lock held runs uncached' '
+	write_script hold-lock "$PERL_PATH" <<-\EOF &&
+	use Fcntl qw(:flock);
+	open(my $lock, ">>", shift) or die "$!\n";
+	flock($lock, LOCK_EX | LOCK_NB) or die "$!\n";
+	exit(system(@ARGV) ? 1 : 0);
+	EOF
+	rm "cache5/$scanned_rc" &&
+	CGIT_CONFIG="$PWD/scanrc" QUERY_STRING="url=" \
+	./hold-lock "cache5/$scanned_rc.lock" cgit >held.out 2>held.err &&
+	grep ">one</a>" held.out &&
+	grep ">two</a>" held.out &&
+	test_path_is_missing "cache5/$scanned_rc" &&
+	test_must_be_empty held.err
+'
+
 test_done
-- 
cgit