From: renato Date: Mon, 8 Aug 2016 21:38:42 +0000 (+0000) Subject: Simplify shutdown process X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a80b52f6dbad5fd928c4ec3178307f6a16c9b0c3;p=openbsd 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. This "pipe teardown" removes a PID reuse race condition, makes the code simpler and allow us to remove "proc" from pledge. OK and tweaks from claudio@ --- diff --git a/usr.sbin/eigrpd/eigrpd.c b/usr.sbin/eigrpd/eigrpd.c index ec22c5fdac9..8fe08c35cdc 100644 --- a/usr.sbin/eigrpd/eigrpd.c +++ b/usr.sbin/eigrpd/eigrpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: eigrpd.c,v 1.16 2016/06/05 17:19:18 renato Exp $ */ +/* $OpenBSD: eigrpd.c,v 1.17 2016/08/08 21:38:42 renato Exp $ */ /* * Copyright (c) 2015 Renato Westphal @@ -40,9 +40,8 @@ void main_sig_handler(int, short, void *); __dead void usage(void); -void eigrpd_shutdown(void); +__dead void eigrpd_shutdown(void); pid_t start_child(enum eigrpd_process, char *, int, int, int, char *); -int check_child(pid_t, const char *); void main_dispatch_eigrpe(int, short, void *); void main_dispatch_rde(int, short, void *); @@ -65,29 +64,11 @@ 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(eigrpe_pid, "eigrp engine")) { - eigrpe_pid = 0; - die = 1; - } - if (check_child(rde_pid, "route decision engine")) { - rde_pid = 0; - die = 1; - } - if (die) - eigrpd_shutdown(); - break; + eigrpd_shutdown(); case SIGHUP: if (eigrp_reload() == -1) log_warnx("configuration reload failed"); @@ -116,7 +97,7 @@ struct eigrpd_global global; 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, rflag = 0, eflag = 0; @@ -248,11 +229,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,42 +266,41 @@ main(int argc, char *argv[]) eigrpd_conf->rdomain) == -1) fatalx("kr_init failed"); - if (pledge("inet rpath stdio proc sendfd", NULL) == -1) + if (pledge("inet rpath stdio sendfd", NULL) == -1) fatal("pledge"); event_dispatch(); eigrpd_shutdown(); - /* NOTREACHED */ - return (0); } -void +__dead void eigrpd_shutdown(void) { - pid_t pid; + pid_t pid; + int status; - if (eigrpe_pid) - kill(eigrpe_pid, SIGTERM); - - if (rde_pid) - kill(rde_pid, SIGTERM); + msgbuf_clear(&iev_eigrpe->ibuf.w); + free(iev_eigrpe); + iev_eigrpe = NULL; + msgbuf_clear(&iev_rde->ibuf.w); + free(iev_rde); + iev_rde = NULL; + config_clear(eigrpd_conf); kr_shutdown(); 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" : + "eigrp engine", WTERMSIG(status)); } while (pid != -1 || (pid == -1 && errno == EINTR)); - config_clear(eigrpd_conf); - - msgbuf_clear(&iev_eigrpe->ibuf.w); - free(iev_eigrpe); - msgbuf_clear(&iev_rde->ibuf.w); - free(iev_rde); - log_info("terminating"); exit(0); } @@ -373,26 +351,6 @@ start_child(enum eigrpd_process p, char *argv0, int fd, int debug, int verbose, fatal("execvp"); } -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 @@ -535,18 +493,20 @@ main_dispatch_rde(int fd, short event, void *bula) } } -void +int main_imsg_compose_eigrpe(int type, pid_t pid, void *data, uint16_t datalen) { if (iev_eigrpe == NULL) - return; - imsg_compose_event(iev_eigrpe, type, 0, pid, -1, data, datalen); + return (-1); + return (imsg_compose_event(iev_eigrpe, type, 0, pid, -1, data, datalen)); } -void +int main_imsg_compose_rde(int type, pid_t pid, void *data, uint16_t datalen) { - imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen); + if (iev_rde == NULL) + return (-1); + return (imsg_compose_event(iev_rde, type, 0, pid, -1, data, datalen)); } void @@ -660,9 +620,9 @@ eigrp_reload(void) int eigrp_sendboth(enum imsg_type type, void *buf, uint16_t len) { - if (imsg_compose_event(iev_eigrpe, type, 0, 0, -1, buf, len) == -1) + if (main_imsg_compose_eigrpe(type, 0, buf, len) == -1) return (-1); - if (imsg_compose_event(iev_rde, type, 0, 0, -1, buf, len) == -1) + if (main_imsg_compose_rde(type, 0, buf, len) == -1) return (-1); return (0); } diff --git a/usr.sbin/eigrpd/eigrpd.h b/usr.sbin/eigrpd/eigrpd.h index 41ddb49e90d..f0e88ff7b51 100644 --- a/usr.sbin/eigrpd/eigrpd.h +++ b/usr.sbin/eigrpd/eigrpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: eigrpd.h,v 1.15 2016/06/05 03:36:41 renato Exp $ */ +/* $OpenBSD: eigrpd.h,v 1.16 2016/08/08 21:38:42 renato Exp $ */ /* * Copyright (c) 2015 Renato Westphal @@ -481,8 +481,8 @@ void addscope(struct sockaddr_in6 *, uint32_t); void clearscope(struct in6_addr *); /* eigrpd.c */ -void main_imsg_compose_eigrpe(int, pid_t, void *, uint16_t); -void main_imsg_compose_rde(int, pid_t, void *, uint16_t); +int main_imsg_compose_eigrpe(int, pid_t, void *, uint16_t); +int main_imsg_compose_rde(int, pid_t, void *, uint16_t); void merge_config(struct eigrpd_conf *, struct eigrpd_conf *); struct eigrpd_conf *config_new_empty(void); void config_clear(struct eigrpd_conf *);