GChildWatchSource: Allow passing -1 for pid
With the goal of modifying gnome-session to use the new Linux
PR_SET_CHILD_SUBREAPER, it needs to be able to reap arbitary children
that may be reparented to it.
But if gnome-session is going to continue to work with GLib and GIO,
even if we converted all child watches in gnome-session to use this
new API, we'd still have to contend with the possibility of it having
a process spawned indirectly (stuff like the GDBus dbus-launch
invocation). Thus, we don't give the -1 child watch *every* waitpid,
only the ones which don't have a specific watcher.
Also, fix up g_spawn_sync() to hold the unix signal lock when forking.
This way we ensure that GLib itself gets the waitpid() result for the
synchronously spawned child, instead of racing with the worker thread.
https://bugzilla.gnome.org/show_bug.cgi?id=687061
diff --git a/glib/gmain-internal.h b/glib/gmain-internal.h
index 648aff3..ef13ff8 100644
--- a/glib/gmain-internal.h
+++ b/glib/gmain-internal.h
@@ -30,6 +30,10 @@
GSource *_g_main_create_unix_signal_watch (int signum);
+#ifdef G_OS_UNIX
+GPid _g_main_fork_and_do_not_reap (void);
+#endif
+
G_END_DECLS
#endif /* __G_MAIN_H__ */
diff --git a/glib/gmain.c b/glib/gmain.c
index af1092d..7bfac1a 100644
--- a/glib/gmain.c
+++ b/glib/gmain.c
@@ -185,6 +185,7 @@
typedef struct _GTimeoutSource GTimeoutSource;
typedef struct _GChildWatchSource GChildWatchSource;
typedef struct _GUnixSignalWatchSource GUnixSignalWatchSource;
+typedef struct _GUnixCatchallChildWatchSource GUnixCatchallChildWatchSource;
typedef struct _GPollRec GPollRec;
typedef struct _GSourceCallback GSourceCallback;
@@ -303,6 +304,12 @@
gboolean pending;
};
+struct _GUnixCatchallChildWatchSource
+{
+ GSource source;
+ GSList *pending_waits;
+};
+
struct _GPollRec
{
GPollFD *fd;
@@ -395,6 +402,13 @@
GSourceFunc callback,
gpointer user_data);
static void g_unix_signal_watch_finalize (GSource *source);
+static gboolean g_unix_catchall_watch_prepare (GSource *source,
+ gint *timeout);
+static gboolean g_unix_catchall_watch_check (GSource *source);
+static gboolean g_unix_catchall_watch_dispatch (GSource *source,
+ GSourceFunc callback,
+ gpointer user_data);
+static void g_unix_catchall_watch_finalize (GSource *source);
#endif
static gboolean g_idle_prepare (GSource *source,
gint *timeout);
@@ -428,6 +442,13 @@
G_LOCK_DEFINE_STATIC (unix_signal_lock);
static GSList *unix_signal_watches;
static GSList *unix_child_watches;
+static GHashTable *unix_catchall_do_not_reap; /* GPid -> UnixWaitStatus */
+static GUnixCatchallChildWatchSource *unix_catchall_child_watch;
+
+typedef struct {
+ pid_t pid;
+ gint estatus;
+} UnixWaitStatus;
static GSourceFuncs g_unix_signal_funcs =
{
@@ -436,6 +457,14 @@
g_unix_signal_watch_dispatch,
g_unix_signal_watch_finalize
};
+
+static GSourceFuncs g_unix_catchall_funcs =
+{
+ g_unix_catchall_watch_prepare,
+ g_unix_catchall_watch_check,
+ g_unix_catchall_watch_dispatch,
+ g_unix_catchall_watch_finalize
+};
#endif /* !G_OS_WIN32 */
G_LOCK_DEFINE_STATIC (main_context_list);
static GSList *main_context_list = NULL;
@@ -4392,6 +4421,130 @@
G_UNLOCK(main_context_list);
}
+static inline pid_t
+safe_waitpid (pid_t wait_for,
+ int *estatus)
+{
+ pid_t pid;
+ do
+ pid = waitpid (wait_for, estatus, WNOHANG);
+ while (pid == -1 && errno == EINTR);
+ return pid;
+}
+
+/* Here the application has explicitly requested use of waitpid(-1,
+ * ...). The contortions below are to be sure we dispatch normal
+ * GChildWatchSource for their requested pid, only passing other pids
+ * to the catchall source.
+ */
+static void
+dispatch_sigchld_with_catchall (void)
+{
+ gboolean caught_one_catchall = FALSE;
+
+ while (TRUE)
+ {
+ gboolean give_to_catchall = TRUE;
+ UnixWaitStatus *waitstatus;
+ GSList *node;
+ int estatus;
+ pid_t pid;
+
+ pid = safe_waitpid (-1, &estatus);
+
+ /* We're done if we have no child processes, or on an error.
+ * Ideally we'd log an error, if one somehow occurred, but we
+ * can't get ECHILD, we're handling EINTR, and the only way we'd
+ * get EINVAL is if the system didn't support WNOHANG, in which
+ * case we're screwed anyways. Those are all of the errnos
+ * listed in the glibc manual.
+ */
+ if (pid <= 0)
+ break;
+
+ for (node = unix_child_watches; node; node = node->next)
+ {
+ GChildWatchSource *source = node->data;
+
+ if (source->child_exited
+ || source->pid != pid)
+ continue;
+
+ source->child_status = estatus;
+ source->child_exited = TRUE;
+ give_to_catchall = FALSE;
+ wake_source ((GSource*)source);
+ }
+
+ /* This is a synchronization mechanism with gspawn.c */
+ waitstatus = g_hash_table_lookup (unix_catchall_do_not_reap,
+ GINT_TO_POINTER (pid));
+ if (waitstatus != NULL)
+ {
+ waitstatus->pid = pid; /* Setting pid = pid makes it valid */
+ waitstatus->estatus = estatus;
+ give_to_catchall = FALSE;
+ }
+
+ /* The catchall watch only gets processes that don't already
+ * have a specific child watch.
+ */
+ if (!give_to_catchall)
+ continue;
+
+ if (unix_catchall_child_watch != NULL)
+ {
+ waitstatus = g_slice_new (UnixWaitStatus);
+ waitstatus->pid = pid;
+ waitstatus->estatus = estatus;
+
+ unix_catchall_child_watch->pending_waits = g_slist_prepend (unix_catchall_child_watch->pending_waits, waitstatus);
+ caught_one_catchall = TRUE;
+ }
+ }
+
+ if (caught_one_catchall)
+ wake_source ((GSource *) unix_catchall_child_watch);
+}
+
+/* In this case, we have to individually scan all of the children.
+ *
+ * The docs promise that we will not reap children that we are
+ * not explicitly watching, so that ties our hands from calling
+ * waitpid(-1) - that is a separate _catchall API. We also
+ * can't use siginfo's si_pid field since if multiple SIGCHLD
+ * arrive at the same time, one of them can be dropped (since a
+ * given UNIX signal can only be pending once).
+ */
+static void
+dispatch_sigchld_individually (void)
+{
+ GSList *node;
+
+ for (node = unix_child_watches; node; node = node->next)
+ {
+ GChildWatchSource *source = node->data;
+ pid_t pid;
+
+ if (source->child_exited)
+ continue;
+
+ pid = safe_waitpid (source->pid, &source->child_status);
+ if (pid > 0)
+ {
+ source->child_exited = TRUE;
+ wake_source ((GSource *) source);
+ }
+ else if (pid == -1 && errno == ECHILD)
+ {
+ g_warning ("GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). Most likely the process is ignoring SIGCHLD, or some other thread is invoking waitpid() with a nonpositive first argument; either behavior can break applications that use g_child_watch_add()/g_spawn_sync() either directly or indirectly.");
+ source->child_exited = TRUE;
+ source->child_status = 0;
+ wake_source ((GSource *) source);
+ }
+ }
+}
+
static void
dispatch_unix_signals (void)
{
@@ -4407,39 +4560,13 @@
{
unix_signal_pending[SIGCHLD] = FALSE;
- /* The only way we can do this is to scan all of the children.
- *
- * The docs promise that we will not reap children that we are not
- * explicitly watching, so that ties our hands from calling
- * waitpid(-1). We also can't use siginfo's si_pid field since if
- * multiple SIGCHLD arrive at the same time, one of them can be
- * dropped (since a given UNIX signal can only be pending once).
- */
- for (node = unix_child_watches; node; node = node->next)
+ if (unix_catchall_child_watch != NULL)
{
- GChildWatchSource *source = node->data;
-
- if (!source->child_exited)
- {
- pid_t pid;
- do
- {
- pid = waitpid (source->pid, &source->child_status, WNOHANG);
- if (pid > 0)
- {
- source->child_exited = TRUE;
- wake_source ((GSource *) source);
- }
- else if (pid == -1 && errno == ECHILD)
- {
- g_warning ("GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). Most likely the process is ignoring SIGCHLD, or some other thread is invoking waitpid() with a nonpositive first argument; either behavior can break applications that use g_child_watch_add()/g_spawn_sync() either directly or indirectly.");
- source->child_exited = TRUE;
- source->child_status = 0;
- wake_source ((GSource *) source);
- }
- }
- while (pid == -1 && errno == EINTR);
- }
+ dispatch_sigchld_with_catchall ();
+ }
+ else
+ {
+ dispatch_sigchld_individually ();
}
}
@@ -4484,6 +4611,14 @@
return child_watch_source->child_exited;
}
+static void
+g_child_watch_finalize (GSource *source)
+{
+ G_LOCK (unix_signal_lock);
+ unix_child_watches = g_slist_remove (unix_child_watches, source);
+ G_UNLOCK (unix_signal_lock);
+}
+
static gboolean
g_unix_signal_watch_prepare (GSource *source,
gint *timeout)
@@ -4530,6 +4665,14 @@
}
static void
+g_unix_signal_watch_finalize (GSource *source)
+{
+ G_LOCK (unix_signal_lock);
+ unix_signal_watches = g_slist_remove (unix_signal_watches, source);
+ G_UNLOCK (unix_signal_lock);
+}
+
+static void
ensure_unix_signal_handler_installed_unlocked (int signum)
{
static sigset_t installed_signal_mask;
@@ -4577,23 +4720,121 @@
return source;
}
+static gboolean
+g_unix_catchall_watch_prepare (GSource *source,
+ gint *timeout)
+{
+ gboolean res;
+ GUnixCatchallChildWatchSource *catchall_source;
+
+ catchall_source = (GUnixCatchallChildWatchSource *) source;
+
+ G_LOCK (unix_signal_lock);
+ res = catchall_source->pending_waits != NULL;
+ G_UNLOCK (unix_signal_lock);
+ return res;
+}
+
+static gboolean
+g_unix_catchall_watch_check (GSource *source)
+{
+ return g_unix_catchall_watch_prepare (source, NULL);
+}
+
+static gboolean
+g_unix_catchall_watch_dispatch (GSource *source,
+ GSourceFunc callback,
+ gpointer user_data)
+{
+ GUnixCatchallChildWatchSource *catchall_source;
+ GChildWatchFunc child_callback = (GChildWatchFunc) callback;
+ GSList *iter;
+ GSList *pending;
+
+ catchall_source = (GUnixCatchallChildWatchSource *) source;
+
+ if (!callback)
+ {
+ g_warning ("Unix catchall source dispatched without callback\n"
+ "You must call g_source_set_callback().");
+ return FALSE;
+ }
+
+ G_LOCK (unix_signal_lock);
+ pending = catchall_source->pending_waits;
+ catchall_source->pending_waits = NULL;
+ G_UNLOCK (unix_signal_lock);
+
+ for (iter = pending; iter; iter = iter->next)
+ {
+ UnixWaitStatus *status = iter->data;
+
+ (child_callback) (status->pid, status->estatus, user_data);
+
+ g_slice_free (UnixWaitStatus, status);
+ }
+
+ g_slist_free (pending);
+
+ return TRUE;
+}
+
static void
-g_unix_signal_watch_finalize (GSource *source)
+g_unix_catchall_watch_finalize (GSource *source)
{
G_LOCK (unix_signal_lock);
- unix_signal_watches = g_slist_remove (unix_signal_watches, source);
+ g_assert (source == (GSource*)unix_catchall_child_watch);
+ unix_catchall_child_watch = NULL;
G_UNLOCK (unix_signal_lock);
}
static void
-g_child_watch_finalize (GSource *source)
+unix_wait_status_free (gpointer data)
{
- G_LOCK (unix_signal_lock);
- unix_child_watches = g_slist_remove (unix_child_watches, source);
- G_UNLOCK (unix_signal_lock);
+ g_slice_free (UnixWaitStatus, data);
}
-#endif /* G_OS_WIN32 */
+/* Used by gspawn.c to ensure that if we have a catchall child watch
+ * installed, we don't race with user code installing a specific child
+ * watch.
+ */
+GPid
+_g_main_fork_and_do_not_reap (void)
+{
+ pid_t pid;
+ int errno_saved;
+
+ G_LOCK (unix_signal_lock);
+
+ pid = fork ();
+ /* We only have special handling for fork() if a
+ * catchall child watch is installed.
+ */
+ if (unix_catchall_child_watch != NULL)
+ {
+ if (pid > 0)
+ {
+ gpointer pid_p = GINT_TO_POINTER (pid);
+ UnixWaitStatus *status;
+
+ if (unix_catchall_do_not_reap == NULL)
+ unix_catchall_do_not_reap = g_hash_table_new_full (NULL, NULL, NULL, unix_wait_status_free);
+
+ status = g_slice_new (UnixWaitStatus);
+ status->pid = -1; /* Mark as pending */
+ g_hash_table_insert (unix_catchall_do_not_reap, pid_p, status);
+ }
+ else if (pid == -1)
+ errno_saved = errno;
+ }
+
+ G_UNLOCK (unix_signal_lock);
+
+ errno = errno_saved;
+ return pid;
+}
+
+#endif /* !G_OS_WIN32 */
static gboolean
g_child_watch_dispatch (GSource *source,
@@ -4629,7 +4870,65 @@
g_wakeup_signal (glib_worker_context->wakeup);
}
-#endif /* !G_OS_WIN32 */
+static GSource *
+child_watch_source_new_unix (GPid pid)
+{
+ GSource *source;
+
+ G_LOCK (unix_signal_lock);
+ ensure_unix_signal_handler_installed_unlocked (SIGCHLD);
+
+ if (pid == -1)
+ {
+ g_assert (unix_catchall_child_watch == NULL);
+ source = g_source_new (&g_unix_catchall_funcs, sizeof (GUnixCatchallChildWatchSource));
+ unix_catchall_child_watch = (GUnixCatchallChildWatchSource*)source;
+ }
+ else
+ {
+ GChildWatchSource *child_watch_source;
+ UnixWaitStatus *waitstatus;
+
+ source = g_source_new (&g_child_watch_funcs, sizeof (GChildWatchSource));
+ child_watch_source = (GChildWatchSource *)source;
+ child_watch_source->pid = pid;
+ unix_child_watches = g_slist_prepend (unix_child_watches, child_watch_source);
+
+ waitstatus = g_hash_table_lookup (unix_catchall_do_not_reap,
+ GINT_TO_POINTER (pid));
+ if (waitstatus != NULL && waitstatus->pid == pid)
+ {
+ child_watch_source->child_status = waitstatus->estatus;
+ g_hash_table_remove (unix_catchall_do_not_reap, GINT_TO_POINTER (pid));
+ }
+ else
+ {
+ if (safe_waitpid (pid, &child_watch_source->child_status) > 0)
+ child_watch_source->child_exited = TRUE;
+ }
+ }
+
+ G_UNLOCK (unix_signal_lock);
+ return source;
+}
+
+#else
+
+static GSource *
+child_watch_source_new_win32 (GPid pid)
+{
+ GSource *source;
+ GChildWatchSource *child_watch_source;
+
+ source = g_source_new (&g_child_watch_funcs, sizeof (GChildWatchSource));
+ child_watch_source = (GChildWatchSource*) source;
+ child_watch_source->poll.fd = (gintptr) pid;
+ child_watch_source->poll.events = G_IO_IN;
+
+ g_source_add_poll (source, &child_watch_source->poll);
+}
+
+#endif
/**
* g_child_watch_source_new:
@@ -4655,6 +4954,15 @@
* compatible with calling <literal>waitpid</literal> with a
* nonpositive first argument in the application. Calling waitpid()
* for individual pids will still work fine.
+ *
+ * Since GLib 2.36, on UNIX platforms, -1 may be given as @pid. In
+ * this mode, the source will watch child processes of the current
+ * process which don't already have another dedicated child watch.
+ * This allows a GLib-using process to perform a function similar to
+ * to calling <literal>waitpid(-1, ...)</literal>. There can at
+ * present be at most one instance of a source in this mode. On
+ * Linux, it makes sense to use this together with
+ * <literal>prctl(PR_SET_CHILD_SUBREAPER)</literal>.
*
* Return value: the newly-created child watch source
*
@@ -4663,26 +4971,11 @@
GSource *
g_child_watch_source_new (GPid pid)
{
- GSource *source = g_source_new (&g_child_watch_funcs, sizeof (GChildWatchSource));
- GChildWatchSource *child_watch_source = (GChildWatchSource *)source;
-
- child_watch_source->pid = pid;
-
#ifdef G_OS_WIN32
- child_watch_source->poll.fd = (gintptr) pid;
- child_watch_source->poll.events = G_IO_IN;
-
- g_source_add_poll (source, &child_watch_source->poll);
-#else /* G_OS_WIN32 */
- G_LOCK (unix_signal_lock);
- ensure_unix_signal_handler_installed_unlocked (SIGCHLD);
- unix_child_watches = g_slist_prepend (unix_child_watches, child_watch_source);
- if (waitpid (pid, &child_watch_source->child_status, WNOHANG) > 0)
- child_watch_source->child_exited = TRUE;
- G_UNLOCK (unix_signal_lock);
-#endif /* G_OS_WIN32 */
-
- return source;
+ return child_watch_source_new_win32 (pid);
+#else
+ return child_watch_source_new_unix (pid);
+#endif
}
/**
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 3545a78..1285dd7 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -47,6 +47,7 @@
#include "genviron.h"
#include "gmem.h"
#include "gmain.h"
+#include "gmain-internal.h"
#include "gshell.h"
#include "gstring.h"
#include "gstrfuncs.h"
@@ -281,7 +282,7 @@
gint ret;
GString *outstr = NULL;
GString *errstr = NULL;
- gboolean failed;
+ gboolean failed = FALSE;
gint status;
SyncWaitpidData waitpid_data;
@@ -300,7 +301,7 @@
if (standard_error)
*standard_error = NULL;
-
+
if (!fork_exec_with_pipes (FALSE,
working_directory,
argv,
@@ -319,11 +320,12 @@
standard_output ? &outpipe : NULL,
standard_error ? &errpipe : NULL,
error))
- return FALSE;
+ {
+ failed = TRUE;
+ goto out;
+ }
/* Read data from child. */
-
- failed = FALSE;
if (outpipe >= 0)
{
@@ -436,13 +438,14 @@
g_source_set_callback (source, (GSourceFunc)on_sync_waitpid, &waitpid_data, NULL);
g_source_attach (source, context);
g_source_unref (source);
-
+
g_main_loop_run (loop);
g_main_context_unref (context);
g_main_loop_unref (loop);
}
+ out:
if (failed)
{
if (outstr)
@@ -1342,7 +1345,10 @@
if (standard_error && !make_pipe (stderr_pipe, error))
goto cleanup_and_fail;
- pid = fork ();
+ if (intermediate_child)
+ pid = fork ();
+ else
+ pid = _g_main_fork_and_do_not_reap ();
if (pid < 0)
{
diff --git a/glib/tests/unix.c b/glib/tests/unix.c
index 329e19a..ffcc6a1 100644
--- a/glib/tests/unix.c
+++ b/glib/tests/unix.c
@@ -147,6 +147,105 @@
}
+typedef struct {
+ GPid regular_pid;
+ GPid catchall_pid;
+ GMainLoop *loop;
+ gboolean regular_exited;
+ gboolean catchall_exited;
+} CatchAllData;
+
+static void
+on_catchall_child (GPid pid,
+ gint estatus,
+ gpointer user_data)
+{
+ CatchAllData *data = user_data;
+ GError *local_error = NULL;
+ GError **error = &local_error;
+
+ g_assert (pid == data->catchall_pid);
+
+ g_spawn_check_exit_status (estatus, error);
+ g_assert_no_error (local_error);
+
+ data->catchall_exited = TRUE;
+ if (data->regular_exited)
+ g_main_loop_quit (data->loop);
+}
+
+static void
+on_regular_child (GPid pid,
+ gint estatus,
+ gpointer user_data)
+{
+ CatchAllData *data = user_data;
+ GError *local_error = NULL;
+ GError **error = &local_error;
+
+ g_assert (pid == data->regular_pid);
+
+ g_spawn_check_exit_status (estatus, error);
+ g_assert_no_error (local_error);
+
+ data->regular_exited = TRUE;
+ if (data->catchall_exited)
+ g_main_loop_quit (data->loop);
+}
+
+static void
+spawn_with_raw_fork (gchar **child_argv,
+ pid_t *out_pid)
+{
+ pid_t pid;
+
+ pid = fork ();
+ g_assert (pid >= 0);
+ if (pid == 0)
+ {
+ execv (child_argv[0], child_argv+1);
+ g_assert_not_reached ();
+ }
+ else
+ *out_pid = pid;
+}
+
+static void
+test_catchall (void)
+{
+ GMainLoop *mainloop;
+ CatchAllData data;
+ GSource *source;
+ GPid pid;
+ GError *local_error = NULL;
+ GError **error = &local_error;
+ char *child_args[] = { "/bin/true", "/bin/true", NULL };
+
+ memset (&data, 0, sizeof (data));
+ mainloop = g_main_loop_new (NULL, FALSE);
+ data.loop = mainloop;
+
+ source = g_child_watch_source_new (-1);
+ g_source_set_callback (source, (GSourceFunc)on_catchall_child, &data, NULL);
+ g_source_attach (source, NULL);
+ g_source_unref (source);
+
+ g_spawn_async (NULL, (char**)child_args, NULL, G_SPAWN_DO_NOT_REAP_CHILD,
+ NULL, NULL, &pid, error);
+ g_child_watch_add (pid, on_regular_child, &data);
+ data.regular_pid = pid;
+
+ spawn_with_raw_fork ((char**)child_args, &pid);
+ data.catchall_pid = pid;
+
+ g_main_loop_run (mainloop);
+
+ g_source_destroy (source);
+ g_main_loop_unref (mainloop);
+
+ g_assert (data.regular_exited && data.catchall_exited);
+}
+
int
main (int argc,
char *argv[])
@@ -159,6 +258,7 @@
g_test_add_func ("/glib-unix/sigterm", test_sigterm);
g_test_add_func ("/glib-unix/sighup_again", test_sighup);
g_test_add_func ("/glib-unix/sighup_add_remove", test_sighup_add_remove);
+ g_test_add_func ("/glib-unix/catchall", test_catchall);
return g_test_run();
}
