Re: History corruption (over NFS)

[Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx>]

On Mon, Feb 14, 2005 at 04:56:30PM +0100, Vincent Lefevre wrote:
> Then the current zsh instances went on writing to the history file
> without noticing that it had been truncated, hence the null bytes.

I think this is an NFS problem, since the zsh instances do not keep the
history file open -- they just open the file for appending, and add an
item to the end.  When the file is rewritten, it is opened for writing
with O_TRUNC, and the new contents output.

A little while ago I wrote a patch that caused zsh to change its
rewriting strategy to use a HISTORY_FILE.new file and then rename it
into place.  The purpose of this patch is to avoid losing any history if
zsh gets interrupted during the writing of the new history file.
However, it should also be a friendlier update strategy for NFS dirs
because the inode of the file changes.  The downside is that it can undo
some user's use of symlinks, hardlinks, or special group permissions on
their history file (because it is replacing the history file instead of
rewriting it in-place).  Perhaps this algorithm should be made optional?

I've attached a diff in case you want to try it out.

..wayne..

--- orig/hist.c	2005-01-28 02:16:20 -0800
+++ hist.c	2004-10-18 15:21:54 -0700
@@ -2004,7 +2004,7 @@
 void
 savehistfile(char *fn, int err, int writeflags)
 {
-    char *t, *start = NULL;
+    char *t, *tmpfile, *start = NULL;
     FILE *out;
     Histent he;
     zlong xcurhist = curhist - !!(histactive & HA_ACTIVE);
@@ -2041,12 +2041,14 @@
 	    extended_history = 1;
     }
     if (writeflags & HFILE_APPEND) {
+	tmpfile = NULL;
 	out = fdopen(open(unmeta(fn),
 			O_CREAT | O_WRONLY | O_APPEND | O_NOCTTY, 0600), "a");
     }
     else {
-	out = fdopen(open(unmeta(fn),
-			 O_CREAT | O_WRONLY | O_TRUNC | O_NOCTTY, 0600), "w");
+	tmpfile = bicat(unmeta(fn), ".new");
+	unlink(tmpfile);
+	out = fdopen(open(tmpfile, O_WRONLY | O_CREAT | O_EXCL, 0600), "w");
     }
     if (out) {
 	for (; he && he->histnum <= xcurhist; he = down_histent(he)) {
@@ -2091,6 +2093,11 @@
 	    lasthist.text = ztrdup(start);
 	}
 	fclose(out);
+	if (tmpfile) {
+	    if (rename(tmpfile, unmeta(fn)) < 0)
+		zerr("can't rename %s.new to $HISTFILE", fn, 0);
+	    free(tmpfile);
+	}
 
 	if (writeflags & HFILE_SKIPOLD
 	 && !(writeflags & (HFILE_FAST | HFILE_NO_REWRITE))) {
@@ -2110,8 +2117,13 @@
 	    pophiststack();
 	    histactive = remember_histactive;
 	}
-    } else if (err)
-	zerr("can't write history file %s", fn, 0);
+    } else if (err) {
+	if (tmpfile) {
+	    zerr("can't write history file %s.new", fn, 0);
+	    free(tmpfile);
+	} else
+	    zerr("can't write history file %s", fn, 0);
+    }
 
     unlockhistfile(fn);
 }