From 49db979922283364238a5c208671591f713ecd78 Mon Sep 17 00:00:00 2001 From: Michael Natterer Date: Thu, 15 Nov 2012 15:23:07 +0100 Subject: [PATCH] Revert "gmain: Add private API to create Unix child watch that uses waitid()" This reverts commit 93bf37ce1507380f74d4cb4cab6640fc7d2eb7d1. --- configure.ac | 2 +- glib/glib-private.c | 3 +- glib/glib-private.h | 3 - glib/gmain.c | 178 +++++++++++--------------------------------- 4 files changed, 45 insertions(+), 141 deletions(-) diff --git a/configure.ac b/configure.ac index 7dd01247d..8badfe34c 100644 --- a/configure.ac +++ b/configure.ac @@ -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 waitid) +AC_CHECK_FUNCS(chown lchmod lchown fchmod fchown link utimes getgrgid getpwuid getresuid) AC_CHECK_FUNCS(getmntent_r setmntent endmntent hasmntopt getfsstat getvfsstat) # Check for high-resolution sleep functions AC_CHECK_FUNCS(splice) diff --git a/glib/glib-private.c b/glib/glib-private.c index c2346c5e3..e7838a600 100644 --- a/glib/glib-private.c +++ b/glib/glib-private.c @@ -41,8 +41,7 @@ glib__private__ (void) g_get_worker_context, g_check_setuid, - g_main_context_new_with_next_id, - g_child_watch_source_new_nowait + g_main_context_new_with_next_id }; return &table; diff --git a/glib/glib-private.h b/glib/glib-private.h index 1ea840dd2..23bfb367a 100644 --- a/glib/glib-private.h +++ b/glib/glib-private.h @@ -29,8 +29,6 @@ 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) @@ -48,7 +46,6 @@ 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; diff --git a/glib/gmain.c b/glib/gmain.c index 6b4312681..c1776a807 100644 --- a/glib/gmain.c +++ b/glib/gmain.c @@ -293,9 +293,7 @@ struct _GChildWatchSource #ifdef G_OS_WIN32 GPollFD poll; #else /* G_OS_WIN32 */ - guint child_exited : 1; - guint wnowait : 1; - guint reserved : 30; + gboolean child_exited; #endif /* G_OS_WIN32 */ }; @@ -4462,82 +4460,6 @@ 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) { @@ -4564,9 +4486,28 @@ dispatch_unix_signals (void) for (node = unix_child_watches; node; node = node->next) { GChildWatchSource *source = node->data; - - if (unix_child_watch_source_waitpid (source)) - wake_source ((GSource *) source); + + 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); + } } } @@ -4758,58 +4699,6 @@ 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 @@ -4842,7 +4731,26 @@ g_child_watch_source_new_nowait (GPid pid) GSource * g_child_watch_source_new (GPid pid) { - return g_child_watch_source_new_internal (pid, FALSE); + 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; } /**