[PATCH] history locking with fcntl

[Vincent Lefevre <vincent@xxxxxxxxxx>]

I've rewritten my patch to lock the history with fcntl, and added
an option. It works well with my settings (INC_APPEND_HISTORY), but
I haven't tested it with other settings. I no longer get the error

zsh: failed to write history file /home/vlefevre/.zhistory: bad file descriptor

-- 
Vincent Lefèvre <vincent@xxxxxxxxxx> - Web: <http://www.vinc17.org/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.org/blog/>
Work: CR INRIA - computer arithmetic / Arenaire project (LIP, ENS-Lyon)

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.58
diff -d -u -r1.58 options.yo
--- Doc/Zsh/options.yo	13 Apr 2008 16:58:42 -0000	1.58
+++ Doc/Zsh/options.yo	15 Apr 2008 00:03:26 -0000
@@ -577,6 +577,11 @@
 events, otherwise this option will behave just like
 tt(HIST_IGNORE_ALL_DUPS) once the history fills up with unique events.
 )
+pindex(HIST_FCNTL_LOCK)
+item(tt(HIST_FCNTL_LOCK))(
+Use fcntl locking when writing out the history file, in order to avoid
+history corruption under NFS.
+)
 pindex(HIST_FIND_NO_DUPS)
 cindex(history, ignoring duplicates in search)
 item(tt(HIST_FIND_NO_DUPS))(
Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.74
diff -d -u -r1.74 hist.c
--- Src/hist.c	14 Apr 2008 13:42:53 -0000	1.74
+++ Src/hist.c	15 Apr 2008 00:03:27 -0000
@@ -2021,6 +2021,20 @@
     else if (!lockhistfile(fn, 1))
 	return;
     if ((in = fopen(unmeta(fn), "r"))) {
+#ifdef HAVE_FCNTL_H
+	if (isset(HISTFCNTLLOCK)) {
+	    struct flock lck;
+
+	    lck.l_type = F_RDLCK;
+	    lck.l_whence = SEEK_SET;
+	    lck.l_start = 0;
+	    lck.l_len = 0;  /* lock the whole file */
+	    if (fcntl(fileno(in), F_SETLKW, &lck) == -1) {
+		fclose(in);
+		return;
+	    }
+	}
+#endif
 	nwordlist = 64;
 	wordlist = (short *)zalloc(nwordlist*sizeof(short));
 	bufsiz = 1024;
@@ -2149,6 +2163,68 @@
     unlockhistfile(fn);
 }
 
+#ifdef HAVE_FCNTL_H
+/**/
+static int
+wlockfile(int fd)
+{
+    struct flock lck;
+    int ctr = 8;
+
+    lck.l_type = F_WRLCK;
+    lck.l_whence = SEEK_SET;
+    lck.l_start = 0;
+    lck.l_len = 0;
+    while (fcntl(fd, F_SETLKW, &lck) == -1) {
+	if (--ctr < 0)
+	    return 1;
+	sleep (1);
+    }
+    return 0;
+}
+#endif
+
+/**/
+static int
+safe_unlink(const char *pathname)
+{
+#ifdef HAVE_FCNTL_H
+    if (isset(HISTFCNTLLOCK)) {
+	int fd = open(pathname, O_WRONLY | O_NOCTTY, 0600);
+	if (fd >= 0) {
+	    int err = wlockfile(fd) || unlink(pathname);
+	    close(fd);
+	    return err;
+	} else {
+	    return errno != ENOENT;
+	}
+    }
+#endif
+    return unlink(pathname) && errno != ENOENT;
+}
+
+/**/
+static int
+safe_rename(const char *oldpath, const char *newpath)
+{
+#ifdef HAVE_FCNTL_H
+    if (isset(HISTFCNTLLOCK)) {
+	int fd = open(newpath, O_CREAT | O_WRONLY | O_NOCTTY, 0600);
+	if (fd < 0) {
+	    return 1;
+	} else if (wlockfile(fd)) {
+	    close(fd);
+	    return 1;
+	} else {
+	    int err = rename(oldpath, newpath);
+	    close(fd);
+	    return err;
+	}
+    }
+#endif
+    return rename(oldpath, newpath);
+}
+
 /**/
 void
 savehistfile(char *fn, int err, int writeflags)
@@ -2158,6 +2234,7 @@
     Histent he;
     zlong xcurhist = curhist - !!(histactive & HA_ACTIVE);
     int extended_history = isset(EXTENDEDHISTORY);
+    int truncate_history = 0;
     int ret;
 
     if (!interact || savehistsiz <= 0 || !hist_ring
@@ -2196,12 +2273,14 @@
 	tmpfile = NULL;
 	out = fd >= 0 ? fdopen(fd, "a") : NULL;
     } else if (!isset(HISTSAVEBYCOPY)) {
-	int fd = open(unmeta(fn), O_CREAT | O_WRONLY | O_TRUNC | O_NOCTTY, 0600);
+	int fd = open(unmeta(fn), O_CREAT | O_WRONLY | O_NOCTTY, 0600);
 	tmpfile = NULL;
 	out = fd >= 0 ? fdopen(fd, "w") : NULL;
+	/* The file should be truncated after its locking. */
+	truncate_history = 1;
     } else {
 	tmpfile = bicat(unmeta(fn), ".new");
-	if (unlink(tmpfile) < 0 && errno != ENOENT)
+	if (safe_unlink(tmpfile))
 	    out = NULL;
 	else {
 	    struct stat sb;
@@ -2239,7 +2318,16 @@
 #endif
 	}
     }
+#ifdef HAVE_FCNTL_H
+    if (out && isset(HISTFCNTLLOCK) && wlockfile(fileno(out))) {
+	zerr("can't lock file (timeout) -- history %s not updated", fn);
+	err = 0; /* Don't report a generic error below. */
+	out = NULL;
+    }
+#endif
     if (out) {
+	if (truncate_history)
+	    ftruncate(fileno(out), 0);
 	ret = 0;
 	for (; he && he->histnum <= xcurhist; he = down_histent(he)) {
 	    if ((writeflags & HFILE_SKIPDUPS && he->node.flags & HIST_DUP)
@@ -2285,16 +2373,19 @@
 		zsfree(lasthist.text);
 		lasthist.text = ztrdup(start);
 	    }
-	}
-	if (fclose(out) < 0 && ret >= 0)
+	} else if (ret >= 0 && fflush(out) < 0) {
 	    ret = -1;
+	}
 	if (ret >= 0) {
 	    if (tmpfile) {
-		if (rename(tmpfile, unmeta(fn)) < 0)
+		/* out has been flushed and the file must be renamed while
+		   being open so that the lock is still valid */
+		if (safe_rename(tmpfile, unmeta(fn)))
 		    zerr("can't rename %s.new to $HISTFILE", fn);
 		free(tmpfile);
 		tmpfile = NULL;
 	    }
+	    fclose(out);
 
 	    if (writeflags & HFILE_SKIPOLD
 		&& !(writeflags & (HFILE_FAST | HFILE_NO_REWRITE))) {
@@ -2314,6 +2405,8 @@
 		pophiststack();
 		histactive = remember_histactive;
 	    }
+	} else {
+	    fclose(out);
 	}
     } else
 	ret = -1;
Index: Src/options.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/options.c,v
retrieving revision 1.40
diff -d -u -r1.40 options.c
--- Src/options.c	13 Apr 2008 16:58:42 -0000	1.40
+++ Src/options.c	15 Apr 2008 00:03:27 -0000
@@ -135,6 +135,7 @@
 {{NULL, "histallowclobber",   0},			 HISTALLOWCLOBBER},
 {{NULL, "histbeep",	      OPT_ALL},			 HISTBEEP},
 {{NULL, "histexpiredupsfirst",0},			 HISTEXPIREDUPSFIRST},
+{{NULL, "histfcntllock",      0},			 HISTFCNTLLOCK},
 {{NULL, "histfindnodups",     0},			 HISTFINDNODUPS},
 {{NULL, "histignorealldups",  0},			 HISTIGNOREALLDUPS},
 {{NULL, "histignoredups",     0},			 HISTIGNOREDUPS},
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.121
diff -d -u -r1.121 zsh.h
--- Src/zsh.h	13 Apr 2008 16:58:42 -0000	1.121
+++ Src/zsh.h	15 Apr 2008 00:03:27 -0000
@@ -1749,6 +1749,7 @@
     HISTALLOWCLOBBER,
     HISTBEEP,
     HISTEXPIREDUPSFIRST,
+    HISTFCNTLLOCK,
     HISTFINDNODUPS,
     HISTIGNOREALLDUPS,
     HISTIGNOREDUPS,