From 460c55279cbe55ba56f7068ace6e7cadbb6d6df1 Mon Sep 17 00:00:00 2001 From: Matthias Andree Date: Tue, 13 Oct 2020 17:21:47 +0200 Subject: pidfile/lockfile handling bugfixes pidfile/locking: log errors thru syslog, too This works by replacing perror()/fprintf(stderr, ...) by report() and strerror(). If the pidfile cannot be unlinked, truncate it. Bump release version to 6.4.13.rc1, and move KNOWN BUGS section up in NEWS. --- NEWS | 26 ++++++++++++++++++++++---- configure.ac | 2 +- fetchmail.c | 7 ++++--- lock.c | 31 +++++++++++++++++++++---------- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 7b9db2e7..727010c1 100644 --- a/NEWS +++ b/NEWS @@ -64,12 +64,22 @@ removed from a 6.5.0 or newer release.) for end-of-life OpenSSL versions may be removed even from patchlevel releases. --------------------------------------------------------------------------------- -fetchmail-6.4.12 (released 2020-09-04, 27596 LoC): +fetchmail-6.4.13 (work in progress): # BUG FIXES: -* The README file is now the one from Git again. The makerelease.pl script - used to roll and upload the tarball sometimes clobbered the README file and - replaced its contents by a part of the NEWS file. +* Errors about lock file (= pidfile) creation could be lost in daemon + configurations (-d option, or set daemon) when using syslog. Now they are also + logged to syslog. Found verifying a pidfile creation issue on 6.4.12 that was + previously reported by Alex Hall of Automatic Distributors. +* If the lock file cannot be removed (no write permission on directory), try + to truncate it, and if that fails, report error. +* If the pidfile was non-default, fetchmail -q or --quit would malfunction and + claim no other fetchmail were running, because it did not read the + configuration files or merge the command line options, thus it would look for + the PID in the wrong file. + +# CHANGES: +* Lockfile (= pidfile) creation errors are now logged with filename and reason. # KNOWN BUGS AND WORKAROUNDS (This section floats upwards through the NEWS file so it stays with the @@ -91,6 +101,14 @@ fetchmail-6.4.12 (released 2020-09-04, 27596 LoC): messages. This will not be fixed, because the maintainer has no Kerberos 5 server to test against. Use GSSAPI. +--------------------------------------------------------------------------------- +fetchmail-6.4.12 (released 2020-09-04, 27596 LoC): + +# BUG FIXES: +* The README file is now the one from Git again. The makerelease.pl script + used to roll and upload the tarball sometimes clobbered the README file and + replaced its contents by a part of the NEWS file. + --------------------------------------------------------------------------------- fetchmail-6.4.11 (released 2020-08-28, 27596 LoC): diff --git a/configure.ac b/configure.ac index a547bee1..3328ba54 100644 --- a/configure.ac +++ b/configure.ac @@ -9,7 +9,7 @@ dnl Process this file with autoconf to produce a configure script. dnl dnl XXX - if bumping version here, check fetchmail.man, too! -AC_INIT([fetchmail],[6.4.12],[fetchmail-users@lists.sourceforge.net]) +AC_INIT([fetchmail],[6.4.13.rc1],[fetchmail-users@lists.sourceforge.net]) AC_CONFIG_SRCDIR([fetchmail.h]) AC_CONFIG_HEADERS([config.h]) AC_CONFIG_LIBOBJ_DIR([.]) diff --git a/fetchmail.c b/fetchmail.c index fb1e6015..3619fd9b 100644 --- a/fetchmail.c +++ b/fetchmail.c @@ -329,9 +329,10 @@ int main(int argc, char **argv) if (system("uname -a")) { /* NOOP to quench GCC complaint */ } } - /* avoid parsing the config file if all we're doing is killing a daemon */ - if (!quitonly) - implicitmode = load_params(argc, argv, optind); + /* We used to avoid parsing the config file if all we're doing is killing + * a daemon, under if (!quitonly) but since the pidfile can be configured + * in the rcfile, this is no longer viable. */ + implicitmode = load_params(argc, argv, optind); if (run.logfile) { /* nodetach -> turn off logfile option */ diff --git a/lock.c b/lock.c index 7f8b8667..9533e25f 100644 --- a/lock.c +++ b/lock.c @@ -60,7 +60,7 @@ static void unlockit(void) /* must-do actions for exit (but we can't count on being able to do malloc) */ { if (lockfile && lock_acquired) - unlink(lockfile); + unlink(lockfile) && truncate(lockfile, (off_t)0); } void fm_lock_dispose(void) @@ -84,7 +84,7 @@ int fm_lock_state(void) bkgd = (args == 2); if (ferror(lockfp)) { - fprintf(stderr, GT_("fetchmail: error reading lockfile \"%s\": %s\n"), + report(stderr, GT_("fetchmail: error reading lockfile \"%s\": %s\n"), lockfile, strerror(errno)); fclose(lockfp); /* not checking should be safe, file mode was "r" */ exit(PS_EXCLUDE); @@ -95,10 +95,13 @@ int fm_lock_state(void) /* ^ could not read PID || process does not exist */ /* => lockfile is stale, unlink it */ pid = 0; - fprintf(stderr,GT_("fetchmail: removing stale lockfile\n")); + report(stderr,GT_("fetchmail: removing stale lockfile \"%s\"\n"), lockfile); if (unlink(lockfile)) { if (errno != ENOENT) { - perror(lockfile); + if (outlevel >= O_VERBOSE) { + report(stderr, GT_("fetchmail: cannot unlink lockfile \"%s\" (%s), trying to write to it\n"), + lockfile, strerror(errno)); + } /* we complain but we don't exit; it might be * writable for us, but in a directory we cannot * write to. This means we can write the new PID to @@ -110,7 +113,8 @@ int fm_lock_state(void) /* but if we cannot truncate the file either, * assume that we cannot write to it later, * complain and quit. */ - perror(lockfile); + report(stderr, GT_("fetchmail: cannot write to lockfile \"%s\" either (%s), exiting\n"), + strerror(errno), lockfile); exit(PS_EXCLUDE); } } @@ -119,7 +123,7 @@ int fm_lock_state(void) } else { pid = 0; if (errno != ENOENT) { - fprintf(stderr, GT_("fetchmail: error opening lockfile \"%s\": %s\n"), + report(stderr, GT_("fetchmail: error opening lockfile \"%s\": %s\n"), lockfile, strerror(errno)); exit(PS_EXCLUDE); } @@ -143,7 +147,11 @@ void fm_lock_or_die(void) if (!lock_acquired) { int e = 0; - if ((fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, 0666)) != -1) { + fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, 0666); + if (fd == -1 && EEXIST == errno) { + fd = open(lockfile, O_WRONLY|O_TRUNC, 0666); + } + if (-1 != fd) { ssize_t wr; snprintf(tmpbuf, sizeof(tmpbuf), "%ld\n", (long)getpid()); @@ -165,8 +173,7 @@ void fm_lock_or_die(void) if (e == 0) { lock_acquired = TRUE; } else { - perror(lockfile); - fprintf(stderr, GT_("fetchmail: lock creation failed.\n")); + report(stderr, GT_("fetchmail: lock creation failed, pidfile \"%s\": %s\n"), lockfile, strerror(errno)); exit(PS_EXCLUDE); } } @@ -175,6 +182,10 @@ void fm_lock_or_die(void) void fm_lock_release(void) /* release a lock on a given host */ { - unlink(lockfile); + if (unlink(lockfile)) { + if (truncate(lockfile, (off_t)0)) { + report(stderr, GT_("fetchmail: cannot remove or truncate pidfile \"%s\": %s\n"), strerror(errno)); + } + } } /* lock.c ends here */ -- cgit v1.2.3