From 6c559e188c2aa3b6e3777c2bd81d0219967eeb38 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Tue, 21 Aug 2018 15:30:07 +0200 Subject: [PATCH] Handle SIGCHLD ourselves instead of through GLib GLib is very careful not to "fetch" any children that it was not asked to fetch. Thus, if awesome inherits some children, it does not reap them and starts collecting zombies. Thus, this commit makes awesome handle SIGCHLD directly: On SIGCHLD, a byte is written to a pipe. The main loop reads from this pipe and collects children via waitpid(-1). Unknown children cause a warning to be printed. We might want to remove this warning if it turns out to be annoying. This commit adds 79 lines and removes 89 lines. Thus, this is a net reduction in lines of codes. This is because previously, we gave the list of still-running children that we know about to the next awesome instance we a --reap command line argument. This was added so that awesome does not get zombie children. However, this commit fixes this problem and makes all the code for this 'feature' unnecessary. Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=886393 Signed-off-by: Uli Schlachter --- awesome.c | 52 ++++++++++++++++++++++++- spawn.c | 113 ++++++++++++++---------------------------------------- spawn.h | 3 +- 3 files changed, 79 insertions(+), 89 deletions(-) diff --git a/awesome.c b/awesome.c index 3513cecf..e391b13b 100644 --- a/awesome.c +++ b/awesome.c @@ -67,6 +67,9 @@ static struct timeval last_wakeup; /** current limit for the main loop's runtime */ static float main_loop_iteration_limit = 0.1; +/** A pipe that is used to asynchronously handle SIGCHLD */ +static int sigchld_pipe[2]; + /** Call before exiting. */ void @@ -115,6 +118,9 @@ awesome_atexit(bool restart) /* Disconnect *after* closing lua */ xcb_cursor_context_free(globalconf.cursor_ctx); xcb_disconnect(globalconf.connection); + + close(sigchld_pipe[0]); + close(sigchld_pipe[1]); } /** Restore the client order after a restart */ @@ -448,6 +454,34 @@ signal_fatal(int signum) fatal("signal %d, dumping backtrace\n%s", signum, buf.s); } +/* Signal handler for SIGCHLD. Causes reap_children() to be called. */ +static void +signal_child(int signum) +{ + assert(signum == SIGCHLD); + int res = write(sigchld_pipe[1], " ", 1); + assert(res == 1); +} + +/* There was a SIGCHLD signal. Read from sigchld_pipe and reap children. */ +static gboolean +reap_children(GIOChannel *channel, GIOCondition condition, gpointer user_data) +{ + pid_t child; + int status; + char buffer[1024]; + ssize_t result = read(sigchld_pipe[0], &buffer[0], sizeof(buffer)); + if (result < 0) + fatal("Error reading from signal pipe: %s", strerror(errno)); + + while ((child = waitpid(-1, &status, WNOHANG)) > 0) { + spawn_child_exited(child, status); + } + if (child < 0 && errno != ECHILD) + warn("waitpid(-1) failed: %s", strerror(errno)); + return TRUE; +} + /** Function to exit on some signals. * \param data currently unused */ @@ -462,7 +496,7 @@ void awesome_restart(void) { awesome_atexit(true); - execvp(awesome_argv[0], spawn_transform_commandline(awesome_argv)); + execvp(awesome_argv[0], awesome_argv); fatal("execv() failed: %s", strerror(errno)); } @@ -577,7 +611,7 @@ main(int argc, char **argv) replace_wm = true; break; case '\1': - spawn_handle_reap(optarg); + /* Silently ignore --reap and its argument */ break; default: exit_help(EXIT_FAILURE); @@ -630,6 +664,16 @@ main(int argc, char **argv) } } + /* Setup pipe for SIGCHLD processing */ + { + if (!g_unix_open_pipe(sigchld_pipe, FD_CLOEXEC, NULL)) + fatal("Failed to create pipe"); + + GIOChannel *channel = g_io_channel_unix_new(sigchld_pipe[0]); + g_io_add_watch(channel, G_IO_IN, reap_children, NULL); + g_io_channel_unref(channel); + } + /* register function for signals */ g_unix_signal_add(SIGINT, exit_on_signal, NULL); g_unix_signal_add(SIGTERM, exit_on_signal, NULL); @@ -644,6 +688,10 @@ main(int argc, char **argv) sigaction(SIGSEGV, &sa, 0); signal(SIGPIPE, SIG_IGN); + sa.sa_handler = signal_child; + sa.sa_flags = SA_NOCLDSTOP | SA_RESTART; + sigaction(SIGCHLD, &sa, 0); + /* We have no clue where the input focus is right now */ globalconf.focus.need_update = true; diff --git a/spawn.c b/spawn.c index 0bb21fa9..fb4afe69 100644 --- a/spawn.c +++ b/spawn.c @@ -62,12 +62,6 @@ /** 20 seconds timeout */ #define AWESOME_SPAWN_TIMEOUT 20.0 -static int -compare_pids(const void *a, const void *b) -{ - return *(GPid *) a - *(GPid *)b; -} - /** Wrapper for unrefing startup sequence. */ static inline void @@ -81,9 +75,20 @@ DO_ARRAY(SnStartupSequence *, SnStartupSequence, a_sn_startup_sequence_unref) /** The array of startup sequence running */ static SnStartupSequence_array_t sn_waits; -DO_BARRAY(GPid, pid, DO_NOTHING, compare_pids) +typedef struct { + GPid pid; + int exit_callback; +} running_child_t; -static pid_array_t running_children; +static int +compare_pids(const void *a, const void *b) +{ + return ((running_child_t *) a)->pid - ((running_child_t *) b)->pid; +} + +DO_BARRAY(running_child_t, running_child, DO_NOTHING, compare_pids) + +static running_child_array_t running_children; /** Remove a SnStartupSequence pointer from an array and forget about it. * \param s The startup sequence to found, remove and unref. @@ -264,20 +269,6 @@ spawn_start_notify(client_t *c, const char * startup_id) } } -static void -remove_running_child(GPid pid, gint status, gpointer user_data) -{ - (void) status; - (void) user_data; - - GPid *pid_in_array = pid_array_lookup(&running_children, &pid); - if (pid_in_array != NULL) { - pid_array_remove(&running_children, pid_in_array); - } else { - warn("(Partially) unknown child %d exited?!", (int) pid); - } -} - /** Initialize program spawner. */ void @@ -291,61 +282,6 @@ spawn_init(void) NULL, NULL); } -void -spawn_handle_reap(const char *arg) -{ - GPid pid = atoll(arg); - pid_array_insert(&running_children, pid); - g_child_watch_add((GPid) pid, remove_running_child, NULL); -} - -/** Called right before exit, serialise state in case of a restart. - */ -char * const * -spawn_transform_commandline(char **argv) -{ - size_t offset = 0; - size_t length = 0; - while(argv[offset] != NULL) - { - if(A_STREQ(argv[offset], "--reap")) - offset += 2; - else { - length++; - offset++; - } - } - - length += 2*running_children.len; - - const char ** transformed = p_new(const char *, length+1); - size_t index = 0; - offset = 0; - while(argv[index + offset] != NULL) - { - if(A_STREQ(argv[index + offset], "--reap")) - offset += 2; - else { - transformed[index] = argv[index + offset]; - index++; - } - } - - foreach(pid, running_children) - { - buffer_t buffer; - buffer_init(&buffer); - buffer_addf(&buffer, "%d", (int) *pid); - - transformed[index++] = "--reap"; - transformed[index++] = buffer_detach(&buffer); - buffer_wipe(&buffer); - } - transformed[index++] = NULL; - - return (char * const *) transformed; -} - static gboolean spawn_launchee_timeout(gpointer context) { @@ -440,12 +376,20 @@ parse_command(lua_State *L, int idx, GError **error) } /** Callback for when a spawned process exits. */ -static void -child_exit_callback(GPid pid, gint status, gpointer user_data) +void +spawn_child_exited(pid_t pid, int status) { + int exit_callback; + running_child_t needle = { .pid = pid }; lua_State *L = globalconf_get_lua_State(); - int exit_callback = GPOINTER_TO_INT(user_data); - remove_running_child(pid, status, user_data); + + running_child_t *child = running_child_array_lookup(&running_children, &needle); + if (child == NULL) { + warn("Unknown child %d exited with status %d", (int) pid, status); + return; + } + exit_callback = child->exit_callback; + running_child_array_remove(&running_children, child); /* 'Decode' the exit status */ if (WIFEXITED(status)) { @@ -570,11 +514,10 @@ luaA_spawn(lua_State *L) if(flags & G_SPAWN_DO_NOT_REAP_CHILD) { - int exit_callback = LUA_REFNIL; /* Only do this down here to avoid leaks in case of errors */ - luaA_registerfct(L, 6, &exit_callback); - pid_array_insert(&running_children, pid); - g_child_watch_add(pid, child_exit_callback, GINT_TO_POINTER(exit_callback)); + running_child_t child = { .pid = pid, .exit_callback = LUA_REFNIL }; + luaA_registerfct(L, 6, &child.exit_callback); + running_child_array_insert(&running_children, child); } /* push pid on stack */ diff --git a/spawn.h b/spawn.h index 0949cce6..95b14b49 100644 --- a/spawn.h +++ b/spawn.h @@ -27,10 +27,9 @@ #include void spawn_init(void); -void spawn_handle_reap(const char *); -char * const * spawn_transform_commandline(char **); void spawn_start_notify(client_t *, const char *); int luaA_spawn(lua_State *); +void spawn_child_exited(pid_t, int); #endif // vim: filetype=c:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80