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, 69 deletions

diff --git a/usr.sbin/ospfd/ospfd.c b/usr.sbin/ospfd/ospfd.c
index 911a6db75df..72a6c7a6cd8 100644
--- a/usr.sbin/ospfd/ospfd.c
+++ b/usr.sbin/ospfd/ospfd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ospfd.c,v 1.90 2016/09/02 14:02:48 benno Exp $ */
+/*	$OpenBSD: ospfd.c,v 1.91 2016/09/03 10:22:57 renato Exp $ */
 
 /*
  * Copyright (c) 2005 Claudio Jeker <claudio@openbsd.org>
@@ -48,8 +48,7 @@
 
 void		main_sig_handler(int, short, void *);
 __dead void	usage(void);
-void		ospfd_shutdown(void);
-int		check_child(pid_t, const char *);
+__dead void	ospfd_shutdown(void);
 
 void	main_dispatch_ospfe(int, short, void *);
 void	main_dispatch_rde(int, short, void *);
@@ -75,29 +74,12 @@ pid_t			 rde_pid = 0;
 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(ospfe_pid, "ospf engine")) {
-			ospfe_pid = 0;
-			die = 1;
-		}
-		if (check_child(rde_pid, "route decision engine")) {
-			rde_pid = 0;
-			die = 1;
-		}
-		if (die)
-			ospfd_shutdown();
-		break;
+		ospfd_shutdown();
+		/* NOTREACHED */
 	case SIGHUP:
 		if (ospf_reload() == -1)
 			log_warnx("configuration reload failed");
@@ -124,7 +106,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;
 	struct area		*a;
 	int			 ch, opts = 0;
 	int			 debug = 0;
@@ -249,11 +231,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);
 
@@ -299,17 +279,18 @@ main(int argc, char *argv[])
 	return (0);
 }
 
-void
+__dead void
 ospfd_shutdown(void)
 {
 	pid_t		 	 pid;
+	int			 status;
 	struct redistribute	*r;
 
-	if (ospfe_pid)
-		kill(ospfe_pid, SIGTERM);
-
-	if (rde_pid)
-		kill(rde_pid, SIGTERM);
+	/* close pipes */
+	msgbuf_clear(&iev_ospfe->ibuf.w);
+	close(iev_ospfe->ibuf.fd);
+	msgbuf_clear(&iev_rde->ibuf.w);
+	close(iev_rde->ibuf.fd);
 
 	control_cleanup(ospfd_conf->csock);
 	while ((r = SIMPLEQ_FIRST(&ospfd_conf->redist_list)) != NULL) {
@@ -319,15 +300,19 @@ ospfd_shutdown(void)
 	kr_shutdown();
 	carp_demote_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" :
+			    "ospf engine", WTERMSIG(status));
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
 
-	msgbuf_clear(&iev_ospfe->ibuf.w);
 	free(iev_ospfe);
-	msgbuf_clear(&iev_rde->ibuf.w);
 	free(iev_rde);
 	free(ospfd_conf);
 
@@ -335,26 +320,6 @@ ospfd_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/ospfd/ospfe.c b/usr.sbin/ospfd/ospfe.c
index 5efef533aa1..891941ececa 100644
--- a/usr.sbin/ospfd/ospfe.c
+++ b/usr.sbin/ospfd/ospfe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ospfe.c,v 1.95 2016/09/02 14:02:48 benno Exp $ */
+/*	$OpenBSD: ospfe.c,v 1.96 2016/09/03 10:22:57 renato Exp $ */
 
 /*
  * Copyright (c) 2005 Claudio Jeker <claudio@openbsd.org>
@@ -43,7 +43,7 @@
 #include "log.h"
 
 void		 ospfe_sig_handler(int, short, void *);
-void		 ospfe_shutdown(void);
+__dead void	 ospfe_shutdown(void);
 void		 orig_rtr_lsa_all(struct area *);
 struct iface	*find_vlink(struct abr_rtr *);
 
@@ -212,12 +212,20 @@ ospfe(struct ospfd_conf *xconf, int pipe_parent2ospfe[2], int pipe_ospfe2rde[2],
 	return (0);
 }
 
-void
+__dead void
 ospfe_shutdown(void)
 {
 	struct area	*area;
 	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 remove all areas */
 	while ((area = LIST_FIRST(&oeconf->area_list)) != NULL) {
 		LIST_FOREACH(iface, &area->iface_list, entry) {
@@ -234,11 +242,7 @@ ospfe_shutdown(void)
 	close(oeconf->ospf_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);
diff --git a/usr.sbin/ospfd/rde.c b/usr.sbin/ospfd/rde.c
index 55f5311fba6..92b9d82e39e 100644
--- a/usr.sbin/ospfd/rde.c
+++ b/usr.sbin/ospfd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.102 2016/09/02 14:02:48 benno Exp $ */
+/*	$OpenBSD: rde.c,v 1.103 2016/09/03 10:22:57 renato Exp $ */
 
 /*
  * Copyright (c) 2004, 2005 Claudio Jeker <claudio@openbsd.org>
@@ -39,7 +39,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 *);
 void		 rde_dispatch_parent(int, short, void *);
 void		 rde_dump_area(struct area *, int, pid_t);
@@ -198,12 +198,18 @@ rde(struct ospfd_conf *xconf, int pipe_parent2rde[2], int pipe_ospfe2rde[2],
 	return (0);
 }
 
-void
+__dead void
 rde_shutdown(void)
 {
 	struct area	*a;
 	struct vertex	*v, *nv;
 
+	/* close pipes */
+	msgbuf_clear(&iev_ospfe->ibuf.w);
+	close(iev_ospfe->ibuf.fd);
+	msgbuf_clear(&iev_main->ibuf.w);
+	close(iev_main->ibuf.fd);
+
 	stop_spf_timer(rdeconf);
 	cand_list_clr();
 	rt_clear();
@@ -219,9 +225,7 @@ rde_shutdown(void)
 	rde_asext_free();
 	rde_nbr_free();
 
-	msgbuf_clear(&iev_ospfe->ibuf.w);
 	free(iev_ospfe);
-	msgbuf_clear(&iev_main->ibuf.w);
 	free(iev_main);
 	free(rdeconf);