Re: History corruption (over NFS)

[Seth Kurtzberg <seth@cql.com>]

Wayne Davison wrote:

>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?
>  
>
Won't this cause problems in the mode where history is shared, and 
history is written for every command, not just once when the shell 
terminates?

>I've attached a diff in case you want to try it out.
>
>..wayne..
>
>
>!DSPAM:4210f8a3201492089188992!
>  
>
>------------------------------------------------------------------------
>
>--- 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);
> }
>  
>