Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: History file lock contention
On Fri, 23 Jan 2015 12:13:09 +0000
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> On 23 January 2015 at 11:46, <ondra@xxxxxxxxxxxxx> wrote:
> > when running zsh from a terminal multiplexer like tmux, it is possible
> > (and not uncommon) to close several processes at the exact same moment.
> > In that case, all of them try to sync their history and all but one fail
> > to acquire the lock. So far so good.
> >
> > However, all of these call nanosleep({1,0}), and since we might be on a
> > multicore system, they might just be scheduled at the same time again,
> > only one of them will progress and the scenario repeats. What the user
> > sees is the terminals not disappearing immediately, but one after the
> > other, a second per process.
>
> A better scheme would be to start with a small wait time, says 0.1 seconds,
> with a random back off, with a maximum length of say the same time again,
> and then double the base time each time we back off, but keeping the
> same maximum time of 10 seconds since we first tried before reporting
> a failure. We can then tweak this as necessary. Or maybe just
> having a random backoff with doubling maximum and no fixed
> base time is good enough for typical problems.
The following doesn't appear to be too fundamentally broken, however I
don't have a set-up that's particularly likely to trigger problems. It
would be good if other people who think they do could try it out before
I commit it --- although I don't think it's too far off from being
usable by bleeding edge types.
pws
P.S. Looks like I forgot to "remove formatting" from webmail last time.
Back with proper mail.
diff --git a/Src/hist.c b/Src/hist.c
index 11d9722..05dcdb6 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2610,7 +2610,8 @@ static int
flockhistfile(char *fn, int keep_trying)
{
struct flock lck;
- int ctr = keep_trying ? 9 : 0;
+ long sleep_us = 0x4000000; /* about 67 ms */
+ time_t end_time;
if (flock_fd >= 0)
return 0; /* already locked */
@@ -2623,13 +2624,22 @@ flockhistfile(char *fn, int keep_trying)
lck.l_start = 0;
lck.l_len = 0; /* lock the whole file */
+ /*
+ * Timeout is ten seconds.
+ */
+ end_time = time(NULL) + (time_t)10;
while (fcntl(flock_fd, F_SETLKW, &lck) == -1) {
- if (--ctr < 0) {
+ if (!keep_trying || time(NULL) >= end_time ||
+ /*
+ * Randomise wait to minimise clashes with shells exiting at
+ * the same time.
+ */
+ !zsleep_random(sleep_us, end_time)) {
close(flock_fd);
flock_fd = -1;
return 1;
}
- sleep(1);
+ sleep_us <<= 1;
}
return 0;
@@ -2865,7 +2875,7 @@ savehistfile(char *fn, int err, int writeflags)
static int lockhistct;
static int
-checklocktime(char *lockfile, time_t then)
+checklocktime(char *lockfile, long *sleep_usp, time_t then)
{
time_t now = time(NULL);
@@ -2875,9 +2885,19 @@ checklocktime(char *lockfile, time_t then)
return -1;
}
- if (now - then < 10)
- sleep(1);
- else
+ if (now - then < 10) {
+ /*
+ * To give the effect of a gradually increasing backoff,
+ * we'll sleep a period based on the time we've spent so far.
+ */
+ DPUTS(now < then, "time flowing backwards through history");
+ /*
+ * Randomise to minimise clashes with shells exiting at the same
+ * time.
+ */
+ (void)zsleep_random(*sleep_usp, then + 10);
+ *sleep_usp <<= 1;
+ } else
unlink(lockfile);
return 0;
@@ -2894,6 +2914,7 @@ lockhistfile(char *fn, int keep_trying)
{
int ct = lockhistct;
int ret = 0;
+ long sleep_us = 0x4000000; /* about 67 ms */
if (!fn && !(fn = getsparam("HISTFILE")))
return 1;
@@ -2937,7 +2958,7 @@ lockhistfile(char *fn, int keep_trying)
continue;
break;
}
- if (checklocktime(lockfile, sb.st_mtime) < 0) {
+ if (checklocktime(lockfile, &sleep_us, sb.st_mtime) < 0) {
ret = 1;
break;
}
@@ -2965,7 +2986,7 @@ lockhistfile(char *fn, int keep_trying)
continue;
ret = 2;
} else {
- if (checklocktime(lockfile, sb.st_mtime) < 0) {
+ if (checklocktime(lockfile, &sleep_us, sb.st_mtime) < 0) {
ret = 1;
break;
}
@@ -2993,7 +3014,7 @@ lockhistfile(char *fn, int keep_trying)
ret = 2;
break;
}
- if (checklocktime(lockfile, sb.st_mtime) < 0) {
+ if (checklocktime(lockfile, &sleep_us, sb.st_mtime) < 0) {
ret = 1;
break;
}
diff --git a/Src/utils.c b/Src/utils.c
index cf18f12..6ee2ec6 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -2261,6 +2261,10 @@ setblock_stdin(void)
* Note that apart from setting (and restoring) non-blocking input,
* this function does not change the input mode. The calling function
* should have set cbreak mode if necessary.
+ *
+ * fd may be -1 to sleep until the timeout in microseconds. This is a
+ * fallback for old systems that don't have nanosleep(). Some very old
+ * systems might not have select: get with it, daddy-o.
*/
/**/
@@ -2282,6 +2286,8 @@ read_poll(int fd, int *readchar, int polltty, zlong microseconds)
struct ttyinfo ti;
#endif
+ if (fd < 0)
+ polltty = 0; /* no tty to poll */
#if defined(HAS_TIO) && !defined(__CYGWIN__)
/*
@@ -2303,7 +2309,7 @@ read_poll(int fd, int *readchar, int polltty, zlong microseconds)
* as plausible as it sounds, but it seems the right way to guess.
* pws 2000/06/26
*/
- if (polltty) {
+ if (fd >= 0) {
gettyinfo(&ti);
if ((polltty = ti.tio.c_cc[VMIN])) {
ti.tio.c_cc[VMIN] = 0;
@@ -2319,16 +2325,24 @@ read_poll(int fd, int *readchar, int polltty, zlong microseconds)
expire_tv.tv_sec = (int) (microseconds / (zlong)1000000);
expire_tv.tv_usec = microseconds % (zlong)1000000;
FD_ZERO(&foofd);
- FD_SET(fd, &foofd);
- ret = select(fd+1, (SELECT_ARG_2_T) &foofd, NULL, NULL, &expire_tv);
+ if (fd > -1) {
+ FD_SET(fd, &foofd);
+ ret = select(fd+1, (SELECT_ARG_2_T) &foofd, NULL, NULL, &expire_tv);
+ } else
+ ret = select(0, NULL, NULL, NULL, &expire_tv);
#else
+ if (fd < 0) {
+ /* OK, can't do that. Just quietly sleep for a second. */
+ sleep(1);
+ return 1;
+ }
#ifdef FIONREAD
if (ioctl(fd, FIONREAD, (char *) &val) == 0)
ret = (val > 0);
#endif
#endif
- if (ret < 0) {
+ if (fd >= 0 && ret < 0) {
/*
* Final attempt: set non-blocking read and try to read a character.
* Praise Bill, this works under Cygwin (nothing else seems to).
@@ -2350,6 +2364,80 @@ read_poll(int fd, int *readchar, int polltty, zlong microseconds)
return (ret > 0);
}
+/*
+ * 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.
+ */
+
+/**/
+int
+zsleep(long us)
+{
+#ifdef HAVE_NANOSLEEP
+ struct timespec sleeptime;
+
+ sleeptime.tv_sec = (time_t)us / (time_t)1000000;
+ sleeptime.tv_nsec = (us % 1000000L) * 1000L;
+ for (;;) {
+ struct timespec rem;
+ int ret = nanosleep(&sleeptime, &rem);
+
+ if (ret == 0)
+ return 1;
+ else if (errno != EINTR)
+ return 0;
+ sleeptime = rem;
+ }
+#else
+ int dummy;
+ return read_poll(-1, &dummy, 0, us);
+#endif
+}
+
+/**
+ * Sleep for time (fairly) randomly up to max_us microseconds.
+ * Don't let the wallclock time extend beyond end_time.
+ * Return 1 if that seemed to work, else 0.
+ *
+ * For best results max_us should be a multiple of 2**16 or large
+ * enough that it doesn't matter.
+ */
+
+/**/
+int
+zsleep_random(long max_us, time_t end_time)
+{
+ int r;
+ time_t now = time(NULL);
+
+ /*
+ * Randomish backoff. Doesn't need to be fundamentally
+ * unpredictable, just probably unlike the value another
+ * exiting shell is using. On some systems the bottom 16
+ * bits aren't that random but the use here doesn't
+ * really care.
+ */
+ r = (long)(rand() & 0xFFFF);
+ /*
+ * Turn this into a fraction of sleep_us. Again, this
+ * doesn't need to be particularly accurate and the base time
+ * is sufficient that we can do the division first and not
+ * worry about the range.
+ */
+ r = (max_us >> 16) * r;
+ /*
+ * Don't sleep beyond timeout.
+ * Not that important as timeout is ridiculously long, but
+ * if there's an interface, interface to it...
+ */
+ while (now + r > end_time)
+ r >>= 1;
+ if (r) /* pedantry */
+ return zsleep(r);
+ return 0;
+}
+
/**/
int
checkrmall(char *s)
diff --git a/configure.ac b/configure.ac
index 8ea9737..bfc02b2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1299,7 +1299,8 @@ AC_CHECK_FUNCS(strftime strptime mktime timelocal \
gdbm_open getxattr \
realpath canonicalize_file_name \
symlink getcwd \
- cygwin_conv_path)
+ cygwin_conv_path \
+ nanosleep)
AC_FUNC_STRCOLL
AH_TEMPLATE([REALPATH_ACCEPTS_NULL],
Messages sorted by:
Reverse Date,
Date,
Thread,
Author