From dc6f687483a94c8831397116fc342ac65d51e44e Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Fri, 2 Sep 2016 15:38:09 +0000 Subject: 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@ --- usr.sbin/dvmrpd/dvmrpd.c | 80 +++++++++++++----------------------------------- usr.sbin/dvmrpd/dvmrpe.c | 20 +++++++----- usr.sbin/dvmrpd/rde.c | 15 +++++---- 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 @@ -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 @@ -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 @@ -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); -- cgit v1.2.3