Merge pull request #2364 from psychon/handle_SIGCHLD_directly

Handle SIGCHLD ourselves instead of through GLib

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;
 

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 */

spawn.h

@@ -27,10 +27,9 @@
 #include <lua.h>
 
 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