[glib] Revert "gmain: Add private API to create Unix child watch that uses waitid()"

[Michael Natterer <mitch src gnome org>]

commit 49db979922283364238a5c208671591f713ecd78
Author: Michael Natterer <mitch gimp org>
Date:   Thu Nov 15 15:23:07 2012 +0100

    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 7dd0124..8badfe3 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 c2346c5..e7838a6 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 1ea840d..23bfb36 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 6b43126..c1776a8 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;
 }
 
 /**