tech-userlevel archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
pidfile_lock(3)
pidfile(3) is pretty crap - it just writes to the file without any locking.
Also, you can only write the pidfile once - subsequent calls to the same file
are a non-op so it's useless after forking.
pidlock(3) is a bit better, but the "lock" is just does the file exist or not
which isn't great if the process has died - there is a chance that another
process with the same pid could spawn later. Also, writing misc data to the
pidfile after the pid is pretty non standard. It's also not mentioned in
pidfile(3) and the only user of it in our tree is getty(8).
So I've created pidfile_lock (patch attached) to address these problems.
It returns 0 on success, otherwise the pid of the process who claims the lock
and if we cannot read that then -1. This makes the error checking similar to
fork(2).
Here is an example of it's use:
pid_t pid;
if ((pid = pidfile_lock(NULL)) != 0) {
if (pid == -1)
syslog(LOG_ERR, "%s: pidfile_lock: %m", __func__);
else
syslog(LOG_ERR, "%s already started on pid %jd",
getprogname(), (intmax_t)pid);
exit(EXIT_FAILURE);
}
syslog(LOG_INFO, "%s starting", getprogname());
if (daemon(0, 0) == -1) {
syslog(LOG_ERR, "daemon: %m");
exit(EXIT_FAILURE);
}
if ((pid = pidfile_lock(NULL)) != 0) {
syslog(LOG_ERR, "%s: pidfile_lock (%jd): %m", __func__,
(intmax_t)pid);
exit(EXIT_FAILURE);
}
pidfile(3) now just calls pidfile_lock(3), but still, returns 0 on success or -1
on error. So there is no way of knowing which process has the lock. But at
least it now does lock the file and respect any other locks so it's still a
massive improvement.
Comments? If none, I'll also adjust the man page and commit during next week.
RoyIndex: include/util.h
===================================================================
RCS file: /cvsroot/src/include/util.h,v
retrieving revision 1.68
diff -u -p -r1.68 util.h
--- include/util.h 24 Sep 2015 14:39:37 -0000 1.68
+++ include/util.h 19 Mar 2016 22:54:15 -0000
@@ -103,6 +103,7 @@ time_t parsedate(const char *, const ti
__RENAME(__parsedate50);
#endif
int pidfile(const char *);
+pid_t pidfile_lock(const char *);
int pidlock(const char *, int, pid_t *, const char *);
int pw_abort(void);
#ifndef __LIBC12_SOURCE__
Index: lib/libutil/pidfile.c
===================================================================
RCS file: /cvsroot/src/lib/libutil/pidfile.c,v
retrieving revision 1.11
diff -u -p -r1.11 pidfile.c
--- lib/libutil/pidfile.c 22 Jan 2015 19:04:28 -0000 1.11
+++ lib/libutil/pidfile.c 19 Mar 2016 22:54:15 -0000
@@ -35,7 +35,11 @@ __RCSID("$NetBSD: pidfile.c,v 1.11 2015/
#endif
#include <sys/param.h>
+#include <sys/proc.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
#include <paths.h>
#include <stdbool.h>
#include <stdlib.h>
@@ -46,14 +50,22 @@ __RCSID("$NetBSD: pidfile.c,v 1.11 2015/
static pid_t pidfile_pid;
static char *pidfile_path;
+static int pidfile_fd = -1;
-/* Deletes an existent pidfile iff it was created by this process. */
+/* Deletes an existent pidfile if it was created by this process. */
static void
pidfile_cleanup(void)
{
- if ((pidfile_path != NULL) && (pidfile_pid == getpid()))
+ if (pidfile_path != NULL && pidfile_pid == getpid()) {
+ (void) close(pidfile_fd);
(void) unlink(pidfile_path);
+ }
+
+ pidfile_pid = 0;
+ free(pidfile_path);
+ pidfile_path = NULL;
+ pidfile_fd = -1;
}
/* Registers an atexit(3) handler to delete the pidfile we have generated.
@@ -75,29 +87,6 @@ register_atexit_handler(void)
return 0;
}
-/* Given a new pidfile name in 'path', deletes any previously-created pidfile
- * if the previous file differs to the new one.
- *
- * If a previous file is deleted, returns 1, which means that a new pidfile
- * must be created. Otherwise, this returns 0, which means that the existing
- * file does not need to be touched. */
-static int
-cleanup_old_pidfile(const char* path)
-{
- if (pidfile_path != NULL) {
- if (strcmp(pidfile_path, path) != 0) {
- pidfile_cleanup();
-
- free(pidfile_path);
- pidfile_path = NULL;
-
- return 1;
- } else
- return 0;
- } else
- return 1;
-}
-
/* Constructs a name for a pidfile in the default location (/var/run). If
* 'bname' is NULL, uses the name of the current program for the name of
* the pidfile.
@@ -118,58 +107,115 @@ generate_varrun_path(const char *bname)
return path;
}
-/* Creates a pidfile with the provided name. The new pidfile is "registered"
- * in the global variables pidfile_path and pidfile_pid so that any further
- * call to pidfile(3) can check if we are recreating the same file or a new
- * one.
+/* Read the contents of path and ensure it matches our notion of a pid.
*
- * Returns 0 on success or -1 if there is any error. */
-static int
-create_pidfile(const char* path)
+ * Returns the pid inside the file on success, otherwise -1. */
+static pid_t
+pidfile_read(const char *path)
+{
+ char buf[16], *eptr;
+ int fd, error, n, e;
+ pid_t pid;
+
+ if ((fd = open(path, O_RDONLY)) == -1)
+ return -1;
+
+ n = read(fd, buf, sizeof(buf) - 1);
+ error = errno;
+ (void) close(fd);
+ if (n == -1) {
+ errno = error;
+ return -1;
+ }
+ buf[n] = '\0';
+ pid = (pid_t)strtoi(buf, &eptr, 10, 1, INT_MAX, &e);
+ if (e && !(e == ENOTSUP && *eptr == '\n')) {
+ errno = e;
+ return -1;
+ }
+ return pid;
+}
+
+/* Locks the pidfile specified by path and writes the process pid to it.
+ *
+ * Returns 0 on success, otherwise the pid of the process who owns the
+ * lock if it can be read, otherwise -1. */
+pid_t
+pidfile_lock(const char *path)
{
- FILE *f;
+ char *default_path;
if (register_atexit_handler() == -1)
return -1;
- if (cleanup_old_pidfile(path) == 0)
- return 0;
-
- pidfile_path = strdup(path);
- if (pidfile_path == NULL)
- return -1;
+ if (path == NULL || strchr(path, '/') == NULL) {
+ if ((default_path = generate_varrun_path(path)) == NULL)
+ return -1;
+ path = default_path;
+ } else
+ default_path = NULL;
- if ((f = fopen(path, "w")) == NULL) {
- free(pidfile_path);
- pidfile_path = NULL;
- return -1;
+ /* If path has changed (no good reason), clean up the old pidfile. */
+ if (pidfile_path != NULL && strcmp(pidfile_path, path) != 0)
+ pidfile_cleanup();
+
+ if (pidfile_fd == -1) {
+ pid_t pid = -1;
+
+ pidfile_fd = open(path,
+ O_WRONLY | O_CREAT | O_CLOEXEC | O_NONBLOCK | O_EXLOCK,
+ 0666);
+ if (pidfile_fd == -1) {
+ if (errno == EAGAIN) {
+ pid = pidfile_read(path);
+ if (errno == EAGAIN)
+ errno = EEXIST;
+ }
+ } else if ((pidfile_path = strdup(path)) == NULL) {
+ int error = errno;
+
+ (void) close(pidfile_fd);
+ (void) unlink(path);
+ pidfile_fd = -1;
+ errno = error;
+ }
+ if (pidfile_fd == -1) {
+ free(default_path);
+ return pid;
+ }
}
pidfile_pid = getpid();
+ free(default_path);
- (void) fprintf(f, "%d\n", pidfile_pid);
- (void) fclose(f);
+ /* Truncate the file, as we could be re-writing it after forking.
+ * Then write the pidfile. */
+ if (ftruncate(pidfile_fd, 0) == -1 ||
+ lseek(pidfile_fd, 0, SEEK_SET) == -1 ||
+ dprintf(pidfile_fd, "%jd\n", (intmax_t)pidfile_pid) == -1)
+ {
+ int error = errno;
+ pidfile_cleanup();
+ errno = error;
+ return -1;
+ }
+
+ /* Hold the fd open to persist the lock. */
return 0;
}
+/* The old function.
+ * Historical behaviour is that pidfile is not re-written
+ * if path has not changed.
+ *
+ * Returns 0 on success, otherwise -1.
+ * As such we have no way of knowing the pid who owns the lock. */
int
pidfile(const char *path)
{
+ pid_t pid;
- if (path == NULL || strchr(path, '/') == NULL) {
- char *default_path;
-
- if ((default_path = generate_varrun_path(path)) == NULL)
- return -1;
-
- if (create_pidfile(default_path) == -1) {
- free(default_path);
- return -1;
- }
-
- free(default_path);
- return 0;
- } else
- return create_pidfile(path);
+ pid = pidfile_lock(path);
+ return pid == 0 ? 0 : -1;
}
Home |
Main Index |
Thread Index |
Old Index