Revert "gmain: Add private API to create Unix child watch that uses waitid()"
authorMichael Natterer <mitch@gimp.org>
Thu, 15 Nov 2012 14:23:07 +0000 (15:23 +0100)
committerMichael Natterer <mitch@gimp.org>
Thu, 15 Nov 2012 14:33:38 +0000 (15:33 +0100)
This reverts commit 93bf37ce1507380f74d4cb4cab6640fc7d2eb7d1.

configure.ac
glib/glib-private.c
glib/glib-private.h
glib/gmain.c

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