Nicholas Marriott <nicholas.marri...@gmail.com> writes: > I wonder if to make it nicer we could unify the signal set > functions into one set_signals(void (*)(int, short, void *)). And > similarly could have one clear_signals function.
> All the processes could ignore SIGINT, SIGPIPE, SIGUSR2, SIGTSTP, > SIGHUP, and all catch SIGCONT, SIGTERM, SIGWINCH, SIGCHLD, > SIGUSR1, then the actual handler could just ignore the ones not > wanted. Handling a signal and doing nothing with it is different than using SIG_IGN, in particular for SIGCHLD using SIG_IGN enables automatic reaping of zombies (at least on Linux). That being said, your approach is indeed cleaner so I implemented it (patch follows), adding a wait() call in the main handler to fix the original zombie issue. Index: client.c =================================================================== RCS file: /cvsroot/tmux/tmux/client.c,v retrieving revision 1.90 diff -u -p -r1.90 client.c --- client.c 4 Dec 2009 22:14:47 -0000 1.90 +++ client.c 28 Apr 2010 21:46:16 -0000 @@ -176,35 +176,12 @@ client_update_event(void) __dead void client_main(void) { - struct event ev_sigcont, ev_sigterm, ev_sigwinch; - struct sigaction sigact; - logfile("client"); /* Note: event_init() has already been called. */ /* Set up signals. */ - memset(&sigact, 0, sizeof sigact); - sigemptyset(&sigact.sa_mask); - sigact.sa_flags = SA_RESTART; - sigact.sa_handler = SIG_IGN; - if (sigaction(SIGINT, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGPIPE, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR1, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR2, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGTSTP, &sigact, NULL) != 0) - fatal("sigaction failed"); - - signal_set(&ev_sigcont, SIGCONT, client_signal, NULL); - signal_add(&ev_sigcont, NULL); - signal_set(&ev_sigterm, SIGTERM, client_signal, NULL); - signal_add(&ev_sigterm, NULL); - signal_set(&ev_sigwinch, SIGWINCH, client_signal, NULL); - signal_add(&ev_sigwinch, NULL); + set_signals(client_signal); /* * imsg_read in the first client poll loop (before the terminal has @@ -250,6 +227,9 @@ client_signal(int sig, unused short even fatal("sigaction failed"); client_write_server(MSG_WAKEUP, NULL, 0); break; + default: + /* signal not handled here, ignore. */ + return; } client_update_event(); Index: cmd-pipe-pane.c =================================================================== RCS file: /cvsroot/tmux/tmux/cmd-pipe-pane.c,v retrieving revision 1.10 diff -u -p -r1.10 cmd-pipe-pane.c --- cmd-pipe-pane.c 4 Dec 2009 22:14:47 -0000 1.10 +++ cmd-pipe-pane.c 28 Apr 2010 21:46:16 -0000 @@ -90,7 +90,7 @@ cmd_pipe_pane_exec(struct cmd *self, str case 0: /* Child process. */ close(pipe_fd[0]); - server_signal_clear(); + clear_signals(); if (dup2(pipe_fd[1], STDIN_FILENO) == -1) _exit(1); Index: job.c =================================================================== RCS file: /cvsroot/tmux/tmux/job.c,v retrieving revision 1.16 diff -u -p -r1.16 job.c --- job.c 6 Apr 2010 21:59:19 -0000 1.16 +++ job.c 28 Apr 2010 21:46:16 -0000 @@ -148,7 +148,7 @@ job_run(struct job *job) case -1: return (-1); case 0: /* child */ - server_signal_clear(); + clear_signals(); environ_push(&global_environ); Index: server.c =================================================================== RCS file: /cvsroot/tmux/tmux/server.c,v retrieving revision 1.239 diff -u -p -r1.239 server.c --- server.c 8 Apr 2010 07:54:43 -0000 1.239 +++ server.c 28 Apr 2010 21:46:16 -0000 @@ -48,9 +48,6 @@ struct clients dead_clients; int server_fd; int server_shutdown; struct event server_ev_accept; -struct event server_ev_sigterm; -struct event server_ev_sigusr1; -struct event server_ev_sigchld; struct event server_ev_second; int server_create_socket(void); @@ -136,6 +133,12 @@ server_start(char *path) } close(pair[0]); + /* event_init() was called in our parent, we need to reinit. */ + if (event_reinit(ev_base) != 0) + fatal("event_reinit failed"); + + clear_signals(); + /* * Must daemonise before loading configuration as the PID changes so * $TMUX would be wrong for sessions created in the config file. @@ -166,22 +169,6 @@ server_start(char *path) setproctitle("server (%s)", rpathbuf); #endif -#ifdef HAVE_BROKEN_KQUEUE - if (setenv("EVENT_NOKQUEUE", "1", 1) != 0) - fatal("setenv failed"); -#endif -#ifdef HAVE_BROKEN_POLL - if (setenv("EVENT_NOPOLL", "1", 1) != 0) - fatal("setenv failed"); -#endif - event_init(); -#ifdef HAVE_BROKEN_KQUEUE - unsetenv("EVENT_NOKQUEUE"); -#endif -#ifdef HAVE_BROKEN_POLL - unsetenv("EVENT_NOPOLL"); -#endif - server_fd = server_create_socket(); server_client_create(pair[1]); @@ -220,7 +207,7 @@ server_start(char *path) evtimer_set(&server_ev_second, server_second_callback, NULL); evtimer_add(&server_ev_second, &tv); - server_signal_set(); + set_signals(server_signal_callback); server_loop(); exit(0); } @@ -359,61 +346,6 @@ server_accept_callback(int fd, short eve server_client_create(newfd); } -/* Set up server signal handling. */ -void -server_signal_set(void) -{ - struct sigaction sigact; - - memset(&sigact, 0, sizeof sigact); - sigemptyset(&sigact.sa_mask); - sigact.sa_flags = SA_RESTART; - sigact.sa_handler = SIG_IGN; - if (sigaction(SIGINT, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGPIPE, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR2, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGTSTP, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGHUP, &sigact, NULL) != 0) - fatal("sigaction failed"); - - signal_set(&server_ev_sigchld, SIGCHLD, server_signal_callback, NULL); - signal_add(&server_ev_sigchld, NULL); - signal_set(&server_ev_sigterm, SIGTERM, server_signal_callback, NULL); - signal_add(&server_ev_sigterm, NULL); - signal_set(&server_ev_sigusr1, SIGUSR1, server_signal_callback, NULL); - signal_add(&server_ev_sigusr1, NULL); -} - -/* Destroy server signal events. */ -void -server_signal_clear(void) -{ - struct sigaction sigact; - - memset(&sigact, 0, sizeof sigact); - sigemptyset(&sigact.sa_mask); - sigact.sa_flags = SA_RESTART; - sigact.sa_handler = SIG_DFL; - if (sigaction(SIGINT, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGPIPE, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR2, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGTSTP, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGHUP, &sigact, NULL) != 0) - fatal("sigaction failed"); - - signal_del(&server_ev_sigchld); - signal_del(&server_ev_sigterm); - signal_del(&server_ev_sigusr1); -} - /* Signal handler. */ /* ARGSUSED */ void @@ -435,6 +367,9 @@ server_signal_callback(int sig, unused s EV_READ|EV_PERSIST, server_accept_callback, NULL); event_add(&server_ev_accept, NULL); break; + default: + /* signal not handled here, ignore. */ + return; } } Index: signal.c =================================================================== RCS file: signal.c diff -N signal.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ signal.c 28 Apr 2010 21:46:16 -0000 @@ -0,0 +1,88 @@ +/* $Id$ */ + +/* + * Copyright (c) 2007 Nicholas Marriott <n...@users.sourceforge.net> + * Copyright (c) 2010 Romain Francoise <rfranco...@debian.org> + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF MIND, USE, DATA OR PROFITS, WHETHER + * IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING + * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include <string.h> +#include <signal.h> + +#include "tmux.h" + +struct event ev_sigchld; +struct event ev_sigcont; +struct event ev_sigterm; +struct event ev_sigusr1; +struct event ev_sigwinch; + +void +set_signals(void(*handler)(int, short, unused void *)) +{ + struct sigaction sigact; + + memset(&sigact, 0, sizeof sigact); + sigemptyset(&sigact.sa_mask); + sigact.sa_flags = SA_RESTART; + sigact.sa_handler = SIG_IGN; + if (sigaction(SIGINT, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGPIPE, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGUSR2, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGTSTP, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGHUP, &sigact, NULL) != 0) + fatal("sigaction failed"); + + signal_set(&ev_sigchld, SIGCHLD, handler, NULL); + signal_add(&ev_sigchld, NULL); + signal_set(&ev_sigcont, SIGCONT, handler, NULL); + signal_add(&ev_sigcont, NULL); + signal_set(&ev_sigterm, SIGTERM, handler, NULL); + signal_add(&ev_sigterm, NULL); + signal_set(&ev_sigusr1, SIGUSR1, handler, NULL); + signal_add(&ev_sigusr1, NULL); + signal_set(&ev_sigwinch, SIGWINCH, handler, NULL); + signal_add(&ev_sigwinch, NULL); +} + +void +clear_signals(void) +{ + struct sigaction sigact; + + memset(&sigact, 0, sizeof sigact); + sigemptyset(&sigact.sa_mask); + sigact.sa_flags = SA_RESTART; + sigact.sa_handler = SIG_DFL; + if (sigaction(SIGINT, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGPIPE, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGUSR2, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGTSTP, &sigact, NULL) != 0) + fatal("sigaction failed"); + if (sigaction(SIGHUP, &sigact, NULL) != 0) + fatal("sigaction failed"); + + event_del(&ev_sigchld); + event_del(&ev_sigcont); + event_del(&ev_sigterm); + event_del(&ev_sigusr1); + event_del(&ev_sigwinch); +} Index: tmux.c =================================================================== RCS file: /cvsroot/tmux/tmux/tmux.c,v retrieving revision 1.205 diff -u -p -r1.205 tmux.c --- tmux.c 22 Apr 2010 21:48:49 -0000 1.205 +++ tmux.c 28 Apr 2010 21:46:16 -0000 @@ -18,6 +18,7 @@ #include <sys/types.h> #include <sys/stat.h> +#include <sys/wait.h> #include <errno.h> #include <event.h> @@ -40,6 +41,8 @@ struct options global_s_options; /* ses struct options global_w_options; /* window options */ struct environ global_environ; +struct event_base *ev_base; + int debug_level; time_t start_time; char *socket_path; @@ -57,11 +60,8 @@ char *makesockpath(const char *); __dead void shell_exec(const char *, const char *); struct imsgbuf *main_ibuf; -struct event main_ev_sigterm; int main_exitval; -void main_set_signals(void); -void main_clear_signals(void); void main_signal(int, short, unused void *); void main_callback(int, short, void *); void main_dispatch(const char *); @@ -241,7 +241,6 @@ main(int argc, char **argv) struct keylist *keylist; struct env_data envdata; struct msg_command_data cmddata; - struct sigaction sigact; char *s, *shellcmd, *path, *label, *home, *cause; char cwd[MAXPATHLEN], **var; void *buf; @@ -541,18 +540,6 @@ main(int argc, char **argv) exit(1); } - /* Catch SIGCHLD to avoid a zombie when starting the server. */ - memset(&sigact, 0, sizeof sigact); - sigemptyset(&sigact.sa_mask); - sigact.sa_handler = SIG_IGN; - if (sigaction(SIGCHLD, &sigact, NULL) != 0) - fatal("sigaction failed"); - - /* Initialise the client socket/start the server. */ - if ((main_ibuf = client_init(path, cmdflags, flags)) == NULL) - exit(1); - xfree(path); - #ifdef HAVE_BROKEN_KQUEUE if (setenv("EVENT_NOKQUEUE", "1", 1) != 0) fatal("setenv failed"); @@ -561,7 +548,7 @@ main(int argc, char **argv) if (setenv("EVENT_NOPOLL", "1", 1) != 0) fatal("setenv failed"); #endif - event_init(); + ev_base = event_init(); #ifdef HAVE_BROKEN_KQUEUE unsetenv("EVENT_NOKQUEUE"); #endif @@ -569,9 +556,14 @@ main(int argc, char **argv) unsetenv("EVENT_NOPOLL"); #endif - imsg_compose(main_ibuf, msg, PROTOCOL_VERSION, -1, -1, buf, len); + set_signals(main_signal); - main_set_signals(); + /* Initialise the client socket/start the server. */ + if ((main_ibuf = client_init(path, cmdflags, flags)) == NULL) + exit(1); + xfree(path); + + imsg_compose(main_ibuf, msg, PROTOCOL_VERSION, -1, -1, buf, len); events = EV_READ; if (main_ibuf->w.queued > 0) @@ -581,65 +573,26 @@ main(int argc, char **argv) main_exitval = 0; event_dispatch(); - main_clear_signals(); + clear_signals(); client_main(); /* doesn't return */ } -void -main_set_signals(void) -{ - struct sigaction sigact; - - memset(&sigact, 0, sizeof sigact); - sigemptyset(&sigact.sa_mask); - sigact.sa_flags = SA_RESTART; - sigact.sa_handler = SIG_IGN; - if (sigaction(SIGINT, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGPIPE, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR1, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR2, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGTSTP, &sigact, NULL) != 0) - fatal("sigaction failed"); - - signal_set(&main_ev_sigterm, SIGTERM, main_signal, NULL); - signal_add(&main_ev_sigterm, NULL); -} - -void -main_clear_signals(void) -{ - struct sigaction sigact; - - memset(&sigact, 0, sizeof sigact); - sigemptyset(&sigact.sa_mask); - sigact.sa_flags = SA_RESTART; - sigact.sa_handler = SIG_DFL; - if (sigaction(SIGINT, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGPIPE, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR1, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGUSR2, &sigact, NULL) != 0) - fatal("sigaction failed"); - if (sigaction(SIGTSTP, &sigact, NULL) != 0) - fatal("sigaction failed"); - - event_del(&main_ev_sigterm); -} - /* ARGSUSED */ void main_signal(int sig, unused short events, unused void *data) { + int st; + switch (sig) { case SIGTERM: exit(1); + case SIGCHLD: + wait(&st); + break; + default: + /* signal not handled here, ignore. */ + return; } } @@ -718,7 +671,7 @@ main_dispatch(const char *shellcmd) memcpy(&shelldata, imsg.data, sizeof shelldata); shelldata.shell[(sizeof shelldata.shell) - 1] = '\0'; - main_clear_signals(); + clear_signals(); shell_exec(shelldata.shell, shellcmd); default: Index: tmux.h =================================================================== RCS file: /cvsroot/tmux/tmux/tmux.h,v retrieving revision 1.555 diff -u -p -r1.555 tmux.h --- tmux.h 6 Apr 2010 21:59:19 -0000 1.555 +++ tmux.h 28 Apr 2010 21:46:17 -0000 @@ -1252,6 +1252,7 @@ extern struct options global_options; extern struct options global_s_options; extern struct options global_w_options; extern struct environ global_environ; +extern struct event_base *ev_base; extern char *cfg_file; extern int debug_level; extern int be_quiet; @@ -1582,8 +1583,6 @@ const char *key_string_lookup_key(int); extern struct clients clients; extern struct clients dead_clients; int server_start(char *); -void server_signal_set(void); -void server_signal_clear(void); void server_update_socket(void); /* server-client.c */ @@ -1899,6 +1898,10 @@ void window_choose_ready(struct window void queue_window_name(struct window *); char *default_window_name(struct window *); +/* signal.c */ +void set_signals(void(*handler)(int, short, unused void *)); +void clear_signals(void); + /* session.c */ extern struct sessions sessions; extern struct sessions dead_sessions; Index: window.c =================================================================== RCS file: /cvsroot/tmux/tmux/window.c,v retrieving revision 1.130 diff -u -p -r1.130 window.c --- window.c 6 Apr 2010 21:59:19 -0000 1.130 +++ window.c 28 Apr 2010 21:46:17 -0000 @@ -527,7 +527,7 @@ window_pane_spawn(struct window_pane *wp environ_push(env); - server_signal_clear(); + clear_signals(); log_close(); if (*wp->cmd != '\0') { ------------------------------------------------------------------------------ _______________________________________________ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users