From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23055 invoked from network); 15 Apr 2008 15:31:46 -0000 X-Spam-Checker-Version: SpamAssassin 3.2.4 (2008-01-01) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.2.4 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 15 Apr 2008 15:31:46 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 1003 invoked from network); 15 Apr 2008 15:31:40 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 15 Apr 2008 15:31:40 -0000 Received: (qmail 15050 invoked by alias); 15 Apr 2008 15:31:36 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 24818 Received: (qmail 15031 invoked from network); 15 Apr 2008 15:31:36 -0000 Received: from bifrost.dotsrc.org (130.225.254.106) by sunsite.dk with SMTP; 15 Apr 2008 15:31:36 -0000 Received: from prunille.vinc17.org (vinc17.pck.nerim.net [213.41.242.187]) by bifrost.dotsrc.org (Postfix) with ESMTP id 06727808A37A for ; Tue, 15 Apr 2008 17:31:21 +0200 (CEST) Received: by prunille.vinc17.org (Postfix, from userid 501) id DFD852198C10; Tue, 15 Apr 2008 17:31:20 +0200 (CEST) Date: Tue, 15 Apr 2008 17:31:20 +0200 From: Vincent Lefevre To: zsh-workers@sunsite.dk Subject: [PATCH] history locking with fcntl Message-ID: <20080415153120.GE1223@prunille.vinc17.org> Mail-Followup-To: zsh-workers@sunsite.dk MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="BXVAT5kNtrzKuDFl" Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Mailer-Info: http://www.vinc17.org/mutt/ User-Agent: Mutt/1.5.17-vl-r21552 (2008-03-11) X-Virus-Scanned: ClamAV 0.91.2/6781/Tue Apr 15 15:41:50 2008 on bifrost X-Virus-Status: Clean --BXVAT5kNtrzKuDFl Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 - Web: 100% accessible validated (X)HTML - Blog: Work: CR INRIA - computer arithmetic / Arenaire project (LIP, ENS-Lyon) --BXVAT5kNtrzKuDFl Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="zsh-histlock.patch" 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, --BXVAT5kNtrzKuDFl--