Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 26 Nov 2023 04:16:01 GMT
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 09a7fe0a5523 - main - lockf: allow locking file descriptors
Message-ID:  <202311260416.3AQ4G1BN045533@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=09a7fe0a5523d53ff6c26ddef9a947f293e18c22

commit 09a7fe0a5523d53ff6c26ddef9a947f293e18c22
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-11-22 07:46:14 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-11-26 04:15:05 +0000

    lockf: allow locking file descriptors
    
    This is most useful inside a shell script, allowing one to lock just
    portions of a script rather than having to wrap the entire script in a
    lock.
    
    PR:             262738
    Reviewed by:    0mp, allanjude (both previous versions)
    Co-authored-by: Daniel O'Connor <darius@dons.net.au>
    Sponsored by:   Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D42718
---
 usr.bin/lockf/lockf.1             |  63 ++++++++++++++++++-
 usr.bin/lockf/lockf.c             | 128 +++++++++++++++++++++++++++++++++-----
 usr.bin/lockf/tests/lockf_test.sh |  91 +++++++++++++++++++++++++++
 3 files changed, 266 insertions(+), 16 deletions(-)

diff --git a/usr.bin/lockf/lockf.1 b/usr.bin/lockf/lockf.1
index 8120b2ed7630..d73033101632 100644
--- a/usr.bin/lockf/lockf.1
+++ b/usr.bin/lockf/lockf.1
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd August 26, 2020
+.Dd November 25, 2023
 .Dt LOCKF 1
 .Os
 .Sh NAME
@@ -35,6 +35,10 @@
 .Ar file
 .Ar command
 .Op Ar arguments
+.Nm
+.Op Fl s
+.Op Fl t Ar seconds
+.Ar fd
 .Sh DESCRIPTION
 The
 .Nm
@@ -64,6 +68,27 @@ the mere existence of the
 .Ar file
 is not considered to constitute a lock.
 .Pp
+.Nm
+may also be used to operate on a file descriptor instead of a file.
+If no
+.Ar command
+is supplied, then
+.Ar fd
+must be a file descriptor.
+The version with a
+.Ar command
+may also be used with a file descriptor by supplying it as a path
+.Pa /dev/fd/N ,
+where N is the desired file descriptor.
+The
+.Fl k
+option is implied when a file descriptor is in use, and the
+.Fl n
+and
+.Fl w
+options are silently ignored.
+This can be used to lock inside a shell script.
+.Pp
 If the
 .Nm
 utility is being used to facilitate concurrency between a number
@@ -186,6 +211,42 @@ $ lockf mylock sleep 1 & lockf -t 5 mylock echo "Success"
 Success
 [1]+  Done                    lockf mylock sleep 1
 .Ed
+Lock a file and run a script, return immediately if the lock is not
+available. Do not delete the file afterward so lock order is
+guaranteed.
+.Pp
+.Dl $ lockf -t 0 -k /tmp/my.lock myscript
+.Pp
+Protect a section of a shell script with a lock, wait up to 5 seconds
+for it to become available.
+Note that the shell script has opened the lock file
+.Fa /tmp/my.lock ,
+and
+.Nm
+is performing the lock call exclusively via the passed in file descriptor (9).
+In this case
+.Fl k
+is implied, and
+.Fl w
+has no effect because the file has already been opened by the shell.
+This example assumes that
+.Ql >
+is implemented in the shell by opening and truncating
+.Pa /tmp/my.lock ,
+rather than by replacing the lock file.
+.Bd -literal -offset indent
+(
+	lockf -s -t 5 9
+	if [ $? -ne 0 ]; then
+		echo "Failed to obtain lock"
+		exit 1
+	fi
+
+	echo Start
+	# Do some stuff
+	echo End
+) 9>/tmp/my.lock
+.Ed
 .Sh SEE ALSO
 .Xr flock 2 ,
 .Xr lockf 3 ,
diff --git a/usr.bin/lockf/lockf.c b/usr.bin/lockf/lockf.c
index 620193cba1d7..db45f7a6b2e7 100644
--- a/usr.bin/lockf/lockf.c
+++ b/usr.bin/lockf/lockf.c
@@ -29,16 +29,26 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 
+#include <assert.h>
 #include <err.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sysexits.h>
 #include <unistd.h>
 
-static int acquire_lock(const char *name, int flags, int silent);
+#define	FDLOCK_PREFIX	"/dev/fd/"
+
+union lock_subject {
+	long		 subj_fd;
+	const char	*subj_name;
+};
+
+static int acquire_lock(union lock_subject *subj, int flags, int silent);
 static void cleanup(void);
 static void killed(int sig);
 static void timeout(int sig);
@@ -48,8 +58,34 @@ static void wait_for_lock(const char *name);
 static const char *lockname;
 static int lockfd = -1;
 static int keep;
+static int fdlock;
 static volatile sig_atomic_t timed_out;
 
+/*
+ * Check if fdlock is implied by the given `lockname`.  We'll write the fd that
+ * is represented by it out to ofd, and the caller is expected to do any
+ * necessary validation on it.
+ */
+static int
+fdlock_implied(const char *name, long *ofd)
+{
+	char *endp;
+	long fd;
+
+	if (strncmp(name, FDLOCK_PREFIX, sizeof(FDLOCK_PREFIX) - 1) != 0)
+		return (0);
+
+	/* Skip past the prefix. */
+	name += sizeof(FDLOCK_PREFIX) - 1;
+	errno = 0;
+	fd = strtol(name, &endp, 10);
+	if (errno != 0 || *endp != '\0')
+		return (0);
+
+	*ofd = fd;
+	return (1);
+}
+
 /*
  * Execute an arbitrary command while holding a file lock.
  */
@@ -58,6 +94,7 @@ main(int argc, char **argv)
 {
 	int ch, flags, silent, status, waitsec;
 	pid_t child;
+	union lock_subject subj;
 
 	silent = keep = 0;
 	flags = O_CREAT | O_RDONLY;
@@ -89,11 +126,54 @@ main(int argc, char **argv)
 			usage();
 		}
 	}
-	if (argc - optind < 2)
-		usage();
-	lockname = argv[optind++];
+
 	argc -= optind;
 	argv += optind;
+
+	if (argc == 0)
+		usage();
+
+	lockname = argv[0];
+
+	argc--;
+	argv++;
+
+	/*
+	 * If there aren't any arguments left, then we must be in fdlock mode.
+	 */
+	if (argc == 0 && *lockname != '/') {
+		fdlock = 1;
+		subj.subj_fd = -1;
+	} else {
+		fdlock = fdlock_implied(lockname, &subj.subj_fd);
+		if (argc == 0 && !fdlock) {
+			fprintf(stderr, "Expected fd, got '%s'\n", lockname);
+			usage();
+		}
+	}
+
+	if (fdlock) {
+		if (subj.subj_fd < 0) {
+			char *endp;
+
+			errno = 0;
+			subj.subj_fd = strtol(lockname, &endp, 10);
+			if (errno != 0 || *endp != '\0') {
+				fprintf(stderr, "Expected fd, got '%s'\n",
+				    lockname);
+				usage();
+			}
+		}
+
+		if (subj.subj_fd < 0 || subj.subj_fd > INT_MAX) {
+			fprintf(stderr, "fd '%ld' out of range\n",
+			    subj.subj_fd);
+			usage();
+		}
+	} else {
+		subj.subj_name = lockname;
+	}
+
 	if (waitsec > 0) {		/* Set up a timeout. */
 		struct sigaction act;
 
@@ -125,13 +205,13 @@ main(int argc, char **argv)
 	 * avoiding the separate step of waiting for the lock.  This
 	 * yields fairness and improved performance.
 	 */
-	lockfd = acquire_lock(lockname, flags | O_NONBLOCK, silent);
+	lockfd = acquire_lock(&subj, flags | O_NONBLOCK, silent);
 	while (lockfd == -1 && !timed_out && waitsec != 0) {
-		if (keep)
-			lockfd = acquire_lock(lockname, flags, silent);
+		if (keep || fdlock)
+			lockfd = acquire_lock(&subj, flags, silent);
 		else {
 			wait_for_lock(lockname);
-			lockfd = acquire_lock(lockname, flags | O_NONBLOCK,
+			lockfd = acquire_lock(&subj, flags | O_NONBLOCK,
 			    silent);
 		}
 	}
@@ -142,7 +222,15 @@ main(int argc, char **argv)
 			exit(EX_TEMPFAIL);
 		errx(EX_TEMPFAIL, "%s: already locked", lockname);
 	}
+
 	/* At this point, we own the lock. */
+
+	/* Nothing else to do for FD lock, just exit */
+	if (argc == 0) {
+		assert(fdlock);
+		return 0;
+	}
+
 	if (atexit(cleanup) == -1)
 		err(EX_OSERR, "atexit failed");
 	if ((child = fork()) == -1)
@@ -166,25 +254,34 @@ main(int argc, char **argv)
 }
 
 /*
- * Try to acquire a lock on the given file, creating the file if
+ * Try to acquire a lock on the given file/fd, creating the file if
  * necessary.  The flags argument is O_NONBLOCK or 0, depending on
  * whether we should wait for the lock.  Returns an open file descriptor
  * on success, or -1 on failure.
  */
 static int
-acquire_lock(const char *name, int flags, int silent)
+acquire_lock(union lock_subject *subj, int flags, int silent)
 {
 	int fd;
 
-	if ((fd = open(name, O_EXLOCK|flags, 0666)) == -1) {
+	if (fdlock) {
+		assert(subj->subj_fd >= 0 && subj->subj_fd <= INT_MAX);
+		fd = (int)subj->subj_fd;
+
+		if (flock(fd, LOCK_EX | LOCK_NB) == -1) {
+			if (errno == EAGAIN || errno == EINTR)
+				return (-1);
+			err(EX_CANTCREAT, "cannot lock fd %d", fd);
+		}
+	} else if ((fd = open(subj->subj_name, O_EXLOCK|flags, 0666)) == -1) {
 		if (errno == EAGAIN || errno == EINTR)
 			return (-1);
 		else if (errno == ENOENT && (flags & O_CREAT) == 0) {
 			if (!silent)
-				warn("%s", name);
+				warn("%s", subj->subj_name);
 			exit(EX_UNAVAILABLE);
 		}
-		err(EX_CANTCREAT, "cannot open %s", name);
+		err(EX_CANTCREAT, "cannot open %s", subj->subj_name);
 	}
 	return (fd);
 }
@@ -196,7 +293,7 @@ static void
 cleanup(void)
 {
 
-	if (keep)
+	if (keep || fdlock)
 		flock(lockfd, LOCK_UN);
 	else
 		unlink(lockname);
@@ -231,7 +328,8 @@ usage(void)
 {
 
 	fprintf(stderr,
-	    "usage: lockf [-knsw] [-t seconds] file command [arguments]\n");
+	    "usage: lockf [-knsw] [-t seconds] file command [arguments]\n"
+	    "       lockf [-s] [-t seconds] fd\n");
 	exit(EX_USAGE);
 }
 
diff --git a/usr.bin/lockf/tests/lockf_test.sh b/usr.bin/lockf/tests/lockf_test.sh
index 8696ab82a996..cc6938d2306e 100644
--- a/usr.bin/lockf/tests/lockf_test.sh
+++ b/usr.bin/lockf/tests/lockf_test.sh
@@ -60,6 +60,96 @@ basic_body()
 	atf_check test ! -e "testlock"
 }
 
+atf_test_case fdlock
+fdlock_body()
+{
+	# First, make sure we don't get a false positive -- existing uses with
+	# numeric filenames shouldn't switch to being fdlocks automatically.
+	atf_check lockf -k "9" sleep 0
+	atf_check test -e "9"
+	rm "9"
+
+	subexit_lockfail=1
+	subexit_created=2
+	subexit_lockok=3
+	subexit_concurrent=4
+	(
+		lockf -s -t 0 9
+		if [ $? -ne 0 ]; then
+			exit "$subexit_lockfail"
+		fi
+
+		if [ -e "9" ]; then
+			exit "$subexit_created"
+		fi
+	) 9> "testlock1"
+	rc=$?
+
+	atf_check test "$rc" -eq 0
+
+	sub_delay=5
+
+	# But is it actually locking?  Child 1 will acquire the lock and then
+	# signal that it's ok for the second child to try.  The second child
+	# will try to acquire the lock and fail immediately, signal that it
+	# tried, then try again with an indefinite timeout.  On that one, we'll
+	# just check how long we ended up waiting -- it should be at least
+	# $sub_delay.
+	(
+		lockf -s -t 0 /dev/fd/9
+		if [ $? -ne 0 ]; then
+			exit "$subexit_lockfail"
+		fi
+
+		# Signal
+		touch ".lock_acquired"
+
+		while [ ! -e ".lock_attempted" ]; do
+			sleep 0.5
+		done
+
+		sleep "$sub_delay"
+
+		if [ -e ".lock_acquired_again" ]; then
+			exit "$subexit_concurrent"
+		fi
+	) 9> "testlock2" &
+	lpid1=$!
+
+	(
+		while [ ! -e ".lock_acquired" ]; do
+			sleep 0.5
+		done
+
+		# Got the signal, try
+		lockf -s -t 0 9
+		if [ $? -ne "${EX_TEMPFAIL}" ]; then
+			exit "$subexit_lockok"
+		fi
+
+		touch ".lock_attempted"
+		start=$(date +"%s")
+		lockf -s 9
+		touch ".lock_acquired_again"
+		now=$(date +"%s")
+		elapsed=$((now - start))
+
+		if [ "$elapsed" -lt "$sub_delay" ]; then
+			exit "$subexit_concurrent"
+		fi
+	) 9> "testlock2" &
+	lpid2=$!
+
+	wait "$lpid1"
+	status1=$?
+
+	wait "$lpid2"
+	status2=$?
+
+	atf_check test "$status1" -eq 0
+	atf_check test "$status2" -eq 0
+}
+
 atf_test_case keep
 keep_body()
 {
@@ -141,6 +231,7 @@ atf_init_test_cases()
 {
 	atf_add_test_case badargs
 	atf_add_test_case basic
+	atf_add_test_case fdlock
 	atf_add_test_case keep
 	atf_add_test_case needfile
 	atf_add_test_case timeout



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202311260416.3AQ4G1BN045533>