Using fifos for locking can be error prone.

flocks are safer, as we only use tmpfs for our lock files.

I don't know how this works for inactive just yet though ...

src/includes/rc-misc.h

@@ -150,6 +150,8 @@ bool rc_conf_yesno(const char *var);
 void env_filter(void);
 void env_config(void);
 int signal_setup(int sig, void (*handler)(int));
+int svc_lock(const char *);
+int svc_unlock(const char *, int);
 pid_t exec_service(const char *, const char *);
 
 #define service_start(service) exec_service(service, "start");

src/rc/rc-misc.c

@@ -29,6 +29,7 @@
  * SUCH DAMAGE.
  */
 
+#include <sys/file.h>
 #include <sys/types.h>
 #include <sys/utsname.h>
 
@@ -38,6 +39,7 @@
 #endif
 
 #include <ctype.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <signal.h>
 #include <stdio.h>
@@ -270,30 +272,56 @@ signal_setup(int sig, void (*handler)(int))
 	return sigaction(sig, &sa, NULL);
 }
 
+int
+svc_lock(const char *applet)
+{
+	char file[PATH_MAX];
+	int fd;
+
+	snprintf(file, sizeof(file), RC_SVCDIR "/exclusive/%s", applet);
+	fd = open(file, O_WRONLY | O_CREAT | O_NONBLOCK, 0664);
+	if (fd == -1)
+		return -1;
+	if (flock(fd, LOCK_EX | LOCK_NB) == -1) {
+		close(fd);
+		return -1;
+	}
+	return fd;
+}
+
+int
+svc_unlock(const char *applet, int fd)
+{
+	char file[PATH_MAX];
+
+	snprintf(file, sizeof(file), RC_SVCDIR "/exclusive/%s", applet);
+	close(fd);
+	unlink(file);
+	return -1;
+}
+
 pid_t
 exec_service(const char *service, const char *arg)
 {
-	char *file;
+	char *file, sfd[32];
-	char fifo[PATH_MAX];
+	int fd;
 	pid_t pid = -1;
 	sigset_t full;
 	sigset_t old;
 	struct sigaction sa;
 
+	fd = svc_lock(basename_c(service));
+	if (fd == -1)
+		return -1;
+	
 	file = rc_service_resolve(service);
 	if (!exists(file)) {
 		rc_service_mark(service, RC_SERVICE_STOPPED);
+		svc_unlock(basename_c(service), fd);
 		free(file);
 		return 0;
 	}
-
+	snprintf(sfd, sizeof(sfd), "%d", fd);
-	/* We create a fifo so that other services can wait until we complete */
-	snprintf(fifo, sizeof(fifo), RC_SVCDIR "/exclusive/%s",
-		 basename_c(service));
-	if (mkfifo(fifo, 0600) != 0 && errno != EEXIST) {
-		free(file);
-		return -1;
-	}
 
 	/* We need to block signals until we have forked */
 	memset(&sa, 0, sizeof (sa));
@@ -316,10 +344,10 @@ exec_service(const char *service, const char *arg)
 		sigprocmask(SIG_SETMASK, &old, NULL);
 
 		/* Safe to run now */
-		execl(file, file, arg, (char *) NULL);
+		execl(file, file, "--lockfd", sfd, arg, (char *) NULL);
 		fprintf(stderr, "unable to exec `%s': %s\n",
 			file, strerror(errno));
-		unlink(fifo);
+		svc_unlock(basename_c(service), fd);
 		_exit(EXIT_FAILURE);
 	}
 

src/rc/runscript.c

@@ -86,8 +86,7 @@ static RC_STRINGLIST *use_services = NULL;
 static RC_STRINGLIST *services = NULL;
 static RC_STRINGLIST *tmplist = NULL;
 static char *service = NULL;
-static char exclusive[PATH_MAX] = { '\0' };
+static int exclusive_fd = -1;
-static char mtime_test[PATH_MAX] = { '\0' };
 static RC_DEPTREE *deptree = NULL;
 static char *runlevel = NULL;
 static bool sighup = false;
@@ -195,58 +194,6 @@ handle_signal(int sig)
 	errno = serrno;
 }
 
-static time_t
-get_mtime(const char *pathname, bool follow_link)
-{
-	struct stat buf;
-	int retval;
-
-	if (!pathname)
-		return 0;
-	retval = follow_link ? stat(pathname, &buf) : lstat(pathname, &buf);
-	if (!retval)
-		return buf.st_mtime;
-	errno = 0;
-	return 0;
-}
-
-static const char *const tests[] = {
-	"starting", "started", "stopping", "inactive", "wasinactive", NULL
-};
-static bool
-in_control()
-{
-	char file[PATH_MAX];
-	time_t m;
-	time_t mtime;
-	int i = 0;
-
-	if (sighup)
-		return false;
-
-	if (!*mtime_test || !exists(mtime_test))
-		return false;
-
-	if (rc_service_state(applet) & RC_SERVICE_STOPPED)
-		return false;
-
-	if (!(mtime = get_mtime(mtime_test, false)))
-		return false;
-
-	while (tests[i]) {
-		snprintf(file, sizeof(file), RC_SVCDIR "/%s/%s",
-			 tests[i], applet);
-		if (exists(file)) {
-			m = get_mtime(file, false);
-			if (mtime < m && m != 0)
-				return false;
-		}
-		i++;
-	}
-
-	return true;
-}
-
 static void
 unhotplug()
 {
@@ -294,7 +241,7 @@ restore_state(void)
 {
 	RC_SERVICE state;
 
-	if (rc_in_plugin || !in_control())
+	if (rc_in_plugin || exclusive_fd == -1)
 		return;
 	state = rc_service_state(applet);
 	if (state & RC_SERVICE_STOPPING) {
@@ -312,11 +259,7 @@ restore_state(void)
 		if (rc_runlevel_starting())
 			rc_service_mark(applet, RC_SERVICE_FAILED);
 	}
-
+	exclusive_fd = svc_unlock(applet, exclusive_fd);
-	if (*exclusive) {
-		unlink(exclusive);
-		*exclusive = '\0';
-	}
 }
 
 static void
@@ -360,9 +303,6 @@ cleanup(void)
 	free(prefix);
 	free(runlevel);
 #endif
-
-	if (*mtime_test && !rc_in_plugin)
-		unlink(mtime_test);
 }
 
 static int
@@ -539,11 +479,11 @@ svc_exec(const char *arg1, const char *arg2)
 static bool
 svc_wait(const char *svc)
 {
-	char fifo[PATH_MAX];
+	char file[PATH_MAX];
 	struct timespec ts;
 	int nloops = WAIT_MAX * (ONE_SECOND / WAIT_INTERVAL);
 	int sloops = (ONE_SECOND / WAIT_INTERVAL) * 5;
-	bool retval = false;
+	int fd;
 	bool forever = false;
 	RC_STRINGLIST *keywords;
 
@@ -553,20 +493,28 @@ svc_wait(const char *svc)
 		forever = true;
 	rc_stringlist_free(keywords);
 
-	snprintf(fifo, sizeof(fifo), RC_SVCDIR "/exclusive/%s",
+	snprintf(file, sizeof(file), RC_SVCDIR "/exclusive/%s",
 		 basename_c(svc));
 	ts.tv_sec = 0;
 	ts.tv_nsec = WAIT_INTERVAL;
 
 	while (nloops) {
-		if (!exists(fifo)) {
+		fd = open(file, O_RDONLY | O_NONBLOCK);
-			retval = true;
+		if (fd != -1) {
-			break;
+			if (flock(fd, LOCK_SH | LOCK_NB) == 0) {
+				close(fd);
+				return true;
 			}
+			close(fd);
+		}
+		if (errno == ENOENT)
+			return true;
+		if (errno != EWOULDBLOCK)
+			eerrorx("%s: open `%s': %s", applet, file, strerror(errno));
 
 		if (nanosleep(&ts, NULL) == -1) {
 			if (errno != EINTR)
-				break;
+				return false;
 		}
 
 		if (!forever) {
@@ -579,9 +527,7 @@ svc_wait(const char *svc)
 		}
 	}
 
-	if (!exists(fifo))
+	return false;
-		retval = true;
-	return retval;
 }
 
 static RC_SERVICE
@@ -617,45 +563,6 @@ svc_status(void)
 	return state;
 }
 
-static void
-make_exclusive(void)
-{
-	/* We create a fifo so that other services can wait until we complete */
-	if (!*exclusive)
-		snprintf(exclusive, sizeof(exclusive),
-			 RC_SVCDIR "/exclusive/%s", applet);
-
-	if (mkfifo(exclusive, 0600) != 0 && errno != EEXIST &&
-	    (errno != EACCES || geteuid () == 0))
-		eerrorx ("%s: unable to create fifo `%s': %s",
-			 applet, exclusive, strerror(errno));
-
-	snprintf(mtime_test, sizeof(mtime_test),
-		 RC_SVCDIR "/exclusive/%s.%d", applet, getpid());
-
-	if (exists(mtime_test) && unlink(mtime_test) != 0) {
-		eerror("%s: unlink `%s': %s",
-		       applet, mtime_test, strerror(errno));
-		*mtime_test = '\0';
-		return;
-	}
-
-	if (symlink(service, mtime_test) != 0) {
-		eerror("%s: symlink `%s' to `%s': %s",
-		       applet, service, mtime_test, strerror(errno));
-		*mtime_test = '\0';
-	}
-}
-
-static void
-unlink_mtime_test(void)
-{
-	if (unlink(mtime_test) != 0)
-		eerror("%s: unlink `%s': %s",
-		       applet, mtime_test, strerror(errno));
-	*mtime_test = '\0';
-}
-
 static void
 get_started_services(void)
 {
@@ -722,24 +629,25 @@ svc_start(bool deps)
 			       " next runlevel", applet);
 	}
 
+	if (exclusive_fd == -1)
+		exclusive_fd = svc_lock(applet);
+	if (exclusive_fd == -1) {
+		if (errno == EACCES)
+			eerrorx("%s: superuser access required", applet);
+		if (state & RC_SERVICE_STOPPING)
+			ewarnx("WARNING: %s is stopping", applet);
+		else
+			ewarnx("WARNING: %s is already starting", applet);
+	}
+	
 	if (state & RC_SERVICE_STARTED) {
 		ewarn("WARNING: %s has already been started", applet);
 		return;
-	} else if (state & RC_SERVICE_STARTING)
+	} else if (state & RC_SERVICE_INACTIVE && ! background)
-		ewarnx("WARNING: %s is already starting", applet);
-	else if (state & RC_SERVICE_STOPPING)
-		ewarnx("WARNING: %s is stopping", applet);
-	else if (state & RC_SERVICE_INACTIVE && ! background)
 		ewarnx("WARNING: %s has already started, but is inactive",
 		       applet);
 	
-	if (!rc_service_mark(service, RC_SERVICE_STARTING)) {
+	rc_service_mark(service, RC_SERVICE_STARTING);
-		if (errno == EACCES)
-			eerrorx("%s: superuser access required", applet);
-		eerrorx("ERROR: %s has been started by something else", applet);
-	}
-
-	make_exclusive();
 	hook_out = RC_HOOK_SERVICE_START_OUT;
 	rc_plugin_run(RC_HOOK_SERVICE_START_IN, applet);
 
@@ -838,7 +746,6 @@ svc_start(bool deps)
 			/* Set the state now, then unlink our exclusive so that
 			   our scheduled list is preserved */
 			rc_service_mark(service, RC_SERVICE_STOPPED);
-			unlink_mtime_test();
 
 			rc_stringlist_free(use_services);
 			use_services = NULL;
@@ -885,23 +792,18 @@ svc_start(bool deps)
 	if (ibsave)
 		unsetenv("IN_BACKGROUND");
 
-	if (in_control()) {
 	if (!started)
 		eerrorx("ERROR: %s failed to start", applet);
-	} else {
+	else {
 		if (rc_service_state(service) & RC_SERVICE_INACTIVE)
 			ewarnx("WARNING: %s has started, but is inactive",
 			       applet);
-		else
-			ewarnx("WARNING: %s not under our control, aborting",
-			       applet);
 	}
 
 	rc_service_mark(service, RC_SERVICE_STARTED);
-	unlink_mtime_test();
+	exclusive_fd = svc_unlock(applet, exclusive_fd);
 	hook_out = RC_HOOK_SERVICE_START_OUT;
 	rc_plugin_run(RC_HOOK_SERVICE_START_DONE, applet);
-	unlink(exclusive);
 
 	/* Now start any scheduled services */
 	services = rc_services_scheduled(service);
@@ -948,20 +850,21 @@ svc_stop(bool deps)
 		    !(state & RC_SERVICE_INACTIVE))
 			exit (EXIT_FAILURE);
 
+	if (exclusive_fd == -1)
+		exclusive_fd = svc_lock(applet);
+	if (exclusive_fd == -1) {
+		if (errno == EACCES)
+			eerrorx("%s: superuser access required", applet);
+		if (state & RC_SERVICE_STOPPING)
+			ewarnx("WARNING: %s is already stopping", applet);
+		eerrorx("ERROR: %d %s has been stopped by something else", exclusive_fd, applet);
+	}
 	if (state & RC_SERVICE_STOPPED) {
 		ewarn("WARNING: %s is already stopped", applet);
 		return;
-	} else if (state & RC_SERVICE_STOPPING)
-		ewarnx("WARNING: %s is already stopping", applet);
-
-	if (!rc_service_mark(service, RC_SERVICE_STOPPING)) {
-		if (errno == EACCES)
-			eerrorx("%s: superuser access required", applet);
-		eerrorx("ERROR: %s has been stopped by something else", applet);
 	}
 
-	make_exclusive();
+	rc_service_mark(service, RC_SERVICE_STOPPING);
-
 	hook_out = RC_HOOK_SERVICE_STOP_OUT;
 	rc_plugin_run(RC_HOOK_SERVICE_STOP_IN, applet);
 
@@ -1062,9 +965,6 @@ svc_stop(bool deps)
 	if (ibsave)
 		unsetenv("IN_BACKGROUND");
 
-	if (!in_control())
-		ewarnx("WARNING: %s not under our control, aborting", applet);
-
 	if (!stopped)
 		eerrorx("ERROR: %s failed to stop", applet);
 
@@ -1073,10 +973,8 @@ svc_stop(bool deps)
 	else
 		rc_service_mark(service, RC_SERVICE_STOPPED);
 
-	unlink_mtime_test();
 	hook_out = RC_HOOK_SERVICE_STOP_OUT;
 	rc_plugin_run(RC_HOOK_SERVICE_STOP_DONE, applet);
-	unlink(exclusive);
 	hook_out = 0;
 	rc_plugin_run(RC_HOOK_SERVICE_STOP_OUT, applet);
 }
@@ -1148,18 +1046,20 @@ service_plugable(void)
 }
 
 #include "_usage.h"
-#define getoptstring "dDsv" getoptstring_COMMON
+#define getoptstring "dDsvl:" getoptstring_COMMON
 #define extraopts "stop | start | restart | describe | zap"
 static const struct option longopts[] = {
 	{ "debug",      0, NULL, 'd'},
 	{ "ifstarted",  0, NULL, 's'},
 	{ "nodeps",     0, NULL, 'D'},
+	{ "lockfd",     1, NULL, 'l'},
 	longopts_COMMON
 };
 static const char *const longopts_help[] = {
 	"set xtrace when running the script",
 	"only run commands when started",
 	"ignore dependencies",
+	"fd of the exclusive lock from rc",
 	longopts_help_COMMON
 };
 #include "_usage.c"
@@ -1291,6 +1191,9 @@ runscript(int argc, char **argv)
 		case 'd':
 			setenv("RC_DEBUG", "YES", 1);
 			break;
+		case 'l':
+			exclusive_fd = atoi(optarg);
+			break;
 		case 's':
 			if (!(rc_service_state(service) & RC_SERVICE_STARTED))
 				exit(EXIT_FAILURE);