wait for exclusive fds properly

src/openrc-run/meson.build

@@ -1,5 +1,5 @@
 executable('openrc-run',
-  ['openrc-run.c', misc_c, plugin_c, rc_exec_c, selinux_c, usage_c, version_h],
+  ['openrc-run.c', misc_c, timeutils_c, plugin_c, rc_exec_c, selinux_c, usage_c, version_h],
   c_args : [cc_audit_flags, cc_branding_flags, cc_pam_flags, cc_selinux_flags],
   link_with: [libeinfo, librc],
   dependencies: [audit_dep, dl_dep, pam_dep, pam_misc_dep, selinux_dep, util_dep, crypt_dep],

src/openrc-run/openrc-run.c

@@ -35,6 +35,7 @@
 #include <time.h>
 #include <unistd.h>
 #include <stdbool.h>
+#include <inttypes.h>
 
 #if defined(__linux__) || (defined(__FreeBSD_kernel__) && defined(__GLIBC__)) \
 	|| defined(__GNU__)
@@ -50,17 +51,15 @@
 #include "rc.h"
 #include "rc_exec.h"
 #include "misc.h"
+#include "timeutils.h"
 #include "plugin.h"
 #include "selinux.h"
 #include "_usage.h"
 #include "helpers.h"
 
-#define WAIT_INTERVAL	20000000	/* usecs to poll the lock file */
 #define WAIT_TIMEOUT	60		/* seconds until we timeout */
 #define WARN_TIMEOUT	10		/* warn about this every N seconds */
 
-static const struct timespec interval = { .tv_nsec = WAIT_INTERVAL };
-
 const char *applet = NULL;
 const char *extraopts = "stop | start | restart | status | describe | zap";
 const char getoptstring[] = "dDsSvl:Z" getoptstring_COMMON;
@@ -93,7 +92,8 @@ static RC_STRINGLIST *use_services;
 static RC_STRINGLIST *want_services;
 static RC_HOOK hook_out;
 static int exclusive_fd = -1, master_tty = -1;
-static bool sighup, skip_mark, in_background, deps, dry_run;
+static bool in_background, deps, dry_run;
+static volatile bool sighup, skip_mark, timedout;
 static pid_t service_pid;
 static int signal_pipe[2] = { -1, -1 };
 
@@ -156,6 +156,10 @@ handle_signal(int sig)
 		exit(EXIT_FAILURE);
 		/* NOTREACHED */
 
+	case SIGALRM:
+		timedout = true;
+		break;
+
 	default:
 		eerror("%s: caught unknown signal %d", applet, sig);
 	}
@@ -353,14 +357,13 @@ svc_exec(const char *arg1, const char *arg2)
 	int flags = 0;
 	struct pollfd fd[2];
 	int s;
-	char *buffer;
+	char buffer[BUFSIZ];
 	size_t bytes;
 	bool prefixed = false;
 	int slave_tty;
 	sigset_t sigchldmask;
 	sigset_t oldmask;
-	struct timespec timeout = { .tv_sec = WAIT_TIMEOUT };
-	struct timespec warn = { .tv_sec = WARN_TIMEOUT };
+	int64_t wait_timeout, warn_timeout, now;
 	RC_STRINGLIST *keywords;
 	bool forever;
 
@@ -396,6 +399,9 @@ svc_exec(const char *arg1, const char *arg2)
 			fcntl(slave_tty, F_SETFD, flags | FD_CLOEXEC);
 	}
 
+	now = tm_now();
+	wait_timeout = now + (WAIT_TIMEOUT * 1000);
+	warn_timeout = now + (WARN_TIMEOUT * 1000);
 	service_pid = fork();
 	if (service_pid == -1)
 		eerrorx("%s: fork: %s", applet, strerror(errno));
@@ -416,61 +422,62 @@ svc_exec(const char *arg1, const char *arg2)
 		/* UNREACHABLE */
 	}
 
-	buffer = xmalloc(sizeof(char) * BUFSIZ);
 	fd[0].fd = signal_pipe[0];
+	fd[1].fd = master_tty;
 	fd[0].events = fd[1].events = POLLIN;
 	fd[0].revents = fd[1].revents = 0;
-	if (master_tty >= 0) {
-		fd[1].fd = master_tty;
-		fd[1].events = POLLIN;
-		fd[1].revents = 0;
-	}
 
 	for (;;) {
-		if ((s = poll(fd, master_tty >= 0 ? 2 : 1, WAIT_INTERVAL / 1000000)) == -1) {
+		int timeout;
+		if (forever) {
+			timeout = -1;
+		} else if ((timeout = warn_timeout - tm_now()) < 0) {
+			timeout = 0;
+		}
+
+		if (poll(fd, master_tty >= 0 ? 2 : 1, timeout) == -1) {
 			if (errno != EINTR) {
 				eerror("%s: poll: %s", applet, strerror(errno));
 				ret = -1;
 				break;
 			}
+			continue;
+		}
+
+		if (fd[1].revents & (POLLIN | POLLHUP)) {
+			bytes = read(master_tty, buffer, BUFSIZ);
+			write_prefix(buffer, bytes, &prefixed);
 		}
 
-		if (s == 0 && !forever) {
-			timespecsub(&timeout, &interval, &timeout);
-			if (timeout.tv_sec <= 0) {
-				kill(service_pid, SIGKILL);
+		/* signal_pipe receives service_pid's exit status */
+		if (fd[0].revents & (POLLIN | POLLHUP)) {
+			if ((s = read(signal_pipe[0], &ret, sizeof(ret))) != sizeof(ret)) {
+				eerror("%s: receive failed: %s", applet, s < 0 ? strerror(errno) : "short read");
 				ret = -1;
 				break;
 			}
-			timespecsub(&warn, &interval, &warn);
-			if (warn.tv_sec <= 0) {
-				ewarn("%s: waiting for service (%d seconds)", applet, (int)timeout.tv_sec);
-				warn = (struct timespec) { .tv_sec = WARN_TIMEOUT };
-			}
-		} else if (s > 0) {
-			if (fd[1].revents & (POLLIN | POLLHUP)) {
-				bytes = read(master_tty, buffer, BUFSIZ);
-				write_prefix(buffer, bytes, &prefixed);
-			}
+			ret = WEXITSTATUS(ret);
+			if (ret != 0 && errno == ECHILD)
+				/* killall5 -9 could cause this */
+				ret = 0;
+			break;
+		}
 
-			/* signal_pipe receives service_pid's exit status */
-			if (fd[0].revents & (POLLIN | POLLHUP)) {
-				if ((s = read(signal_pipe[0], &ret, sizeof(ret))) != sizeof(ret)) {
-					eerror("%s: receive failed: %s", applet, s < 0 ? strerror(errno) : "short read");
-					ret = -1;
-					break;
-				}
-				ret = WEXITSTATUS(ret);
-				if (ret != 0 && errno == ECHILD)
-					/* killall5 -9 could cause this */
-					ret = 0;
-				break;
-			}
+		if (forever)
+			continue;
+
+		if ((now = tm_now()) >= wait_timeout) {
+			kill(service_pid, SIGKILL);
+			ret = -1;
+			break;
+		} else if (now >= warn_timeout) {
+			/* the timer might get off by a few ms, add 500ms so we get round numbers matching svc_wait. */
+			ewarn("%s: waiting for service (%"PRId64" seconds)", applet, (wait_timeout - now + 500) / 1000);
+			if ((warn_timeout += (WARN_TIMEOUT * 1000)) > wait_timeout)
+				warn_timeout = wait_timeout;
 		}
 	}
 
-	free(buffer);
-
 	sigemptyset (&sigchldmask);
 	sigaddset (&sigchldmask, SIGCHLD);
 	sigprocmask (SIG_BLOCK, &sigchldmask, &oldmask);
@@ -499,55 +506,44 @@ svc_wait(const char *svc)
 	int fd;
 	bool forever = false;
 	RC_STRINGLIST *keywords;
-	struct timespec timeout, warn;
 	const char *base = basename_c(svc);
+	int timeout = WAIT_TIMEOUT;
+	bool retval = false;
 
 	/* Some services don't have a timeout, like fsck */
 	keywords = rc_deptree_depend(deptree, svc, "keyword");
-	if (rc_stringlist_find(keywords, "-timeout") ||
-	    rc_stringlist_find(keywords, "notimeout"))
+	if (rc_stringlist_find(keywords, "-timeout") || rc_stringlist_find(keywords, "notimeout"))
 		forever = true;
 	rc_stringlist_free(keywords);
 
-	timeout.tv_sec = WAIT_TIMEOUT;
-	timeout.tv_nsec = 0;
-	warn.tv_sec = WARN_TIMEOUT;
-	warn.tv_nsec = 0;
+	if ((fd = openat(rc_dirfd(RC_DIR_EXCLUSIVE), base, O_RDONLY | O_NONBLOCK)) == -1)
+		return errno == ENOENT;
+
+	timedout = false;
 	for (;;) {
-		fd = openat(rc_dirfd(RC_DIR_EXCLUSIVE), base, O_RDONLY | O_NONBLOCK);
-		if (fd != -1) {
-			if (flock(fd, LOCK_SH | LOCK_NB) == 0) {
-				close(fd);
-				return true;
-			}
-			close(fd);
-		}
-		if (errno == ENOENT) {
-			return true;
-		}
-		if (errno != EWOULDBLOCK) {
-			eerror("%s: open '%s/exclusive/%s': %s", applet, rc_svcdir(), base, strerror(errno));
-			exit(EXIT_FAILURE);
-		}
-		if (nanosleep(&interval, NULL) == -1) {
-			if (errno != EINTR)
-				goto finish;
+		if (!forever)
+			alarm(WARN_TIMEOUT);
+
+		if (flock(fd, LOCK_SH) == 0) {
+			retval = true;
+			break;
 		}
-		if (!forever) {
-			timespecsub(&timeout, &interval, &timeout);
-			if (timeout.tv_sec <= 0)
-				goto finish;
-			timespecsub(&warn, &interval, &warn);
-			if (warn.tv_sec <= 0) {
-				ewarn("%s: waiting for %s (%d seconds)",
-				    applet, svc, (int)timeout.tv_sec);
-				warn.tv_sec = WARN_TIMEOUT;
-				warn.tv_nsec = 0;
-			}
+
+		if (errno != EINTR)
+			break;
+
+		if (!forever && timedout) {
+			timedout = false;
+			if ((timeout -= WARN_TIMEOUT) > 0)
+				ewarn("%s: waiting for %s (%d seconds)", applet, base, timeout);
+			else
+				break;
 		}
 	}
-finish:
-	return false;
+
+	alarm(0);
+	close(fd);
+	return retval;
 }
 
 static void
@@ -1236,6 +1232,7 @@ int main(int argc, char **argv)
 
 	/* Setup a signal handler */
 	signal_setup(SIGHUP, handle_signal);
+	signal_setup(SIGALRM, handle_signal);
 	signal_setup(SIGUSR1, handle_signal);
 	signal_setup(SIGINT, handle_signal);
 	signal_setup(SIGQUIT, handle_signal);

src/shared/misc.c

@@ -367,7 +367,7 @@ exec_service(const char *service, const char *arg)
 		fprintf(stderr, "fork: %s\n",strerror (errno));
 		svc_unlock(basename_c(service), fd);
 	} else
-		fcntl(fd, F_SETFD, fcntl(fd, F_GETFD, 0) | FD_CLOEXEC);
+		close(fd);
 
 	sigprocmask(SIG_SETMASK, &old, NULL);
 	free(file);

src/shared/misc.h

@@ -73,8 +73,8 @@ static const rc_service_state_name_t rc_service_state_names[] = {
  */
 int is_writable(const char *);
 
-#define service_start(service) exec_service(service, "start");
-#define service_stop(service)  exec_service(service, "stop");
+#define service_start(service) exec_service(service, "start")
+#define service_stop(service)  exec_service(service, "stop")
 
 int parse_mode(mode_t *, char *);