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.

"reads good" claudio@

3 files changed, 43 insertions, 72 deletions

diff --git a/usr.sbin/dvmrpd/dvmrpd.c b/usr.sbin/dvmrpd/dvmrpd.c
index 4f61727f67e..cd39c5f8e38 100644
--- a/usr.sbin/dvmrpd/dvmrpd.c
+++ b/usr.sbin/dvmrpd/dvmrpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dvmrpd.c,v 1.23 2016/09/02 15:35:34 renato Exp $ */
+/*	$OpenBSD: dvmrpd.c,v 1.24 2016/09/02 15:38:08 renato Exp $ */
 
 /*
  * Copyright (c) 2005 Claudio Jeker <claudio@openbsd.org>
@@ -50,8 +50,7 @@
 
 void		main_sig_handler(int, short, void *);
 __dead void	usage(void);
-void		dvmrpd_shutdown(void);
-int		check_child(pid_t, const char *);
+__dead void	dvmrpd_shutdown(void);
 
 void	main_dispatch_dvmrpe(int, short, void *);
 void	main_dispatch_rde(int, short, void *);
@@ -72,28 +71,12 @@ pid_t			 rde_pid;
 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(dvmrpe_pid, "dvmrp engine")) {
-			dvmrpe_pid = 0;
-			die = 1;
-		}
-		if (check_child(rde_pid, "route decision engine")) {
-			rde_pid = 0;
-			die = 1;
-		}
-		if (die)
-			dvmrpd_shutdown();
-		break;
+		dvmrpd_shutdown();
+		/* NOTREACHED */
 	case SIGHUP:
 		/* reconfigure */
 		/* ... */
@@ -116,7 +99,7 @@ usage(void)
 int
 main(int argc, char *argv[])
 {
-	struct event	 ev_sigint, ev_sigterm, ev_sigchld, ev_sighup;
+	struct event	 ev_sigint, ev_sigterm, ev_sighup;
 	char		*conffile;
 	int		 ch, opts = 0;
 	int		 debug = 0;
@@ -236,11 +219,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);
 
@@ -285,63 +266,46 @@ main(int argc, char *argv[])
 	return (0);
 }
 
-void
+__dead void
 dvmrpd_shutdown(void)
 {
 	struct iface	*iface;
 	pid_t		 pid;
+	int		 status;
 
-	if (dvmrpe_pid)
-		kill(dvmrpe_pid, SIGTERM);
-
-	if (rde_pid)
-		kill(rde_pid, SIGTERM);
+	/* close pipes */
+	msgbuf_clear(&iev_dvmrpe->ibuf.w);
+	close(iev_dvmrpe->ibuf.fd);
+	msgbuf_clear(&iev_rde->ibuf.w);
+	close(iev_rde->ibuf.fd);
 
 	control_cleanup();
 	kmr_shutdown();
 	kr_shutdown();
-
 	LIST_FOREACH(iface, &conf->iface_list, entry) {
 		if_del(iface);
 	}
-
 	mrt_done(conf->mroute_socket);
 
+	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" :
+			    "dvmrp engine", WTERMSIG(status));
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
 
-	msgbuf_clear(&iev_dvmrpe->ibuf.w);
 	free(iev_dvmrpe);
-	msgbuf_clear(&iev_rde->ibuf.w);
 	free(iev_rde);
 
 	log_info("terminating");
 	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 */
 void
 main_dispatch_dvmrpe(int fd, short event, void *bula)
diff --git a/usr.sbin/dvmrpd/dvmrpe.c b/usr.sbin/dvmrpd/dvmrpe.c
index 76b8cb16ea3..f907b122fc2 100644
--- a/usr.sbin/dvmrpd/dvmrpe.c
+++ b/usr.sbin/dvmrpd/dvmrpe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dvmrpe.c,v 1.17 2016/09/02 15:35:34 renato Exp $ */
+/*	$OpenBSD: dvmrpe.c,v 1.18 2016/09/02 15:38:08 renato Exp $ */
 
 /*
  * Copyright (c) 2005 Claudio Jeker <claudio@openbsd.org>
@@ -42,8 +42,8 @@
 #include "control.h"
 #include "log.h"
 
-void	 dvmrpe_sig_handler(int, short, void *);
-void	 dvmrpe_shutdown(void);
+void		 dvmrpe_sig_handler(int, short, void *);
+__dead void	 dvmrpe_shutdown(void);
 
 volatile sig_atomic_t	 dvmrpe_quit = 0;
 struct dvmrpd_conf	*deconf = NULL;
@@ -190,11 +190,19 @@ dvmrpe(struct dvmrpd_conf *xconf, int pipe_parent2dvmrpe[2],
 	return (0);
 }
 
-void
+__dead void
 dvmrpe_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);
+
 	/* stop all interfaces and delete them */
 	LIST_FOREACH(iface, &deconf->iface_list, entry) {
 		if (if_fsm(iface, IF_EVT_DOWN)) {
@@ -205,11 +213,7 @@ dvmrpe_shutdown(void)
 	}
 
 	/* 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(pkt_ptr);
 
diff --git a/usr.sbin/dvmrpd/rde.c b/usr.sbin/dvmrpd/rde.c
index fc12291e6de..99149157ecf 100644
--- a/usr.sbin/dvmrpd/rde.c
+++ b/usr.sbin/dvmrpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.27 2016/09/02 15:35:34 renato Exp $ */
+/*	$OpenBSD: rde.c,v 1.28 2016/09/02 15:38:08 renato Exp $ */
 
 /*
  * Copyright (c) 2004, 2005 Claudio Jeker <claudio@openbsd.org>
@@ -40,7 +40,7 @@
 #include "rde.h"
 
 void		 rde_sig_handler(int sig, short, void *);
-void		 rde_shutdown(void);
+__dead void	 rde_shutdown(void);
 void		 rde_dispatch_imsg(int, short, void *);
 
 int		 rde_select_ds_ifs(struct mfc *, struct iface *);
@@ -147,15 +147,20 @@ rde(struct dvmrpd_conf *xconf, int pipe_parent2rde[2], int pipe_dvmrpe2rde[2],
 
 	rde_shutdown();
 	/* NOTREACHED */
-
 	return (0);
 }
 
-void
+__dead void
 rde_shutdown(void)
 {
 	struct iface	*iface;
 
+	/* close pipes */
+	msgbuf_clear(&iev_dvmrpe->ibuf.w);
+	close(iev_dvmrpe->ibuf.fd);
+	msgbuf_clear(&iev_main->ibuf.w);
+	close(iev_main->ibuf.fd);
+
 	rt_clear();
 	mfc_clear();
 
@@ -163,9 +168,7 @@ rde_shutdown(void)
 		if_del(iface);
 	}
 
-	msgbuf_clear(&iev_dvmrpe->ibuf.w);
 	free(iev_dvmrpe);
-	msgbuf_clear(&iev_main->ibuf.w);
 	free(iev_main);
 	free(rdeconf);