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@

1 files changed, 22 insertions, 57 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