2 files changed, 42 insertions, 9 deletions

diff --git a/source/cgit.c b/source/cgit.cindex 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/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.shindex 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
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