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 claudio@

3 files changed, 43 insertions, 71 deletions

diff --git a/usr.sbin/ldpd/lde.c b/usr.sbin/ldpd/lde.c
index 94f4adaf2b6..84f5565941a 100644
--- a/usr.sbin/ldpd/lde.c
+++ b/usr.sbin/ldpd/lde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: lde.c,v 1.64 2016/08/08 21:42:13 renato Exp $ */
+/*	$OpenBSD: lde.c,v 1.65 2016/09/02 17:03:24 renato Exp $ */
 
 /*
  * Copyright (c) 2013, 2016 Renato Westphal <renato@openbsd.org>
@@ -40,7 +40,7 @@
 #include "lde.h"
 
 static void		 lde_sig_handler(int sig, short, void *);
-static void		 lde_shutdown(void);
+static __dead void	 lde_shutdown(void);
 static int		 lde_imsg_compose_parent(int, pid_t, void *, uint16_t);
 static void		 lde_dispatch_imsg(int, short, void *);
 static void		 lde_dispatch_parent(int, short, void *);
@@ -148,18 +148,22 @@ lde(int debug, int verbose)
 	return (0);
 }
 
-static void
+static __dead void
 lde_shutdown(void)
 {
+	/* close pipes */
+	msgbuf_clear(&iev_ldpe->ibuf.w);
+	close(iev_ldpe->ibuf.fd);
+	msgbuf_clear(&iev_main->ibuf.w);
+	close(iev_main->ibuf.fd);
+
 	lde_gc_stop_timer();
 	lde_nbr_clear();
 	fec_tree_clear();
 
 	config_clear(ldeconf);
 
-	msgbuf_clear(&iev_ldpe->ibuf.w);
 	free(iev_ldpe);
-	msgbuf_clear(&iev_main->ibuf.w);
 	free(iev_main);
 
 	log_info("label decision engine exiting");
diff --git a/usr.sbin/ldpd/ldpd.c b/usr.sbin/ldpd/ldpd.c
index 48f98ad91c2..86f31752d60 100644
--- a/usr.sbin/ldpd/ldpd.c
+++ b/usr.sbin/ldpd/ldpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ldpd.c,v 1.57 2016/07/15 17:03:10 renato Exp $ */
+/*	$OpenBSD: ldpd.c,v 1.58 2016/09/02 17:03:24 renato Exp $ */
 
 /*
  * Copyright (c) 2013, 2016 Renato Westphal <renato@openbsd.org>
@@ -37,9 +37,8 @@
 
 static void		 main_sig_handler(int, short, void *);
 static __dead void	 usage(void);
-static void		 ldpd_shutdown(void);
+static __dead void	 ldpd_shutdown(void);
 static pid_t		 start_child(enum ldpd_process, char *, int, int, int);
-static int		 check_child(pid_t, const char *);
 static void		 main_dispatch_ldpe(int, short, void *);
 static void		 main_dispatch_lde(int, short, void *);
 static int		 main_imsg_compose_both(enum imsg_type, void *,
@@ -74,29 +73,12 @@ static pid_t		 lde_pid;
 static 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(ldpe_pid, "ldp engine")) {
-			ldpe_pid = 0;
-			die = 1;
-		}
-		if (check_child(lde_pid, "label decision engine")) {
-			lde_pid = 0;
-			die = 1;
-		}
-		if (die)
-			ldpd_shutdown();
-		break;
+		ldpd_shutdown();
+		/* NOTREACHED */
 	case SIGHUP:
 		if (ldp_reload() == -1)
 			log_warnx("configuration reload failed");
@@ -122,7 +104,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			*saved_argv0;
 	int			 ch;
 	int			 debug = 0, lflag = 0, eflag = 0;
@@ -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);
 
@@ -287,30 +267,34 @@ main(int argc, char *argv[])
 	return (0);
 }
 
-static void
+static __dead void
 ldpd_shutdown(void)
 {
 	pid_t		 pid;
+	int		 status;
 
-	if (ldpe_pid)
-		kill(ldpe_pid, SIGTERM);
-
-	if (lde_pid)
-		kill(lde_pid, SIGTERM);
+	/* close pipes */
+	msgbuf_clear(&iev_ldpe->ibuf.w);
+	close(iev_ldpe->ibuf.fd);
+	msgbuf_clear(&iev_lde->ibuf.w);
+	close(iev_lde->ibuf.fd);
 
 	kr_shutdown();
+	config_clear(ldpd_conf);
 
+	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 == lde_pid) ? "label decision engine" :
+			    "ldp engine", WTERMSIG(status));
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
 
-	config_clear(ldpd_conf);
-
-	msgbuf_clear(&iev_ldpe->ibuf.w);
 	free(iev_ldpe);
-	msgbuf_clear(&iev_lde->ibuf.w);
 	free(iev_lde);
 
 	log_info("terminating");
@@ -358,26 +342,6 @@ start_child(enum ldpd_process p, char *argv0, int fd, int debug, int verbose)
 	fatal("execvp");
 }
 
-static 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 */
 static void
diff --git a/usr.sbin/ldpd/ldpe.c b/usr.sbin/ldpd/ldpe.c
index 8fb724b2b09..4169f988b5d 100644
--- a/usr.sbin/ldpd/ldpe.c
+++ b/usr.sbin/ldpd/ldpe.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ldpe.c,v 1.68 2016/08/08 21:42:13 renato Exp $ */
+/*	$OpenBSD: ldpe.c,v 1.69 2016/09/02 17:03:24 renato Exp $ */
 
 /*
  * Copyright (c) 2013, 2016 Renato Westphal <renato@openbsd.org>
@@ -35,7 +35,7 @@
 #include "log.h"
 
 static void	 ldpe_sig_handler(int, short, void *);
-static void	 ldpe_shutdown(void);
+static __dead void ldpe_shutdown(void);
 static void	 ldpe_dispatch_main(int, short, void *);
 static void	 ldpe_dispatch_lde(int, short, void *);
 static void	 ldpe_dispatch_pfkey(int, short, void *);
@@ -157,12 +157,20 @@ ldpe(int debug, int verbose)
 	return (0);
 }
 
-static void
+static __dead void
 ldpe_shutdown(void)
 {
 	struct if_addr		*if_addr;
 	struct adj		*adj;
 
+	/* close pipes */
+	msgbuf_write(&iev_lde->ibuf.w);
+	msgbuf_clear(&iev_lde->ibuf.w);
+	close(iev_lde->ibuf.fd);
+	msgbuf_write(&iev_main->ibuf.w);
+	msgbuf_clear(&iev_main->ibuf.w);
+	close(iev_main->ibuf.fd);
+
 	control_cleanup();
 	config_clear(leconf);
 
@@ -182,11 +190,7 @@ ldpe_shutdown(void)
 		adj_del(adj, S_SHUTDOWN);
 
 	/* clean up */
-	msgbuf_write(&iev_lde->ibuf.w);
-	msgbuf_clear(&iev_lde->ibuf.w);
 	free(iev_lde);
-	msgbuf_write(&iev_main->ibuf.w);
-	msgbuf_clear(&iev_main->ibuf.w);
 	free(iev_main);
 	free(pkt_ptr);