On Tue, Sep 06, 2016 at 11:31:04PM +0200, Eric Faurot wrote:
> Previously, all processes would shutdown on receiving SIGINT or SIGTERM.
> When going down, the parent process would kill all the other process and
> waitpid() them.
>
> Now, only the parent process handles SIGTERM and SIGINT, other processes
> ignore them. Upon receiving one of these signals, the parent process all
> imsg sockets and waitpid() for the children. It fatal()s if one of the
> imsg sockets is closed unexpectedly.
>
> Other processes exit() "normally" when one of their imsg socket is closed
> (except for client connection on the control socket of course). That's how
> they are supposed to stop now. When doing so, they log as "debug" instead
> of "info" because useless logs are useless.
>
> This makes the shutdown sequence much saner.
>
ok
> Index: ca.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/ca.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 ca.c
> --- ca.c 4 Sep 2016 16:10:31 -0000 1.24
> +++ ca.c 6 Sep 2016 19:33:45 -0000
> @@ -66,29 +66,14 @@ static uint64_t rsae_reqid = 0;
> static void
> ca_shutdown(void)
> {
> - log_info("info: ca agent exiting");
> + log_debug("debug: ca agent exiting");
> _exit(0);
> }
>
> -static void
> -ca_sig_handler(int sig, short event, void *p)
> -{
> - switch (sig) {
> - case SIGINT:
> - case SIGTERM:
> - ca_shutdown();
> - break;
> - default:
> - fatalx("ca_sig_handler: unexpected signal");
> - }
> -}
> -
> int
> ca(void)
> {
> struct passwd *pw;
> - struct event ev_sigint;
> - struct event ev_sigterm;
>
> purge_config(PURGE_LISTENERS|PURGE_TABLES|PURGE_RULES);
>
> @@ -110,10 +95,8 @@ ca(void)
> imsg_callback = ca_imsg;
> event_init();
>
> - signal_set(&ev_sigint, SIGINT, ca_sig_handler, NULL);
> - signal_set(&ev_sigterm, SIGTERM, ca_sig_handler, NULL);
> - signal_add(&ev_sigint, NULL);
> - signal_add(&ev_sigterm, NULL);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGTERM, SIG_IGN);
> signal(SIGPIPE, SIG_IGN);
> signal(SIGHUP, SIG_IGN);
>
> @@ -242,6 +225,9 @@ ca_imsg(struct mproc *p, struct imsg *im
> int ret = 0;
> uint64_t id;
> int v;
> +
> + if (imsg == NULL)
> + ca_shutdown();
>
> if (p->proc == PROC_PARENT) {
> switch (imsg->hdr.type) {
> Index: control.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/control.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 control.c
> --- control.c 4 Sep 2016 16:10:31 -0000 1.116
> +++ control.c 6 Sep 2016 19:33:45 -0000
> @@ -63,7 +63,6 @@ static void control_shutdown(void);
> static void control_listen(void);
> static void control_accept(int, short, void *);
> static void control_close(struct ctl_conn *);
> -static void control_sig_handler(int, short, void *);
> static void control_dispatch_ext(struct mproc *, struct imsg *);
> static void control_digest_update(const char *, size_t, int);
> static void control_broadcast_verbose(int, int);
> @@ -89,6 +88,12 @@ control_imsg(struct mproc *p, struct ims
> const void *data;
> size_t sz;
>
> + if (imsg == NULL) {
> + if (p->proc != PROC_CLIENT)
> + control_shutdown();
> + return;
> + }
> +
> if (p->proc == PROC_PONY) {
> switch (imsg->hdr.type) {
> case IMSG_CTL_SMTP_SESSION:
> @@ -186,19 +191,6 @@ control_imsg(struct mproc *p, struct ims
> imsg_to_str(imsg->hdr.type));
> }
>
> -static void
> -control_sig_handler(int sig, short event, void *p)
> -{
> - switch (sig) {
> - case SIGINT:
> - case SIGTERM:
> - control_shutdown();
> - break;
> - default:
> - fatalx("control_sig_handler: unexpected signal");
> - }
> -}
> -
> int
> control_create_socket(void)
> {
> @@ -245,8 +237,6 @@ int
> control(void)
> {
> struct passwd *pw;
> - struct event ev_sigint;
> - struct event ev_sigterm;
>
> purge_config(PURGE_EVERYTHING);
>
> @@ -271,10 +261,8 @@ control(void)
> imsg_callback = control_imsg;
> event_init();
>
> - signal_set(&ev_sigint, SIGINT, control_sig_handler, NULL);
> - signal_set(&ev_sigterm, SIGTERM, control_sig_handler, NULL);
> - signal_add(&ev_sigint, NULL);
> - signal_add(&ev_sigterm, NULL);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGTERM, SIG_IGN);
> signal(SIGPIPE, SIG_IGN);
> signal(SIGHUP, SIG_IGN);
>
> @@ -305,7 +293,7 @@ control(void)
> static void
> control_shutdown(void)
> {
> - log_info("info: control process exiting");
> + log_debug("debug: control agent exiting");
> _exit(0);
> }
>
> Index: lka.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.196
> diff -u -p -r1.196 lka.c
> --- lka.c 4 Sep 2016 16:10:31 -0000 1.196
> +++ lka.c 6 Sep 2016 19:33:46 -0000
> @@ -83,6 +83,9 @@ lka_imsg(struct mproc *p, struct imsg *i
> uint64_t reqid;
> int v;
>
> + if (imsg == NULL)
> + lka_shutdown();
> +
> if (imsg->hdr.type == IMSG_MTA_DNS_HOST ||
> imsg->hdr.type == IMSG_MTA_DNS_PTR ||
> imsg->hdr.type == IMSG_SMTP_DNS_PTR ||
> @@ -383,10 +386,6 @@ lka_sig_handler(int sig, short event, vo
> pid_t pid;
>
> switch (sig) {
> - case SIGINT:
> - case SIGTERM:
> - lka_shutdown();
> - break;
> case SIGCHLD:
> do {
> pid = waitpid(-1, &status, WNOHANG);
> @@ -400,7 +399,7 @@ lka_sig_handler(int sig, short event, vo
> void
> lka_shutdown(void)
> {
> - log_info("info: lookup agent exiting");
> + log_debug("debug: lookup agent exiting");
> _exit(0);
> }
>
> @@ -408,8 +407,6 @@ int
> lka(void)
> {
> struct passwd *pw;
> - struct event ev_sigint;
> - struct event ev_sigterm;
> struct event ev_sigchld;
>
> purge_config(PURGE_LISTENERS);
> @@ -427,12 +424,10 @@ lka(void)
> imsg_callback = lka_imsg;
> event_init();
>
> - signal_set(&ev_sigint, SIGINT, lka_sig_handler, NULL);
> - signal_set(&ev_sigterm, SIGTERM, lka_sig_handler, NULL);
> signal_set(&ev_sigchld, SIGCHLD, lka_sig_handler, NULL);
> - signal_add(&ev_sigint, NULL);
> - signal_add(&ev_sigterm, NULL);
> signal_add(&ev_sigchld, NULL);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGTERM, SIG_IGN);
> signal(SIGPIPE, SIG_IGN);
> signal(SIGHUP, SIG_IGN);
>
> Index: mproc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/mproc.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 mproc.c
> --- mproc.c 3 Sep 2016 16:06:26 -0000 1.26
> +++ mproc.c 6 Sep 2016 19:33:46 -0000
> @@ -88,6 +88,8 @@ mproc_init(struct mproc *p, int fd)
> void
> mproc_clear(struct mproc *p)
> {
> + log_debug("debug: clearing p=%s, fd=%d, pid=%d", p->name,
> p->imsgbuf.fd, p->pid);
> +
> event_del(&p->ev);
> close(p->imsgbuf.fd);
> imsg_clear(&p->imsgbuf);
> @@ -166,10 +168,8 @@ mproc_dispatch(int fd, short event, void
> /* NOTREACHED */
> case 0:
> /* this pipe is dead, so remove the event handler */
> - if (smtpd_process != PROC_CONTROL ||
> - p->proc != PROC_CLIENT)
> - log_warnx("warn: %s -> %s: pipe closed",
> - proc_name(smtpd_process), p->name);
> + log_debug("debug: %s -> %s: pipe closed",
> + proc_name(smtpd_process), p->name);
> p->handler(p, NULL);
> return;
> default:
> @@ -181,10 +181,8 @@ mproc_dispatch(int fd, short event, void
> n = msgbuf_write(&p->imsgbuf.w);
> if (n == 0 || (n == -1 && errno != EAGAIN)) {
> /* this pipe is dead, so remove the event handler */
> - if (smtpd_process != PROC_CONTROL ||
> - p->proc != PROC_CLIENT)
> - log_warnx("warn: %s -> %s: pipe closed",
> - proc_name(smtpd_process), p->name);
> + log_debug("debug: %s -> %s: pipe closed",
> + proc_name(smtpd_process), p->name);
> p->handler(p, NULL);
> return;
> }
> Index: pony.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/pony.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 pony.c
> --- pony.c 4 Sep 2016 16:10:31 -0000 1.15
> +++ pony.c 6 Sep 2016 19:33:46 -0000
> @@ -46,7 +46,6 @@ void mta_imsg(struct mproc *, struct ims
> void smtp_imsg(struct mproc *, struct imsg *);
>
> static void pony_shutdown(void);
> -static void pony_sig_handler(int, short, void *);
>
> void
> pony_imsg(struct mproc *p, struct imsg *imsg)
> @@ -54,6 +53,9 @@ pony_imsg(struct mproc *p, struct imsg *
> struct msg m;
> int v;
>
> + if (imsg == NULL)
> + pony_shutdown();
> +
> switch (imsg->hdr.type) {
> case IMSG_CONF_START:
> return;
> @@ -132,22 +134,9 @@ pony_imsg(struct mproc *p, struct imsg *
> }
>
> static void
> -pony_sig_handler(int sig, short event, void *p)
> -{
> - switch (sig) {
> - case SIGINT:
> - case SIGTERM:
> - pony_shutdown();
> - break;
> - default:
> - fatalx("pony_sig_handler: unexpected signal");
> - }
> -}
> -
> -static void
> pony_shutdown(void)
> {
> - log_info("info: pony agent exiting");
> + log_debug("debug: pony agent exiting");
> _exit(0);
> }
>
> @@ -155,8 +144,6 @@ int
> pony(void)
> {
> struct passwd *pw;
> - struct event ev_sigint;
> - struct event ev_sigterm;
>
> mda_postfork();
> mta_postfork();
> @@ -191,10 +178,8 @@ pony(void)
> mta_postprivdrop();
> smtp_postprivdrop();
>
> - signal_set(&ev_sigint, SIGINT, pony_sig_handler, NULL);
> - signal_set(&ev_sigterm, SIGTERM, pony_sig_handler, NULL);
> - signal_add(&ev_sigint, NULL);
> - signal_add(&ev_sigterm, NULL);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGTERM, SIG_IGN);
> signal(SIGPIPE, SIG_IGN);
> signal(SIGHUP, SIG_IGN);
>
> Index: queue.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/queue.c,v
> retrieving revision 1.181
> diff -u -p -r1.181 queue.c
> --- queue.c 4 Sep 2016 16:10:31 -0000 1.181
> +++ queue.c 6 Sep 2016 19:33:47 -0000
> @@ -45,7 +45,6 @@ static void queue_imsg(struct mproc *, s
> static void queue_timeout(int, short, void *);
> static void queue_bounce(struct envelope *, struct delivery_bounce *);
> static void queue_shutdown(void);
> -static void queue_sig_handler(int, short, void *);
> static void queue_log(const struct envelope *, const char *, const char *);
> static void queue_msgid_walk(int, short, void *);
>
> @@ -66,6 +65,9 @@ queue_imsg(struct mproc *p, struct imsg
> size_t n_evp;
> int fd, mta_ext, ret, v, flags, code;
>
> + if (imsg == NULL)
> + queue_shutdown();
> +
> memset(&bounce, 0, sizeof(struct delivery_bounce));
> if (p->proc == PROC_PONY) {
>
> @@ -636,22 +638,9 @@ queue_bounce(struct envelope *e, struct
> }
>
> static void
> -queue_sig_handler(int sig, short event, void *p)
> -{
> - switch (sig) {
> - case SIGINT:
> - case SIGTERM:
> - queue_shutdown();
> - break;
> - default:
> - fatalx("queue_sig_handler: unexpected signal");
> - }
> -}
> -
> -static void
> queue_shutdown(void)
> {
> - log_info("info: queue handler exiting");
> + log_debug("debug: queue agent exiting");
> queue_close();
> _exit(0);
> }
> @@ -662,8 +651,6 @@ queue(void)
> struct passwd *pw;
> struct timeval tv;
> struct event ev_qload;
> - struct event ev_sigint;
> - struct event ev_sigterm;
>
> purge_config(PURGE_EVERYTHING);
>
> @@ -698,10 +685,8 @@ queue(void)
> imsg_callback = queue_imsg;
> event_init();
>
> - signal_set(&ev_sigint, SIGINT, queue_sig_handler, NULL);
> - signal_set(&ev_sigterm, SIGTERM, queue_sig_handler, NULL);
> - signal_add(&ev_sigint, NULL);
> - signal_add(&ev_sigterm, NULL);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGTERM, SIG_IGN);
> signal(SIGPIPE, SIG_IGN);
> signal(SIGHUP, SIG_IGN);
>
> Index: scheduler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/scheduler.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 scheduler.c
> --- scheduler.c 4 Sep 2016 16:10:31 -0000 1.54
> +++ scheduler.c 6 Sep 2016 19:33:47 -0000
> @@ -47,7 +47,6 @@
>
> static void scheduler_imsg(struct mproc *, struct imsg *);
> static void scheduler_shutdown(void);
> -static void scheduler_sig_handler(int, short, void *);
> static void scheduler_reset_events(void);
> static void scheduler_timeout(int, short, void *);
>
> @@ -75,6 +74,9 @@ scheduler_imsg(struct mproc *p, struct i
> time_t timestamp;
> int v, r, type;
>
> + if (imsg == NULL)
> + scheduler_shutdown();
> +
> switch (imsg->hdr.type) {
>
> case IMSG_QUEUE_ENVELOPE_SUBMIT:
> @@ -404,22 +406,9 @@ scheduler_imsg(struct mproc *p, struct i
> }
>
> static void
> -scheduler_sig_handler(int sig, short event, void *p)
> -{
> - switch (sig) {
> - case SIGINT:
> - case SIGTERM:
> - scheduler_shutdown();
> - break;
> - default:
> - fatalx("scheduler_sig_handler: unexpected signal");
> - }
> -}
> -
> -static void
> scheduler_shutdown(void)
> {
> - log_info("info: scheduler handler exiting");
> + log_debug("debug: scheduler agent exiting");
> _exit(0);
> }
>
> @@ -438,8 +427,6 @@ int
> scheduler(void)
> {
> struct passwd *pw;
> - struct event ev_sigint;
> - struct event ev_sigterm;
>
> backend = scheduler_backend_lookup(backend_scheduler);
> if (backend == NULL)
> @@ -473,10 +460,8 @@ scheduler(void)
> imsg_callback = scheduler_imsg;
> event_init();
>
> - signal_set(&ev_sigint, SIGINT, scheduler_sig_handler, NULL);
> - signal_set(&ev_sigterm, SIGTERM, scheduler_sig_handler, NULL);
> - signal_add(&ev_sigint, NULL);
> - signal_add(&ev_sigterm, NULL);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGTERM, SIG_IGN);
> signal(SIGPIPE, SIG_IGN);
> signal(SIGHUP, SIG_IGN);
>
> Index: smtpd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
> retrieving revision 1.285
> diff -u -p -r1.285 smtpd.c
> --- smtpd.c 6 Sep 2016 16:34:29 -0000 1.285
> +++ smtpd.c 6 Sep 2016 19:33:48 -0000
> @@ -60,7 +60,7 @@
> static void parent_imsg(struct mproc *, struct imsg *);
> static void usage(void);
> static int smtpd(void);
> -static void parent_shutdown(int);
> +static void parent_shutdown(void);
> static void parent_send_config(int, short, void *);
> static void parent_send_config_lka(void);
> static void parent_send_config_pony(void);
> @@ -162,8 +162,8 @@ parent_imsg(struct mproc *p, struct imsg
> int fd, n, v, ret;
>
> if (imsg == NULL)
> - exit(1);
> -
> + fatalx("process %s socket closed", p->name);
> +
> if (p->proc == PROC_LKA) {
> switch (imsg->hdr.type) {
> case IMSG_LKA_OPEN_FORWARD:
> @@ -275,16 +275,16 @@ usage(void)
> }
>
> static void
> -parent_shutdown(int ret)
> +parent_shutdown(void)
> {
> - void *iter;
> - struct child *child;
> - pid_t pid;
> + pid_t pid;
>
> - iter = NULL;
> - while (tree_iter(&children, &iter, NULL, (void**)&child))
> - if (child->type == CHILD_DAEMON)
> - kill(child->pid, SIGTERM);
> + mproc_clear(p_ca);
> + mproc_clear(p_pony);
> + mproc_clear(p_control);
> + mproc_clear(p_lka);
> + mproc_clear(p_scheduler);
> + mproc_clear(p_queue);
>
> do {
> pid = waitpid(WAIT_MYPGRP, NULL, 0);
> @@ -292,8 +292,8 @@ parent_shutdown(int ret)
>
> unlink(SMTPD_SOCKET);
>
> - log_warnx("warn: parent terminating");
> - exit(ret);
> + log_info("Exiting");
> + exit(0);
> }
>
> static void
> @@ -333,16 +333,17 @@ static void
> parent_sig_handler(int sig, short event, void *p)
> {
> struct child *child;
> - int die = 0, die_gracefully = 0, status, fail;
> + int status, fail;
> pid_t pid;
> char *cause;
>
> switch (sig) {
> case SIGTERM:
> case SIGINT:
> - log_info("info: %s, shutting down", strsignal(sig));
> - die_gracefully = 1;
> - /* FALLTHROUGH */
> + log_debug("debug: got signal %d", sig);
> + parent_shutdown();
> + /* NOT REACHED */
> +
> case SIGCHLD:
> do {
> int len;
> @@ -379,7 +380,6 @@ parent_sig_handler(int sig, short event,
>
> switch (child->type) {
> case CHILD_DAEMON:
> - die = 1;
> if (fail)
> log_warnx("warn: lost child: %s %s",
> child->title, cause);
> @@ -434,10 +434,6 @@ parent_sig_handler(int sig, short event,
> free(cause);
> } while (pid > 0 || (pid == -1 && errno == EINTR));
>
> - if (die)
> - parent_shutdown(1);
> - else if (die_gracefully)
> - parent_shutdown(0);
> break;
> default:
> fatalx("smtpd: unexpected signal");
> @@ -1597,7 +1593,7 @@ imsg_dispatch(struct mproc *p, struct im
> int msg;
>
> if (imsg == NULL) {
> - exit(1);
> + imsg_callback(p, imsg);
> return;
> }
>
>
--
Gilles Chehade
https://www.poolp.org @poolpOrg
