Implement a different fix for the waitpid race condition not needing a reaper thread for each child

3 files changed, 39 insertions, 9 deletions

diff --git a/src/fastd.c b/src/fastd.c
index 789b3a6..8dbfc74 100644
--- a/src/fastd.c
+++ b/src/fastd.c
@@ -75,7 +75,24 @@ static void on_sigusr1(int signo UNUSED) {
 }
 
 static void on_sigchld(int signo UNUSED) {
-	while (waitpid(-1, NULL, WNOHANG) > 0) {}
+	size_t i;
+	for (i = 0; i < VECTOR_LEN(ctx.async_pids);) {
+		pid_t pid = VECTOR_INDEX(ctx.async_pids, i);
+		if (waitpid(pid, NULL, WNOHANG) > 0) {
+			pr_debug("child process %u finished", (unsigned)pid);
+		}
+		else {
+			if (errno == ECHILD) {
+				i++;
+				continue;
+			}
+			else {
+				pr_error_errno("waitpid");
+			}
+		}
+
+		VECTOR_DELETE(ctx.async_pids, i);
+	}
 }
 
 static void init_signals(void) {
@@ -579,6 +596,7 @@ int main(int argc, char *argv[]) {
 
 	VECTOR_ALLOC(ctx.eth_addrs, 0);
 	VECTOR_ALLOC(ctx.peers, 0);
+	VECTOR_ALLOC(ctx.async_pids, 0);
 
 	fastd_peer_hashtable_init();
 
@@ -625,6 +643,7 @@ int main(int argc, char *argv[]) {
 
 	fastd_peer_hashtable_free();
 
+	VECTOR_FREE(ctx.async_pids);
 	VECTOR_FREE(ctx.peers);
 	VECTOR_FREE(ctx.eth_addrs);
 
diff --git a/src/fastd.h b/src/fastd.h
index 232778d..6580458 100644
--- a/src/fastd.h
+++ b/src/fastd.h
@@ -246,6 +246,7 @@ struct fastd_context {
 	fastd_dlist_head_t handshake_queue;
 	struct timespec next_maintenance;
 
+	VECTOR(pid_t) async_pids;
 	int async_rfd;
 	int async_wfd;
 
diff --git a/src/shell.c b/src/shell.c
index 9d03fbc..2be7f82 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -103,18 +103,15 @@ static void shell_command_setenv(pid_t pid, const fastd_shell_env_t *env) {
 	}
 }
 
-static bool shell_command_do_exec(const fastd_shell_command_t *command, const fastd_shell_env_t *env, pid_t *pid_ret) {
+static bool shell_command_do_exec(const fastd_shell_command_t *command, const fastd_shell_env_t *env, pid_t *pid) {
 	pid_t parent = getpid();
 
-	pid_t pid = fork();
-	if (pid < 0) {
+	*pid = fork();
+	if (*pid < 0) {
 		pr_error_errno("shell_command_do_exec: fork");
 		return false;
 	}
-	else if (pid > 0) {
-		if (pid_ret)
-			*pid_ret = pid;
-
+	else if (*pid > 0) {
 		return true;
 	}
 
@@ -174,6 +171,19 @@ bool fastd_shell_command_exec_sync(const fastd_shell_command_t *command, const f
 	return true;
 }
 
+static void shell_command_exec_async(const fastd_shell_command_t *command, const fastd_shell_env_t *env) {
+	/* block SIGCHLD */
+	sigset_t set, oldset;
+	sigemptyset(&set);
+	sigaddset(&set, SIGCHLD);
+	pthread_sigmask(SIG_BLOCK, &set, &oldset);
+
+	pid_t pid;
+	if (shell_command_do_exec(command, env, &pid))
+		VECTOR_ADD(ctx.async_pids, pid);
+
+	pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+}
 
 void fastd_shell_command_exec(const fastd_shell_command_t *command, const fastd_shell_env_t *env) {
 	if (!fastd_shell_command_isset(command))
@@ -182,5 +192,5 @@ void fastd_shell_command_exec(const fastd_shell_command_t *command, const fastd_
 	if (command->sync)
 		fastd_shell_command_exec_sync(command, env, NULL);
 	else
-		shell_command_do_exec(command, env, NULL);
+		shell_command_exec_async(command, env);
 }