From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23739 invoked from network); 17 Apr 2008 16:22:53 -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; 17 Apr 2008 16:22:53 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 39119 invoked from network); 17 Apr 2008 16:22:51 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 17 Apr 2008 16:22:51 -0000 Received: (qmail 5683 invoked by alias); 17 Apr 2008 16:22:49 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 24831 Received: (qmail 5668 invoked from network); 17 Apr 2008 16:22:48 -0000 Received: from bifrost.dotsrc.org (130.225.254.106) by sunsite.dk with SMTP; 17 Apr 2008 16:22:48 -0000 Received: from dot.blorf.net (dsl-74-220-69-132.cruzio.com [74.220.69.132]) by bifrost.dotsrc.org (Postfix) with ESMTP id B2DA68043AC7 for ; Thu, 17 Apr 2008 18:22:44 +0200 (CEST) Received: by dot.blorf.net (Postfix, from userid 1000) id 4A99C1733F; Thu, 17 Apr 2008 09:23:07 -0700 (PDT) Date: Thu, 17 Apr 2008 09:23:07 -0700 From: Wayne Davison To: zsh-workers@sunsite.dk Subject: Re: [PATCH] history locking with fcntl Message-ID: <20080417162307.GB22594@blorf.net> References: <20080415153120.GE1223@prunille.vinc17.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="EVF5PPMfhYS0aIcm" Content-Disposition: inline In-Reply-To: <20080415153120.GE1223@prunille.vinc17.org> User-Agent: Mutt/1.5.15+20070412 (2007-04-11) X-Virus-Scanned: ClamAV 0.91.2/6810/Thu Apr 17 13:25:25 2008 on bifrost X-Virus-Status: Clean --EVF5PPMfhYS0aIcm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Apr 15, 2008 at 05:31:20PM +0200, Vincent Lefevre wrote: > I no longer get the error > > zsh: failed to write history file /home/vlefevre/.zhistory: bad file descriptor I fixed the bogus "bad file descriptor" part of that error back on March 6th, as well as making some minor improvements to the errors. I'd like to see what the real error is when your flock patch is not present. I personally think this patch goes too far. There is a lockhistfile() call that is done to give a particular shell exclusive control over the history files. I'd prefer to see nay extra locking done there. Attached is an alternative patch for the Src/utils.c file that implements this (it is easier to see what I've done than diffing it against the file with the current fcntl changes). Let me know what you think. Additional discussion: I think the option name should be made more generic so that it can be made to support more types of file locking, such as the flock() call. Perhaps call it HIST_EXTRA_LOCKING? ..wayne.. --EVF5PPMfhYS0aIcm Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="fcntl.patch" --- Src/hist.c 14 Apr 2008 13:42:53 -0000 1.74 +++ Src/hist.c 17 Apr 2008 16:12:04 -0000 @@ -2330,6 +2330,36 @@ savehistfile(char *fn, int err, int writ unlockhistfile(fn); } +#ifdef HAVE_FCNTL_H +static int flock_fd = -1; + +static int +flockhistfile(char *fn, int keep_trying) +{ + struct flock lck; + int ctr = keep_trying ? 9 : 0; + + if ((flock_fd = open(unmeta(fn), O_RDWR | O_NOCTTY)) < 0) + return errno == ENOENT; /* "successfully" locked missing file */ + + lck.l_type = F_WRLCK; + lck.l_whence = SEEK_SET; + lck.l_start = 0; + lck.l_len = 0; /* lock the whole file */ + + while (fcntl(flock_fd, F_SETLKW, &lck) == -1) { + if (--ctr < 0) { + close(flock_fd); + flock_fd = -1; + return 0; + } + sleep(1); + } + + return 1; +} +#endif + static int lockhistct; /**/ @@ -2340,6 +2370,12 @@ lockhistfile(char *fn, int keep_trying) if (!fn && !(fn = getsparam("HISTFILE"))) return 0; + +#ifdef HAVE_FCNTL_H + if (isset(HISTFCNTLLOCK) && !lockhistct && !flockhistfile(fn, keep_trying)) + return 0; +#endif + if (!lockhistct++) { struct stat sb; int fd; @@ -2404,7 +2440,15 @@ lockhistfile(char *fn, int keep_trying) #endif /* not HAVE_LINK */ free(lockfile); } - return ct != lockhistct; + + if (ct == lockhistct) { +#ifdef HAVE_FCNTL_H + close(flock_fd); + flock_fd = -1; +#endif + return 0; + } + return 1; } /* Unlock the history file if this corresponds to the last nested lock @@ -2428,6 +2472,12 @@ unlockhistfile(char *fn) sprintf(lockfile, "%s.LOCK", fn); unlink(lockfile); free(lockfile); +#ifdef HAVE_FCNTL_H + if (flock_fd >= 0) { + close(flock_fd); + flock_fd = -1; + } +#endif } } --EVF5PPMfhYS0aIcm--