bin/170775: lockf(1) should support a flag to not create a lock file if one does not exist

Matthew Story matthewstory at gmail.com
Mon Aug 20 00:00:25 UTC 2012


>Number:         170775
>Category:       bin
>Synopsis:       lockf(1) should support a flag to not create a lock file if one does not exist
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          update
>Submitter-Id:   current-users
>Arrival-Date:   Mon Aug 20 00:00:23 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Matthew Story
>Release:        10.0-CURRENT
>Organization:
>Environment:
>Description:
I sometimes want to scan a directory and dispatch a job per file in the directory.  In such cases I typically run a bunch of scanning/executing jobs on the directory and use lock(1) to control concurrent and potentially competing  attempts to scan, think something like:

jot - 1 10 | while read line; do find work/ -depth 1 -print0 | xargs -0 -n1 -I {} lockf -k -t0 {} sh -c '
    # do something
    mv "$1" done/' worker {} &
done

In cases where I want to lock on a file that I know exists, do some work and move on, there is a small race-condition were a secondary job to attempt to lock on a file that had been moved.

In other cases, it would be nice to use mv to short-circuit a coordinated lockf (with -k) effort:

lockf -k foo/bar sh -c 'something || rm foo/bar' &
lockf -k foo/bar sh -c 'second thing || rm foo/bar' & 

To this end, I propose the attached `-n' flag, which augments the behavior of lockf(1) by removing the O_CREAT flag at open(2) time: making the above

jot - 1 10 | while read line; do find work/ -depth 1 -print0 | xargs -0 -n1 -I {} lockf -kn -t0 {} sh -c '
    # do something
    mv "$1" done/' worker {} &
done

and

lockf -k foo/bar sh -c 'something || rm foo/bar' &
lockf -kn foo/bar sh -c 'second thing || rm foo/bar'

The patch causes exit due to nonexistence to exit with EX_UNAVAILABLE if -n is specified and the lock file does not exist.  

The patch does not distinguish between failure due to missing intermediate directory and missing lock-file, although lockf could probably be cleaned up to use open + fopenat to cut down on stats for retried locks, which would also allow -n to distinguish between the two failure cases trivially.

The patch does not cause failure due to missing lock file to have it's diagnostic information suppressed by the -s (silent) flag, although perhaps it should (I was unsure about which way to go on this one).
>How-To-Repeat:

>Fix:
patch attached, also available via HTTP: 

http://axe0.blackskyresearch.net/patches/matt/freebsd.nocreate_lockf.patch.txt

Patch attached with submission follows:

Index: usr.bin/lockf/lockf.c
===================================================================
--- usr.bin/lockf/lockf.c	(revision 239407)
+++ usr.bin/lockf/lockf.c	(working copy)
@@ -56,12 +56,13 @@
 int
 main(int argc, char **argv)
 {
-	int ch, silent, status, waitsec;
+	int ch, flags, silent, status, waitsec;
 	pid_t child;
 
 	silent = keep = 0;
+	flags = O_CREAT;
 	waitsec = -1;	/* Infinite. */
-	while ((ch = getopt(argc, argv, "skt:")) != -1) {
+	while ((ch = getopt(argc, argv, "sknt:")) != -1) {
 		switch (ch) {
 		case 'k':
 			keep = 1;
@@ -69,6 +70,9 @@
 		case 's':
 			silent = 1;
 			break;
+		case 'n':
+			flags &= ~O_CREAT;
+			break;
 		case 't':
 		{
 			char *endptr;
@@ -118,13 +122,13 @@
 	 * avoiding the separate step of waiting for the lock.  This
 	 * yields fairness and improved performance.
 	 */
-	lockfd = acquire_lock(lockname, O_NONBLOCK);
+	lockfd = acquire_lock(lockname, flags|O_NONBLOCK);
 	while (lockfd == -1 && !timed_out && waitsec != 0) {
-		if (keep)
-			lockfd = acquire_lock(lockname, 0);
+		if (keep) 
+			lockfd = acquire_lock(lockname, flags);
 		else {
 			wait_for_lock(lockname);
-			lockfd = acquire_lock(lockname, O_NONBLOCK);
+			lockfd = acquire_lock(lockname, flags|O_NONBLOCK);
 		}
 	}
 	if (waitsec > 0)
@@ -165,9 +169,12 @@
 {
 	int fd;
 
-	if ((fd = open(name, O_RDONLY|O_CREAT|O_EXLOCK|flags, 0666)) == -1) {
+	if ((fd = open(name, O_RDONLY|O_EXLOCK|flags, 0666)) == -1) {
 		if (errno == EAGAIN || errno == EINTR)
 			return (-1);
+		else if (errno == ENOENT && (flags&O_CREAT) == 0)
+			err(EX_UNAVAILABLE, "%s", name);
+
 		err(EX_CANTCREAT, "cannot open %s", name);
 	}
 	return (fd);
@@ -215,7 +222,7 @@
 {
 
 	fprintf(stderr,
-	    "usage: lockf [-ks] [-t seconds] file command [arguments]\n");
+	    "usage: lockf [-ksn] [-t seconds] file command [arguments]\n");
 	exit(EX_USAGE);
 }
 
Index: usr.bin/lockf/lockf.1
===================================================================
--- usr.bin/lockf/lockf.1	(revision 239407)
+++ usr.bin/lockf/lockf.1	(working copy)
@@ -90,6 +90,18 @@
 .Nm
 to operate silently.
 Failure to acquire the lock is indicated only in the exit status.
+.It Fl n
+Causes
+.Nm
+to fail if the specified lock
+.Ar file
+does not exist. If
+.Fl n
+is not specified,
+.Nm
+will create
+.Ar file
+if necessary.
 .It Fl t Ar seconds
 Specifies a timeout for waiting for the lock.
 By default,
@@ -130,6 +142,10 @@
 utility
 was unable to create the lock file, e.g., because of insufficient access
 privileges.
+.It Dv EX_UNAVAILABLE
+The
+.Fl n
+option is specified and the specified lock file does not exist.
 .It Dv EX_USAGE
 There was an error on the
 .Nm


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list