From: Volker Lendecke <vl@samba.org>
Date: Mon, 15 Feb 2021 10:38:18 +0000 (+0100)
Subject: lib: Make pidfile_path_create() return the existing PID on conflict
X-Git-Tag: tevent-0.11.0~1540
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cf43f331be818c6253bb45fcfa89327a3c858188;p=thirdparty%2Fsamba.git

lib: Make pidfile_path_create() return the existing PID on conflict

Use F_GETLK to get the lock holder PID, this is more accurate than
reading the file contents: A conflicting process might not have
written its PID yet. Also, F_GETLK easily allows to do a retry if the
lock holder just died.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---

diff --git a/ctdb/common/pidfile.c b/ctdb/common/pidfile.c
index e78542d5f2f..47589f4b083 100644
--- a/ctdb/common/pidfile.c
+++ b/ctdb/common/pidfile.c
@@ -54,7 +54,7 @@ int pidfile_context_create(TALLOC_CTX *mem_ctx, const char *pidfile,
 
 	pid_ctx->pid = getpid();
 
-	ret = pidfile_path_create(pid_ctx->pidfile, &fd);
+	ret = pidfile_path_create(pid_ctx->pidfile, &fd, NULL);
 	if (ret != 0) {
 		return ret;
 	}
diff --git a/lib/util/pidfile.c b/lib/util/pidfile.c
index b7daa089223..4f4e5e9782c 100644
--- a/lib/util/pidfile.c
+++ b/lib/util/pidfile.c
@@ -27,16 +27,14 @@
 
 #include "lib/util/pidfile.h"
 
-int pidfile_path_create(const char *path, int *outfd)
+int pidfile_path_create(const char *path, int *pfd, pid_t *existing_pid)
 {
 	struct flock lck;
 	char tmp[64] = { 0 };
-	pid_t pid;
 	int fd, ret = 0;
 	int len;
 	ssize_t nwritten;
-
-	pid = getpid();
+	bool retried = false;
 
 	fd = open(path, O_CREAT|O_WRONLY|O_NONBLOCK, 0644);
 	if (fd == -1) {
@@ -44,10 +42,11 @@ int pidfile_path_create(const char *path, int *outfd)
 	}
 
 	if (! set_close_on_exec(fd)) {
-		close(fd);
-		return EIO;
+		ret = errno;
+		goto fail;
 	}
 
+retry:
 	lck = (struct flock) {
 		.l_type = F_WRLCK,
 		.l_whence = SEEK_SET,
@@ -59,25 +58,41 @@ int pidfile_path_create(const char *path, int *outfd)
 
 	if (ret != 0) {
 		ret = errno;
-		close(fd);
-		return ret;
+
+		if ((ret == EACCES) || (ret == EAGAIN)) {
+			do {
+				ret = fcntl(fd, F_GETLK, &lck);
+			} while ((ret == -1) && (errno == EINTR));
+
+			if (ret == -1) {
+				ret = errno;
+				goto fail;
+			}
+
+			if (lck.l_type == F_UNLCK) {
+				if (!retried) {
+					/* Lock holder died, retry once */
+					retried = true;
+					goto retry;
+				}
+				/* Something badly wrong */
+				ret = EIO;
+				goto fail;
+			}
+
+			if (existing_pid != NULL) {
+				*existing_pid = lck.l_pid;
+			}
+			return EAGAIN;
+		}
+		goto fail;
 	}
 
 	/*
 	 * PID file is locked by us so from here on we should unlink
 	 * on failure
 	 */
-
-	do {
-		ret = ftruncate(fd, 0);
-	} while ((ret == -1) && (errno == EINTR));
-
-	if (ret == -1) {
-		ret = EIO;
-		goto fail_unlink;
-	}
-
-	len = snprintf(tmp, sizeof(tmp), "%u\n", pid);
+	len = snprintf(tmp, sizeof(tmp), "%u\n", getpid());
 	if (len < 0) {
 		ret = errno;
 		goto fail_unlink;
@@ -92,17 +107,25 @@ int pidfile_path_create(const char *path, int *outfd)
 	} while ((nwritten == -1) && (errno == EINTR));
 
 	if ((nwritten == -1) || (nwritten != len)) {
-		ret = EIO;
+		ret = errno;
 		goto fail_unlink;
 	}
 
-	if (outfd != NULL) {
-		*outfd = fd;
+	do {
+		ret = ftruncate(fd, len);
+	} while ((ret == -1) && (errno == EINTR));
+
+	if (ret == -1) {
+		ret = errno;
+		goto fail_unlink;
 	}
+
+	*pfd = fd;
 	return 0;
 
 fail_unlink:
 	unlink(path);
+fail:
 	close(fd);
 	return ret;
 }
@@ -185,24 +208,17 @@ void pidfile_create(const char *piddir, const char *name)
 	size_t len = strlen(piddir) + strlen(name) + 6;
 	char pidFile[len];
 	pid_t pid;
-	int ret;
+	int ret, fd;
 
 	snprintf(pidFile, sizeof(pidFile), "%s/%s.pid", piddir, name);
 
-	pid = pidfile_pid(piddir, name);
-	if (pid != 0) {
+	ret = pidfile_path_create(pidFile, &fd, &pid);
+	if (ret == EAGAIN) {
 		DEBUG(0,("ERROR: %s is already running. File %s exists and process id %d is running.\n",
 			 name, pidFile, (int)pid));
 		exit(1);
 	}
 
-	ret = pidfile_path_create(pidFile, NULL);
-	if (ret != 0) {
-		DBG_ERR("ERROR: Failed to create PID file %s (%s)\n",
-			pidFile, strerror(ret));
-		exit(1);
-	}
-
 	/* Leave pid file open & locked for the duration... */
 }
 
diff --git a/lib/util/pidfile.h b/lib/util/pidfile.h
index b1b9f549339..6f345a9d95d 100644
--- a/lib/util/pidfile.h
+++ b/lib/util/pidfile.h
@@ -29,15 +29,17 @@
 /**
  * @brief Create a PID file
  *
- * Opens file, locks it, and writes PID.  Returns EACCES or EAGAIN if
- * another process has the PID file locked.  Use unlink(2) and
+ * Opens file, locks it, and writes PID.  Returns EAGAIN if another
+ * process has the PID file locked.  Use unlink(2) and
  * pidfile_fd_close() to remove the PID file.
  *
  * @param[in] path PID file name
  * @param[out] outfd File descriptor of open/locked PID file
+ * @param[out] existing_pid Return existing PID on EAGAIN
  * @return 0 on success, errno on failure
  */
-int pidfile_path_create(const char *path, int *outfd);
+int pidfile_path_create(
+	const char *path, int *outfd, pid_t *existing_pid);
 
 /**
  * @brief Unlock and close a PID file