Share one lock-file helper between the caches

1 file changed, 45 insertions, 29 deletions

diff --git a/source/cache.c b/source/cache.cindex 6317a12..0a3166d 100644--- a/source/cache.c+++ b/source/cache.c@@ -198,55 +198,71 @@ static int is_modified(struct cache_slot *slot)  * already being rebuilt, so it returns an errno instead of waiting. Every  * other failure is logged here, for both callers.  */-static int lock_slot(struct cache_slot *slot)+int cache_lock_file(const char *path, int *fd) { 	struct stat held, named; 	int err; -	// Close-on-exec, so that a program the fill runs gets the file only-	// as its stdout, and holds the lock no longer than it can write there.-	slot->lock_fd = open(slot->lock_path, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);-	if (slot->lock_fd == -1) {+	// Close-on-exec, so that a program a cache fill runs gets the file+	// only as its stdout, and holds the lock no longer than it can write+	// there.+	*fd = open(path, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);+	if (*fd == -1) { 		err = errno;-		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", slot->lock_path, strerror(err), err);+		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", path, strerror(err), err); 		return err; 	}-	// An flock rather than a POSIX record lock. The descriptor is put on-	// stdout for the fill, and a record lock is dropped as soon as any+	// An flock rather than a POSIX record lock. A slot's descriptor is put+	// on stdout for the fill, and a record lock is dropped as soon as any 	// descriptor on the file closes, which an exec filter does to stdout,-	// while an flock holds until the last descriptor closes.-	if (flock(slot->lock_fd, LOCK_EX | LOCK_NB) < 0) {+	// while an flock holds until the last descriptor closes. Either way a+	// holder that dies takes its lock with it.+	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 keeps the slot-		// from ever being filled or refreshed and does deserve one.+		// filesystem without lock support say, keeps the cache from+		// ever being written and does deserve one. 		err = errno; 		if (err != EWOULDBLOCK)-			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", slot->lock_path, strerror(err), err);-		close(slot->lock_fd);-		slot->lock_fd = -1;-		return err;+			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", path, strerror(err), err);+		goto fail; 	} 	// The lock landed on whatever inode the path named at open. A holder-	// finishing in between renames that inode into place as the live-	// slot, and once the path is confirmed to still name this file that-	// rename can no longer happen, because it takes the lock held here.+	// finishing in between renames that inode into place, and once the+	// path is confirmed to still name this file that rename can no longer+	// happen, because it takes the lock held here. 	if (-		fstat(slot->lock_fd, &held)   ||-		stat(slot->lock_path, &named) ||-		held.st_ino != named.st_ino   ||+		fstat(*fd, &held)           ||+		stat(path, &named)          ||+		held.st_ino != named.st_ino || 		held.st_dev != named.st_dev 	) {-		close(slot->lock_fd);-		slot->lock_fd = -1;-		return EAGAIN;+		err = EAGAIN;+		goto fail; 	} 	// A run that died before its rename leaves the lock file behind, so 	// start from empty now that nobody else can be writing it.-	if (-		ftruncate(slot->lock_fd, 0) < 0 ||-		write_in_full(slot->lock_fd, slot->key, slot->keylen + 1) < 0-	) {+	if (ftruncate(*fd, 0) < 0) {+		err = errno;+		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", path, strerror(err), err);+		unlink(path);+		goto fail;+	}+	return 0;++fail:+	close(*fd);+	*fd = -1;+	return err;+}++static int lock_slot(struct cache_slot *slot)+{+	int err = cache_lock_file(slot->lock_path, &slot->lock_fd);++	if (err)+		return err;+	if (write_in_full(slot->lock_fd, slot->key, slot->keylen + 1) < 0) { 		err = errno; 		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", slot->lock_path, strerror(err), err); 		unlink(slot->lock_path);
diff --git a/source/cache.c b/source/cache.c
index 6317a12..0a3166d 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -198,55 +198,71 @@ static int is_modified(struct cache_slot *slot)
  * already being rebuilt, so it returns an errno instead of waiting. Every
  * other failure is logged here, for both callers.
  */
-static int lock_slot(struct cache_slot *slot)
+int cache_lock_file(const char *path, int *fd)
 {
 	struct stat held, named;
 	int err;
 
-	// Close-on-exec, so that a program the fill runs gets the file only
-	// as its stdout, and holds the lock no longer than it can write there.
-	slot->lock_fd = open(slot->lock_path, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);
-	if (slot->lock_fd == -1) {
+	// Close-on-exec, so that a program a cache fill runs gets the file
+	// only as its stdout, and holds the lock no longer than it can write
+	// there.
+	*fd = open(path, O_RDWR | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR);
+	if (*fd == -1) {
 		err = errno;
-		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", slot->lock_path, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", path, strerror(err), err);
 		return err;
 	}
-	// An flock rather than a POSIX record lock. The descriptor is put on
-	// stdout for the fill, and a record lock is dropped as soon as any
+	// An flock rather than a POSIX record lock. A slot's descriptor is put
+	// on stdout for the fill, and a record lock is dropped as soon as any
 	// descriptor on the file closes, which an exec filter does to stdout,
-	// while an flock holds until the last descriptor closes.
-	if (flock(slot->lock_fd, LOCK_EX | LOCK_NB) < 0) {
+	// while an flock holds until the last descriptor closes. Either way a
+	// holder that dies takes its lock with it.
+	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 keeps the slot
-		// from ever being filled or refreshed and does deserve one.
+		// filesystem without lock support say, keeps the cache from
+		// ever being written and does deserve one.
 		err = errno;
 		if (err != EWOULDBLOCK)
-			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", slot->lock_path, strerror(err), err);
-		close(slot->lock_fd);
-		slot->lock_fd = -1;
-		return err;
+			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", path, strerror(err), err);
+		goto fail;
 	}
 	// The lock landed on whatever inode the path named at open. A holder
-	// finishing in between renames that inode into place as the live
-	// slot, and once the path is confirmed to still name this file that
-	// rename can no longer happen, because it takes the lock held here.
+	// finishing in between renames that inode into place, and once the
+	// path is confirmed to still name this file that rename can no longer
+	// happen, because it takes the lock held here.
 	if (
-		fstat(slot->lock_fd, &held)   ||
-		stat(slot->lock_path, &named) ||
-		held.st_ino != named.st_ino   ||
+		fstat(*fd, &held)           ||
+		stat(path, &named)          ||
+		held.st_ino != named.st_ino ||
 		held.st_dev != named.st_dev
 	) {
-		close(slot->lock_fd);
-		slot->lock_fd = -1;
-		return EAGAIN;
+		err = EAGAIN;
+		goto fail;
 	}
 	// A run that died before its rename leaves the lock file behind, so
 	// start from empty now that nobody else can be writing it.
-	if (
-		ftruncate(slot->lock_fd, 0) < 0 ||
-		write_in_full(slot->lock_fd, slot->key, slot->keylen + 1) < 0
-	) {
+	if (ftruncate(*fd, 0) < 0) {
+		err = errno;
+		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", path, strerror(err), err);
+		unlink(path);
+		goto fail;
+	}
+	return 0;
+
+fail:
+	close(*fd);
+	*fd = -1;
+	return err;
+}
+
+static int lock_slot(struct cache_slot *slot)
+{
+	int err = cache_lock_file(slot->lock_path, &slot->lock_fd);
+
+	if (err)
+		return err;
+	if (write_in_full(slot->lock_fd, slot->key, slot->keylen + 1) < 0) {
 		err = errno;
 		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", slot->lock_path, strerror(err), err);
 		unlink(slot->lock_path);