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

Reply via email to