[PATCH 4/4] Use threads for event processing

[PATCH 4/4] Use threads for event processing

[Alan Jenkins]

Do-not-merge: libc calls need to be audited for thread safety.

diff --git a/udev/test-udev.c b/udev/test-udev.c
index 929f752..49ddd5b 100644
--- a/udev/test-udev.c
+++ b/udev/test-udev.c
@@ -107,6 +107,7 @@ int main(int argc, char *argv[])
 fail:
 	udev_rules_unref(rules);
 exit:
+	udev_exec_cleanup(udev);
 	udev_selinux_exit(udev);
 	udev_unref(udev);
 	if (err != 0)
diff --git a/udev/udev.h b/udev/udev.h
index fd3c73d..53516e4 100644
--- a/udev/udev.h
+++ b/udev/udev.h
@@ -39,6 +39,9 @@
 #define READ_END				0
 #define WRITE_END				1
 
+#define UDEVD_PRIORITY			-4 /* nice level for daemon */
+#define UDEV_PRIORITY			-2 /* nice level for programs run by daemon*/
+
 static inline void logging_init(const char *program_name)
 {
 	openlog(program_name, LOG_PID | LOG_CONS, LOG_DAEMON);
@@ -77,7 +80,7 @@ struct udev_event {
 	int test;
 
 	struct udev_list_node node;
-	pid_t pid;
+	pthread_t thread;
 	int exitstatus;
 	time_t queue_time;
 };
@@ -106,6 +109,7 @@ extern void udev_node_update_old_links(struct udev_device *dev, struct udev_devi
 
 /* udev-exec.c */
 extern int udev_exec_init(struct udev *udev);
+extern void udev_exec_cleanup(struct udev *udev);
 extern int udev_exec(struct udev_device *dev, const char *command,
 		     char *result, size_t ressize, size_t *reslen);
 
diff --git a/udev/udevd.c b/udev/udevd.c
index a2c4230..9d2b67d 100644
--- a/udev/udevd.c
+++ b/udev/udevd.c
@@ -34,12 +34,10 @@
 #ifdef HAVE_INOTIFY
 #include <sys/inotify.h>
 #endif
+#include <pthread.h>
 
 #include "udev.h"
 
-#define UDEVD_PRIORITY			-4
-#define UDEV_PRIORITY			-2
-
 /* maximum limit of forked childs */
 #define UDEVD_MAX_CHILDS		256
 
@@ -64,7 +62,7 @@ static struct udev_monitor *kernel_monitor;
 static int inotify_fd = -1;
 
 static int signal_pipe[2] = {-1, -1};
-static volatile int sigchilds_waiting;
+static int event_done_pipe[2] = {-1, -1};
 static volatile int udev_exit;
 static volatile int reload_config;
 static int run_exec_q;
@@ -74,6 +72,8 @@ static int max_childs;
 static struct udev_list_node exec_list;
 static struct udev_list_node running_list;
 
+pthread_attr_t thread_attr;
+
 enum event_state {
 	EVENT_QUEUED,
 	EVENT_FINISHED,
@@ -168,71 +168,68 @@ static void event_queue_delete(struct udev_event *event)
 	udev_event_unref(event);
 }
 
-static void asmlinkage event_sig_handler(int signum)
+static int event_run(struct udev_event *event)
 {
-	if (signum = SIGALRM)
-		exit(1);
+	int err;
+
+	/* apply rules, create node, symlinks */
+	err = udev_event_execute_rules(event, rules);
+
+	/* execute RUN= */
+	if (err = 0 && !event->ignore_device && udev_get_run(event->udev))
+		udev_event_execute_run(event);
+	info(event->udev, "seq %llu exit with %i\n", udev_device_get_seqnum(event->dev), err);
+
+	return err;
 }
 
-static void event_fork(struct udev_event *event)
+static void *event_run_pthread(void *arg)
 {
-	pid_t pid;
-	struct sigaction act;
+	struct udev_event *event = arg;
 	int err;
 
-	pid = fork();
-	switch (pid) {
-	case 0:
-		/* child */
-		udev_monitor_unref(kernel_monitor);
-		udev_ctrl_unref(udev_ctrl);
-		if (inotify_fd >= 0)
-			close(inotify_fd);
-		close(signal_pipe[READ_END]);
-		close(signal_pipe[WRITE_END]);
-		logging_close();
-		logging_init("udevd-event");
-		setpriority(PRIO_PROCESS, 0, UDEV_PRIORITY);
-
-		/* set signal handlers */
-		memset(&act, 0x00, sizeof(act));
-		act.sa_handler = (void (*)(int)) event_sig_handler;
-		sigemptyset (&act.sa_mask);
-		act.sa_flags = 0;
-		sigaction(SIGALRM, &act, NULL);
-
-		/* reset to default */
-		act.sa_handler = SIG_DFL;
-		sigaction(SIGINT, &act, NULL);
-		sigaction(SIGTERM, &act, NULL);
-		sigaction(SIGCHLD, &act, NULL);
-		sigaction(SIGHUP, &act, NULL);
-
-		/* apply rules, create node, symlinks */
-		err = udev_event_execute_rules(event, rules);
-
-		/* execute RUN= */
-		if (err = 0 && !event->ignore_device && udev_get_run(event->udev))
-			udev_event_execute_run(event);
-		info(event->udev, "seq %llu exit with %i\n", udev_device_get_seqnum(event->dev), err);
-		logging_close();
-		if (err != 0)
-			exit(1);
-		exit(0);
-	case -1:
-		err(event->udev, "fork of child failed: %m\n");
+	err = event_run(event);
+
+	/* write to pipe, which will wakeup select() in our mainloop */
+	write(event_done_pipe[WRITE_END], &event, sizeof(struct udev_event *));
+
+	return (void *) (uintptr_t) err;
+}
+
+static void event_thread_create(struct udev_event *event)
+{
+	if (pthread_create(&event->thread, &thread_attr, event_run_pthread, event) != 0) {
+		err(event->udev, "create thread failed: %m\n");
 		event_queue_delete(event);
-		break;
-	default:
-		/* get SIGCHLD in main loop */
-		info(event->udev, "seq %llu forked, pid [%d], '%s' '%s', %ld seconds old\n",
-		     udev_device_get_seqnum(event->dev),
-		     pid,
-		     udev_device_get_action(event->dev),
-		     udev_device_get_subsystem(event->dev),
-		     time(NULL) - event->queue_time);
-		event->pid = pid;
 	}
+
+	info(event->udev, "seq %llu created, '%s' '%s', %ld seconds old\n",
+	     udev_device_get_seqnum(event->dev),
+	     udev_device_get_action(event->dev),
+	     udev_device_get_subsystem(event->dev),
+	     time(NULL) - event->queue_time);
+}
+
+static int event_thread_join(struct udev_event *event)
+{
+	void *result;
+	int err;
+
+	err = pthread_join(event->thread, &result);
+	if (err) {
+		err(event->udev, "failed to retrieve thread return value: %m\n");
+		return -1;
+	}
+
+	return (int) (uintptr_t) result;
+}
+
+static void event_thread_kill(struct udev_event *event)
+{
+	pthread_kill(event->thread, SIGTERM);
+
+	/* Make sure thread has died before we return */
+	event_thread_join(event);
 }
 
 static void event_queue_insert(struct udev_event *event)
@@ -261,8 +258,7 @@ static void event_queue_insert(struct udev_event *event)
 	/* run one event after the other in debug mode */
 	if (debug_trace) {
 		udev_list_node_append(&event->node, &running_list);
-		event_fork(event);
-		waitpid(event->pid, NULL, 0);
+		event->exitstatus = event_run(event);
 		event_queue_delete(event);
 		return;
 	}
@@ -270,7 +266,7 @@ static void event_queue_insert(struct udev_event *event)
 	/* run all events with a timeout set immediately */
 	if (udev_device_get_timeout(event->dev) > 0) {
 		udev_list_node_append(&event->node, &running_list);
-		event_fork(event);
+		event_thread_create(event);
 		return;
 	}
 
@@ -451,7 +447,7 @@ static void event_queue_manager(struct udev *udev)
 		/* move event to run list */
 		udev_list_node_remove(&loop_event->node);
 		udev_list_node_append(&loop_event->node, &running_list);
-		event_fork(loop_event);
+		event_thread_create(loop_event);
 		dbg(udev, "moved seq %llu to running list\n", udev_device_get_seqnum(loop_event->dev));
 	}
 }
@@ -530,11 +526,8 @@ static void asmlinkage sig_handler(int signum)
 	switch (signum) {
 		case SIGINT:
 		case SIGTERM:
-			udev_exit = 1;
-			break;
 		case SIGCHLD:
-			/* set flag, then write to pipe if needed */
-			sigchilds_waiting = 1;
+			udev_exit = 1;
 			break;
 		case SIGHUP:
 			reload_config = 1;
@@ -545,45 +538,17 @@ static void asmlinkage sig_handler(int signum)
 	write(signal_pipe[WRITE_END], "", 1);
 }
 
-static void udev_done(int pid, int exitstatus)
+static void udev_done(struct udev_event *event)
 {
-	struct udev_list_node *loop;
+	event->exitstatus = event_thread_join(event);
+	info(event->udev, "seq %llu cleanup, status %i, %ld seconds old\n",
+	     udev_device_get_seqnum(event->dev),
+	     event->exitstatus, time(NULL) - event->queue_time);
 
-	/* find event associated with pid and delete it */
-	udev_list_node_foreach(loop, &running_list) {
-		struct udev_event *loop_event = node_to_event(loop);
-
-		if (loop_event->pid = pid) {
-			info(loop_event->udev, "seq %llu cleanup, pid [%d], status %i, %ld seconds old\n",
-			     udev_device_get_seqnum(loop_event->dev), loop_event->pid,
-			     exitstatus, time(NULL) - loop_event->queue_time);
-			loop_event->exitstatus = exitstatus;
-			event_queue_delete(loop_event);
+	event_queue_delete(event);
 
-			/* there may be events waiting with the same devpath */
-			run_exec_q = 1;
-			return;
-		}
-	}
-}
-
-static void reap_sigchilds(void)
-{
-	pid_t pid;
-	int status;
-
-	while (1) {
-		pid = waitpid(-1, &status, WNOHANG);
-		if (pid <= 0)
-			break;
-		if (WIFEXITED(status))
-			status = WEXITSTATUS(status);
-		else if (WIFSIGNALED(status))
-			status = WTERMSIG(status) + 128;
-		else
-			status = 0;
-		udev_done(pid, status);
-	}
+	/* there may be events waiting with the same devpath */
+	run_exec_q = 1;
 }
 
 static void export_initial_seqnum(struct udev *udev)
@@ -620,6 +585,8 @@ int main(int argc, char *argv[])
 	int err;
 	int fd;
 	struct sigaction act;
+	int pagesize;
+	int stacksize;
 	fd_set readfds;
 	const char *value;
 	int daemonize = 0;
@@ -729,6 +696,11 @@ int main(int argc, char *argv[])
 		err(udev, "error getting pipes: %m\n");
 		goto exit;
 	}
+	err = pipe(event_done_pipe);
+	if (err < 0) {
+		err(udev, "error getting pipes: %m\n");
+		goto exit;
+	}
 
 	err = fcntl(signal_pipe[READ_END], F_GETFL, 0);
 	if (err < 0) {
@@ -791,6 +763,29 @@ int main(int argc, char *argv[])
 	/* set scheduling priority for the daemon */
 	setpriority(PRIO_PROCESS, 0, UDEVD_PRIORITY);
 
+	pthread_attr_init(&thread_attr);
+
+	/* To create many threads on 32-bit, it may be necessary to explicitly limit the stack size.
+
+	   udevd doesn't use crazy recursion or deep call-chains.
+	   But it does allocate quite a few buffers and temporary strings on the stack.
+	   We may also need extra space for exotic NSS modules (user/group lookup, e.g. over LDAP).
+	   Make a generous guess, double it, and double it again!
+
+	   This comes to just over 200Kb.  The default on i386 is 2Mb.
+	 */
+	stacksize = PTHREAD_STACK_MIN;
+	stacksize += UTIL_PATH_SIZE * 8;
+	stacksize += UTIL_LINE_SIZE * 8;
+	stacksize += UTIL_NAME_SIZE * 8;
+	stacksize += 4096 * 8;
+	stacksize *= 4;
+
+	/* Round up to a whole number of pages */
+	pagesize = sysconf(_SC_PAGESIZE);
+	stacksize = ((stacksize / pagesize) + 1) * pagesize;
+	pthread_attr_setstacksize(&thread_attr, stacksize);
+
 	/* OOM_DISABLE = -17 */
 	fd = open("/proc/self/oom_adj", O_RDWR);
 	if (fd < 0)
@@ -872,12 +867,14 @@ int main(int argc, char *argv[])
 	maxfd = udev_ctrl_get_fd(udev_ctrl);
 	maxfd = UDEV_MAX(maxfd, udev_monitor_get_fd(kernel_monitor));
 	maxfd = UDEV_MAX(maxfd, signal_pipe[READ_END]);
+	maxfd = UDEV_MAX(maxfd, event_done_pipe[READ_END]);
 	maxfd = UDEV_MAX(maxfd, inotify_fd);
 	while (!udev_exit) {
 		int fdcount;
 
 		FD_ZERO(&readfds);
 		FD_SET(signal_pipe[READ_END], &readfds);
+		FD_SET(event_done_pipe[READ_END], &readfds);
 		FD_SET(udev_ctrl_get_fd(udev_ctrl), &readfds);
 		FD_SET(udev_monitor_get_fd(kernel_monitor), &readfds);
 		if (inotify_fd >= 0)
@@ -916,6 +913,21 @@ int main(int argc, char *argv[])
 			read(signal_pipe[READ_END], &buf, sizeof(buf));
 		}
 
+		/* processed events */
+		if (FD_ISSET(event_done_pipe[READ_END], &readfds)) {
+			struct udev_event *buf[32];
+			ssize_t count;
+			unsigned i;
+
+			count = read(event_done_pipe[READ_END], &buf, sizeof(buf));
+			if (count < 0) {
+				err(udev, "read error on thread pipe: %m\n");
+			} else {
+				for (i = 0; i < count / sizeof(buf[0]); i++)
+					udev_done(buf[i]);
+			}
+		}
+
 		/* rules directory inotify watch */
 		if ((inotify_fd >= 0) && FD_ISSET(inotify_fd, &readfds)) {
 			int nbytes;
@@ -948,11 +960,6 @@ int main(int argc, char *argv[])
 			}
 		}
 
-		if (sigchilds_waiting) {
-			sigchilds_waiting = 0;
-			reap_sigchilds();
-		}
-
 		if (run_exec_q) {
 			run_exec_q = 0;
 			if (!stop_exec_q)
@@ -961,8 +968,20 @@ int main(int argc, char *argv[])
 	}
 	rc = 0;
 
+	/* Kill event threads before destroying global udev context */
+	if (!udev_list_is_empty(&running_list)) {
+		struct udev_list_node *loop;
+
+		udev_list_node_foreach(loop, &running_list) {
+			struct udev_event *loop_event = node_to_event(loop);
+
+			event_thread_kill(loop_event);
+		}
+	}
+
 exit:
 	udev_rules_unref(rules);
+	pthread_attr_destroy(&thread_attr);
 
 	if (signal_pipe[READ_END] >= 0)
 		close(signal_pipe[READ_END]);
@@ -974,6 +993,7 @@ exit:
 		close(inotify_fd);
 	udev_monitor_unref(kernel_monitor);
 
+	udev_exec_cleanup(udev);
 	udev_selinux_exit(udev);
 	udev_unref(udev);
 	logging_close();