Simplify shutdown process.

On shutdown, there's no need to use kill(2) to kill the child
processes. Just closing the IPC sockets will make the children receive
an EOF, break out from the event loop and then exit.

Tha advantages of this "pipe teardown" are:
* simpler code;
* no need to pledge "proc" in the parent process;
* removal of a (hard to trigger) PID reuse race condition.

ok benno@ claudio@

3 files changed, 42 insertions, 70 deletions

diff --git a/usr.sbin/ripd/rde.c b/usr.sbin/ripd/rde.c
index 70940d62a26..61085344060 100644
--- a/usr.sbin/ripd/rde.c
+++ b/usr.sbin/ripd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.20 2016/09/02 14:07:52 benno Exp $ */
+/*	$OpenBSD: rde.c,v 1.21 2016/09/03 10:28:08 renato Exp $ */
 
 /*
  * Copyright (c) 2006 Michele Marchetto <mydecay@openbeer.it>
@@ -45,7 +45,7 @@ struct imsgev		*iev_ripe;
 struct imsgev		*iev_main;
 
 void	rde_sig_handler(int, short, void *);
-void	rde_shutdown(void);
+__dead void rde_shutdown(void);
 void	rde_dispatch_imsg(int, short, void *);
 void	rde_dispatch_parent(int, short, void *);
 int	rde_imsg_compose_ripe(int, u_int32_t, pid_t, void *, u_int16_t);
@@ -159,14 +159,17 @@ rde(struct ripd_conf *xconf, int pipe_parent2rde[2], int pipe_ripe2rde[2],
 	return (0);
 }
 
-void
+__dead void
 rde_shutdown(void)
 {
-	rt_clear();
-
+	/* close pipes */
 	msgbuf_clear(&iev_ripe->ibuf.w);
-	free(iev_ripe);
+	close(iev_ripe->ibuf.fd);
 	msgbuf_clear(&iev_main->ibuf.w);
+	close(iev_main->ibuf.fd);
+
+	rt_clear();
+	free(iev_ripe);
 	free(iev_main);
 	free(rdeconf);
 
diff --git a/usr.sbin/ripd/ripd.c b/usr.sbin/ripd/ripd.c
index 95bf8f56d5f..1450973d9bd 100644
--- a/usr.sbin/ripd/ripd.c
+++ b/usr.sbin/ripd/ripd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ripd.c,v 1.29 2016/09/02 14:07:52 benno Exp $ */
+/*	$OpenBSD: ripd.c,v 1.30 2016/09/03 10:28:08 renato Exp $ */
 
 /*
  * Copyright (c) 2006 Michele Marchetto <mydecay@openbeer.it>
@@ -48,9 +48,8 @@
 #include "rde.h"
 
 __dead void		 usage(void);
-int			 check_child(pid_t, const char *);
 void			 main_sig_handler(int, short, void *);
-void			 ripd_shutdown(void);
+__dead void		 ripd_shutdown(void);
 void			 main_dispatch_ripe(int, short, void *);
 void			 main_dispatch_rde(int, short, void *);
 
@@ -80,29 +79,12 @@ usage(void)
 void
 main_sig_handler(int sig, short event, void *arg)
 {
-	/*
-	 * signal handler rules don't apply, libevent decouples for us
-	 */
-
-	int	die = 0;
-
+	/* signal handler rules don't apply, libevent decouples for us */
 	switch (sig) {
 	case SIGTERM:
 	case SIGINT:
-		die = 1;
-		/* FALLTHROUGH */
-	case SIGCHLD:
-		if (check_child(ripe_pid, "rip engine")) {
-			ripe_pid = 0;
-			die = 1;
-		}
-		if (check_child(rde_pid, "route decision engine")) {
-			rde_pid = 0;
-			die = 1;
-		}
-		if (die)
-			ripd_shutdown();
-		break;
+		ripd_shutdown();
+		/* NOTREACHED */
 	case SIGHUP:
 		/* reconfigure */
 		/* ... */
@@ -116,7 +98,7 @@ main_sig_handler(int sig, short event, void *arg)
 int
 main(int argc, char *argv[])
 {
-	struct event	 ev_sigint, ev_sigterm, ev_sigchld, ev_sighup;
+	struct event	 ev_sigint, ev_sigterm, ev_sighup;
 	int		 mib[4];
 	int		 debug = 0;
 	int		 ipforwarding;
@@ -234,11 +216,9 @@ main(int argc, char *argv[])
 	/* setup signal handler */
 	signal_set(&ev_sigint, SIGINT, main_sig_handler, NULL);
 	signal_set(&ev_sigterm, SIGTERM, main_sig_handler, NULL);
-	signal_set(&ev_sigchld, SIGCHLD, main_sig_handler, NULL);
 	signal_set(&ev_sighup, SIGHUP, main_sig_handler, NULL);
 	signal_add(&ev_sigint, NULL);
 	signal_add(&ev_sigterm, NULL);
-	signal_add(&ev_sigchld, NULL);
 	signal_add(&ev_sighup, NULL);
 	signal(SIGPIPE, SIG_IGN);
 
@@ -278,17 +258,18 @@ main(int argc, char *argv[])
 	return (0);
 }
 
-void
+__dead void
 ripd_shutdown(void)
 {
 	struct iface	*i;
 	pid_t		 pid;
+	int		 status;
 
-	if (ripe_pid)
-		kill(ripe_pid, SIGTERM);
-
-	if (rde_pid)
-		kill(rde_pid, SIGTERM);
+	/* close pipes */
+	msgbuf_clear(&iev_ripe->ibuf.w);
+	close(iev_ripe->ibuf.fd);
+	msgbuf_clear(&iev_rde->ibuf.w);
+	close(iev_rde->ibuf.fd);
 
 	while ((i = LIST_FIRST(&conf->iface_list)) != NULL) {
 		LIST_REMOVE(i, entry);
@@ -298,15 +279,19 @@ ripd_shutdown(void)
 	control_cleanup(conf->csock);
 	kr_shutdown();
 
+	log_debug("waiting for children to terminate");
 	do {
-		if ((pid = wait(NULL)) == -1 &&
-		    errno != EINTR && errno != ECHILD)
-			fatal("wait");
+		pid = wait(&status);
+		if (pid == -1) {
+			if (errno != EINTR && errno != ECHILD)
+				fatal("wait");
+		} else if (WIFSIGNALED(status))
+			log_warnx("%s terminated; signal %d",
+			    (pid == rde_pid) ? "route decision engine" :
+			    "rip engine", WTERMSIG(status));
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
 
-	msgbuf_clear(&iev_ripe->ibuf.w);
 	free(iev_ripe);
-	msgbuf_clear(&iev_rde->ibuf.w);
 	free(iev_rde);
 	free(conf);
 
@@ -314,26 +299,6 @@ ripd_shutdown(void)
 	exit(0);
 }
 
-int
-check_child(pid_t pid, const char *pname)
-{
-	int	status;
-
-	if (waitpid(pid, &status, WNOHANG) > 0) {
-		if (WIFEXITED(status)) {
-			log_warnx("lost child: %s exited", pname);
-			return (1);
-		}
-		if (WIFSIGNALED(status)) {
-			log_warnx("lost child: %s terminated; signal %d",
-			    pname, WTERMSIG(status));
-			return (1);
-		}
-	}
-
-	return (0);
-}
-
 /* imsg handling */
 /* ARGSUSED */
 void
diff --git a/usr.sbin/ripd/ripe.c b/usr.sbin/ripd/ripe.c
index b70fb8c151c..2a10c003387 100644
--- a/usr.sbin/ripd/ripe.c
+++ b/usr.sbin/ripd/ripe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ripe.c,v 1.21 2016/09/02 14:07:52 benno Exp $ */
+/*	$OpenBSD: ripe.c,v 1.22 2016/09/03 10:28:08 renato Exp $ */
 
 /*
  * Copyright (c) 2006 Michele Marchetto <mydecay@openbeer.it>
@@ -43,7 +43,7 @@
 #include "control.h"
 
 void		 ripe_sig_handler(int, short, void *);
-void		 ripe_shutdown(void);
+__dead void	 ripe_shutdown(void);
 
 struct ripd_conf	*oeconf = NULL;
 struct imsgev		*iev_main;
@@ -450,11 +450,19 @@ ripe_dispatch_rde(int fd, short event, void *bula)
 	}
 }
 
-void
+__dead void
 ripe_shutdown(void)
 {
 	struct iface	*iface;
 
+	/* close pipes */
+	msgbuf_write(&iev_rde->ibuf.w);
+	msgbuf_clear(&iev_rde->ibuf.w);
+	close(iev_rde->ibuf.fd);
+	msgbuf_write(&iev_main->ibuf.w);
+	msgbuf_clear(&iev_main->ibuf.w);
+	close(iev_main->ibuf.fd);
+
 	LIST_FOREACH(iface, &oeconf->iface_list, entry) {
 		if (if_fsm(iface, IF_EVT_DOWN)) {
 			log_debug("error stopping interface %s",
@@ -469,11 +477,7 @@ ripe_shutdown(void)
 	close(oeconf->rip_socket);
 
 	/* clean up */
-	msgbuf_write(&iev_rde->ibuf.w);
-	msgbuf_clear(&iev_rde->ibuf.w);
 	free(iev_rde);
-	msgbuf_write(&iev_main->ibuf.w);
-	msgbuf_clear(&iev_main->ibuf.w);
 	free(iev_main);
 	free(oeconf);
 	free(pkt_ptr);