Re: [PATCH] Enable sub-second timeout in zsystem flock

[Cedric Ware <cedric.ware__bml@xxxxxxxxxxxxxx>]

	Hello,

This is a re-send/update of a patch I'd proposed some time ago
(see http://www.zsh.org/mla/workers/2019/msg00620.html), to allow a
sub-second granularity for the zsystem flock function's timeout in the
zsh/system module.  This version of the patch is against 5.7.1-test-2,
and also updates the documentation in Doc/Zsh/mod_system.yo .

At the time, I'd been answered:

Bart Schaefer (Monday 2019-07-29):
> In future, please either inline patches or attach them as text/plain
> (e.g. with a ".txt" extension instead of ".patch").  Thanks!

Sure, please find it at the end of this mail.

> > implement this timeout: set a timer such as setitimer(2) which sends
> > a SIGALRM on timeout
> 
> Unfortunately that technique can't be used because ALRM is a signal
> that the user is allowed to trap via the "trap" command or TRAPALRM
> function, and which is controlled by the TMOUT variable which is also
> user settable.

Indeed, I see now.  But, to be clear, I didn't use that technique here.
That was just speculation.  With this patch, the function still polls
the lock periodically as before, except now the period can be specified
instead of being hardcoded to 1 second.

Also, I did some testing, but only checked that invoking zsystem flock
with various values for -t and -p seemed to do what I intended.
Is there a more rigorous test framework for this kind of thing, or
anything else to be done that might help this patch be included?

					Thanks, best regards,
					Cedric Ware.


diff -ru zsh-5.7.1-test-2.orig/Doc/Zsh/mod_system.yo zsh-5.7.1-test-2/Doc/Zsh/mod_system.yo
--- zsh-5.7.1-test-2.orig/Doc/Zsh/mod_system.yo	2019-08-17 23:14:27.000000000 +0200
+++ zsh-5.7.1-test-2/Doc/Zsh/mod_system.yo	2020-01-03 21:12:34.623024717 +0100
@@ -196,9 +196,10 @@
 
 By default the shell waits indefinitely for the lock to succeed.
 The option tt(-t) var(timeout) specifies a timeout for the lock in
-seconds; currently this must be an integer.  The shell will attempt
-to lock the file once a second during this period.  If the attempt
-times out, status 2 is returned.
+seconds; fractional seconds are allowed.  The shell will attempt
+to lock the file every var(period) seconds during this period:
+once a second by default, or according to the  value set by option
+tt(-p).  If the attempt times out, status 2 is returned.
 
 If the option tt(-e) is given, the file descriptor for the lock is
 preserved when the shell uses tt(exec) to start a new process;
diff -ru zsh-5.7.1-test-2.orig/Src/Modules/system.c zsh-5.7.1-test-2/Src/Modules/system.c
--- zsh-5.7.1-test-2.orig/Src/Modules/system.c	2019-08-17 23:14:27.000000000 +0200
+++ zsh-5.7.1-test-2/Src/Modules/system.c	2020-01-03 21:13:34.019588503 +0100
@@ -532,6 +532,8 @@
 {
     int cloexec = 1, unlock = 0, readlock = 0;
     zlong timeout = -1;
+    long timeout_retry = 1e6;
+    mnumber timeout_param;
     char *fdvar = NULL;
 #ifdef HAVE_FCNTL_H
     struct flock lck;
@@ -583,7 +585,35 @@
 		} else {
 		    optarg = *args++;
 		}
-		timeout = mathevali(optarg);
+		timeout_param = matheval(optarg);
+		if (!(timeout_param.type & MN_FLOAT)) {
+		    timeout_param.type = MN_FLOAT;
+		    timeout_param.u.d = (double)timeout_param.u.l;
+		}
+		timeout = (zlong)(timeout_param.u.d * 1e6);
+		break;
+
+	    case 'p':
+		/* retry period in seconds */
+		if (optptr[1]) {
+		    optarg = optptr + 1;
+		    optptr += strlen(optarg) - 1;
+		} else if (!*args) {
+		    zwarnnam(nam,
+			     "flock: option %c requires a numeric retry period",
+			     opt);
+		    return 1;
+		} else {
+		    optarg = *args++;
+		}
+		timeout_param = matheval(optarg);
+		if (!(timeout_param.type & MN_FLOAT)) {
+		    timeout_param.type = MN_FLOAT;
+		    timeout_param.u.d = (double)timeout_param.u.l;
+		}
+		zlong timeout_retry_tmp = timeout_param.u.d * 1e6;
+		timeout_retry = (timeout_retry_tmp > LONG_MAX) ?
+		    LONG_MAX : timeout_retry_tmp;
 		break;
 
 	    case 'u':
@@ -647,7 +677,7 @@
     lck.l_len = 0;  /* lock the whole file */
 
     if (timeout > 0) {
-	time_t end = time(NULL) + (time_t)timeout;
+	zlong end = time_clock_us() + timeout;
 	while (fcntl(flock_fd, F_SETLK, &lck) < 0) {
 	    if (errflag) {
                 zclose(flock_fd);
@@ -658,11 +688,15 @@
 		zwarnnam(nam, "failed to lock file %s: %e", args[0], errno);
 		return 1;
 	    }
-	    if (time(NULL) >= end) {
+	    zlong now = time_clock_us();
+	    if (now >= end) {
                 zclose(flock_fd);
 		return 2;
             }
-	    sleep(1);
+	    if (now + timeout_retry > end) {
+		timeout_retry = end - now;
+	    }
+	    zsleep(timeout_retry);
 	}
     } else {
 	while (fcntl(flock_fd, timeout == 0 ? F_SETLK : F_SETLKW, &lck) < 0) {
diff -ru zsh-5.7.1-test-2.orig/Src/utils.c zsh-5.7.1-test-2/Src/utils.c
--- zsh-5.7.1-test-2.orig/Src/utils.c	2019-12-21 09:29:14.000000000 +0100
+++ zsh-5.7.1-test-2/Src/utils.c	2020-01-03 21:13:34.023588812 +0100
@@ -2749,6 +2749,26 @@
 }
 
 /*
+ * Return the current time in microseconds, using the system's
+ * monotonic clock if supported, the wall clock if not.
+ */
+
+/**/
+zlong
+time_clock_us(void)
+{
+#if defined(HAS_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC)
+    struct timespec ts;
+    clock_gettime(CLOCK_MONOTONIC, &ts);
+    return ts.tv_sec * (zlong)1e6 + ts.tv_nsec / 1000;
+#else
+    struct timeval tv;
+    gettimeofday(&tv, NULL);
+    return tv.tv_sec * (zlong)1e6 + tv.tv_usec;
+#endif
+}
+
+/*
  * Sleep for the given number of microseconds --- must be within
  * range of a long at the moment, but this is only used for
  * limited internal purposes.