gmain: Add private API to create Unix child watch that uses waitid()

This avoids collecting the zombie child, which means that the PID
can't be reused.  This prevents possible race conditions that might
occur were one to send e.g. SIGTERM to a child.

This race condition has always existed due to the way we called
waitpid() for the app, but the window was widened when we moved the
waitpid() calls into a separate thread.

If waitid() isn't available, we return NULL, and consumers of this
private API (namely, GSubprocess) will need to handle that.

https://bugzilla.gnome.org/show_bug.cgi?id=672102
This commit is contained in:
Colin Walters 2012-05-21 17:09:06 -04:00
parent e744a4aa7a
commit 93bf37ce15
4 changed files with 141 additions and 45 deletions

View File

@ -989,7 +989,7 @@ AC_MSG_RESULT(unsigned $glib_size_type)
# Check for some functions
AC_CHECK_FUNCS(lstat strerror strsignal memmove vsnprintf stpcpy strcasecmp strncasecmp poll getcwd vasprintf setenv unsetenv getc_unlocked readlink symlink fdwalk memmem)
AC_CHECK_FUNCS(chown lchmod lchown fchmod fchown link utimes getgrgid getpwuid getresuid)
AC_CHECK_FUNCS(chown lchmod lchown fchmod fchown link utimes getgrgid getpwuid getresuid waitid)
AC_CHECK_FUNCS(getmntent_r setmntent endmntent hasmntopt getfsstat getvfsstat)
# Check for high-resolution sleep functions
AC_CHECK_FUNCS(splice)

View File

@ -41,7 +41,8 @@ glib__private__ (void)
g_get_worker_context,
g_check_setuid,
g_main_context_new_with_next_id
g_main_context_new_with_next_id,
g_child_watch_source_new_nowait
};
return &table;

View File

@ -29,6 +29,8 @@ G_GNUC_INTERNAL
gboolean g_check_setuid (void);
G_GNUC_INTERNAL
GMainContext * g_main_context_new_with_next_id (guint next_id);
G_GNUC_INTERNAL
GSource * g_child_watch_source_new_nowait (GPid pid);
#define GLIB_PRIVATE_CALL(symbol) (glib__private__()->symbol)
@ -46,6 +48,7 @@ typedef struct {
gboolean (* g_check_setuid) (void);
GMainContext * (* g_main_context_new_with_next_id) (guint next_id);
GSource * (* g_child_watch_source_new_nowait) (GPid pid);
/* Add other private functions here, initialize them in glib-private.c */
} GLibPrivateVTable;

View File

@ -293,7 +293,9 @@ struct _GChildWatchSource
#ifdef G_OS_WIN32
GPollFD poll;
#else /* G_OS_WIN32 */
gboolean child_exited;
guint child_exited : 1;
guint wnowait : 1;
guint reserved : 30;
#endif /* G_OS_WIN32 */
};
@ -4460,6 +4462,82 @@ wake_source (GSource *source)
G_UNLOCK(main_context_list);
}
/* This is a hack; we want to use the newer waitid() call, but the use
* of the waitpid() status code API is baked into the GChildWatchSource
* callback signature, so we need to synthesize it from the siginfo_t
* we get from waitid().
*/
#ifdef HAVE_WAITID
#define __G_MAKE_EXITSTATUS(ecode, signum) ((ecode) << 8 | (signum))
static gint
waitpid_status_from_siginfo (siginfo_t *siginfo)
{
G_STATIC_ASSERT(((WIFEXITED (__G_MAKE_EXITSTATUS (1, 0))) && \
!(WIFSIGNALED (__G_MAKE_EXITSTATUS (1, 0))) && \
(WEXITSTATUS (__G_MAKE_EXITSTATUS (1, 0)) == 1)));
G_STATIC_ASSERT((!WIFEXITED (__G_MAKE_EXITSTATUS (0, 1))) && \
(WIFSIGNALED (__G_MAKE_EXITSTATUS (0, 1))) && \
(WTERMSIG (__G_MAKE_EXITSTATUS (0, 1)) == 1));
if (siginfo->si_code == CLD_EXITED)
return __G_MAKE_EXITSTATUS(siginfo->si_status, 0);
else if (siginfo->si_code == CLD_KILLED || siginfo->si_code == CLD_DUMPED)
return __G_MAKE_EXITSTATUS(0, siginfo->si_status);
else
return __G_MAKE_EXITSTATUS(0xFF, 0);
}
#undef __G_MAKE_EXITSTATUS
#endif /* HAVE_WAITID */
/* Returns TRUE if we need to wake the source */
static gboolean
unix_child_watch_source_waitpid (GChildWatchSource *source)
{
pid_t pid;
if (source->child_exited)
return FALSE;
#ifdef HAVE_WAITID
if (source->wnowait)
{
siginfo_t siginfo;
int r;
siginfo.si_pid = 0;
do
r = waitid (P_PID, (id_t)source->pid, &siginfo, WEXITED | WNOHANG | WNOWAIT);
while (r == -1 && errno == EINTR);
if (r == 0 && siginfo.si_pid == source->pid)
{
source->child_exited = TRUE;
source->child_status = waitpid_status_from_siginfo (&siginfo);
return TRUE;
}
} else
#endif
{
do
pid = waitpid (source->pid, &source->child_status, WNOHANG);
while (pid == -1 && errno == EINTR);
if (pid > 0)
{
source->child_exited = TRUE;
return TRUE;
}
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;
return TRUE;
}
}
return FALSE;
}
static void
dispatch_unix_signals (void)
{
@ -4486,28 +4564,9 @@ dispatch_unix_signals (void)
for (node = unix_child_watches; node; node = node->next)
{
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);
}
if (unix_child_watch_source_waitpid (source))
wake_source ((GSource *) source);
}
}
@ -4699,6 +4758,58 @@ g_unix_signal_handler (int signum)
#endif /* !G_OS_WIN32 */
static GSource *
g_child_watch_source_new_internal (GPid pid,
gboolean wnowait)
{
GSource *source;
GChildWatchSource *child_watch_source;
#if defined(G_OS_UNIX) && !defined(HAVE_WAITID)
if (wnowait)
return NULL;
#else
source = g_source_new (&g_child_watch_funcs, sizeof (GChildWatchSource));
child_watch_source = (GChildWatchSource *)source;
child_watch_source->pid = pid;
#if defined(G_OS_UNIX) && defined(HAVE_WAITID)
if (wnowait)
child_watch_source->wnowait = TRUE;
#endif
#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);
unix_child_watch_source_waitpid (child_watch_source);
G_UNLOCK (unix_signal_lock);
#endif /* G_OS_WIN32 */
return source;
#endif
}
/* internal
* Create a child watch, using @wnowait to enable a mode
* where the child won't be reaped. This is used by
* GSubprocess to ensure that the pid is valid for
* the lifetime of the GSubprocess object. Otherwise
* there'd be a possible race condition where we reap
* the child with waitpid() in the worker thread,
* while the main thread is calling kill() on it.
*/
GSource *
g_child_watch_source_new_nowait (GPid pid)
{
return g_child_watch_source_new_internal (pid, TRUE);
}
/**
* g_child_watch_source_new:
* @pid: process to watch. On POSIX the pid of a child process. On
@ -4731,26 +4842,7 @@ g_unix_signal_handler (int signum)
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 g_child_watch_source_new_internal (pid, FALSE);
}
/**