zsh-workers
 help / Atom feed
* [PATCH] Enable sub-second timeout in zsystem flock
@ 2019-07-29 20:35 Cedric Ware
  2019-07-29 22:25 ` Bart Schaefer
  0 siblings, 1 reply; 2+ messages in thread
From: Cedric Ware @ 2019-07-29 20:35 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]


	Hello,

I'm interested in using the zsystem flock function in the zsh/system
module to lock files, with a timeout if the lock is already taken, but
currently the timeout value has a whole-second granularity and I'd like
to specify sub-second timeouts.

The attached patch (against zsh 5.7.1) modifies the bin_zsystem_flock()
function, allowing the -t option to take floating-point parameters, and
handles the timeout with microsecond granularity.  It also adds option
-p to control the retrying period, which was previously hardcoded to
1 second.

I kept the same function's logic of periodically retrying to acquire
the lock in non-blocking mode.  However, there may be a better way to
implement this timeout: set a timer such as setitimer(2) which sends
a SIGALRM on timeout, trap SIGALRM without SA_RESTART, then acquire the
lock in blocking mode; fcntl() will then block until SIGALRM is caught,
after which errno will be EINTR.  Unfortunately, lacking knowledge of
zsh internals especially signal handling, I'm not confident that I can
reimplement it this way by myself.

The patch modifies Src/Modules/system.c and also Src/utils.c (to add
a function akin to time(NULL) but that returns microseconds, using
clock_gettime(CLOCK_MONOTONIC) if supported, or else gettimeofday()).

I did not modify the documentation yet, not being sure of people being
interested in this feature, nor of whether I should edit just the
Doc/Zsh/mod_system.yo file, or also Doc/{zshmodules.1,zsh.texi} or
others.

Also, I'm afraid that I only did minimal testing.  I checked that
invoking zsystem flock with various values for -t and -p seems to do
what I intended.  Is there a more rigorous test framework for this
kind of thing?

					Thanks, best regards,
					Cedric Ware.

[-- Attachment #2: zsh-5.7.1-flock-timeout.patch --]
[-- Type: text/x-diff, Size: 3047 bytes --]

diff -ur zsh-5.7.1.orig/Src/Modules/system.c zsh-5.7.1/Src/Modules/system.c
--- zsh-5.7.1.orig/Src/Modules/system.c	2018-12-14 08:50:17.000000000 +0100
+++ zsh-5.7.1/Src/Modules/system.c	2019-07-29 20:19:48.835060428 +0200
@@ -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 -ur zsh-5.7.1.orig/Src/utils.c zsh-5.7.1/Src/utils.c
--- zsh-5.7.1.orig/Src/utils.c	2019-02-01 01:37:34.000000000 +0100
+++ zsh-5.7.1/Src/utils.c	2019-07-29 20:24:57.827073902 +0200
@@ -2724,6 +2724,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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Enable sub-second timeout in zsystem flock
  2019-07-29 20:35 [PATCH] Enable sub-second timeout in zsystem flock Cedric Ware
@ 2019-07-29 22:25 ` Bart Schaefer
  0 siblings, 0 replies; 2+ messages in thread
From: Bart Schaefer @ 2019-07-29 22:25 UTC (permalink / raw)
  To: Cedric Ware; +Cc: zsh-workers

On Mon, Jul 29, 2019 at 1:36 PM Cedric Ware
<cedric.ware__bml@normalesup.org> wrote:
>
> The attached patch (against zsh 5.7.1) modifies the bin_zsystem_flock()

In future, please either inline patches or attach them as text/plain
(e.g. with a ".txt" extension instead of ".patch").  Thanks!

> I kept the same function's logic of periodically retrying to acquire
> the lock in non-blocking mode.  However, there may be a better way to
> 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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-29 20:35 [PATCH] Enable sub-second timeout in zsystem flock Cedric Ware
2019-07-29 22:25 ` Bart Schaefer

zsh-workers

Archives are clonable: git clone --mirror http://inbox.vuxu.org/zsh-workers

Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


AGPL code for this site: git clone https://public-inbox.org/ public-inbox