[PATCH 1/3] support: Add support_process_state_wait

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/3] support: Add support_process_state_wait

Adhemerval Zanella-2
It allows parent process to wait for children state using a polling
strategy over procfs on Linux.  The polling is used over ptrace to
avoid the need to handle signals on the target pid and to handle some
system specific limitations (such as YAMA).

The polling has some limitations, such as resource consumption due
the procfs read in a loop and the lack of synchronization after the
state is obtained.

The interface idea is to simplify some sleep synchronization in waitid
tests.
---
 support/Makefile                    |   2 +
 support/process_state.h             |  41 +++++++++++
 support/support_process_state.c     |  89 ++++++++++++++++++++++++
 support/tst-support-process_state.c | 101 ++++++++++++++++++++++++++++
 4 files changed, 233 insertions(+)
 create mode 100644 support/process_state.h
 create mode 100644 support/support_process_state.c
 create mode 100644 support/tst-support-process_state.c

diff --git a/support/Makefile b/support/Makefile
index 23c6d74627..b0d28200af 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -56,6 +56,7 @@ libsupport-routines = \
   support_format_hostent \
   support_format_netent \
   support_isolate_in_subprocess \
+  support_process_state \
   support_ptrace \
   support_openpty \
   support_paths \
@@ -223,6 +224,7 @@ tests = \
   tst-support_capture_subprocess \
   tst-support_descriptors \
   tst-support_format_dns_packet \
+  tst-support-process_state \
   tst-support_quote_blob \
   tst-support_quote_string \
   tst-support_record_failure \
diff --git a/support/process_state.h b/support/process_state.h
new file mode 100644
index 0000000000..d616f0e3e6
--- /dev/null
+++ b/support/process_state.h
@@ -0,0 +1,41 @@
+/* Wait for process state.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef SUPPORT_PROCESS_STATE_H
+#define SUPPORT_PROCESS_STATE_H
+
+#include <sys/types.h>
+
+enum process_state_t
+{
+  process_state_running      = 0x01,  /* R (running)  */
+  process_state_sleeping     = 0x02,  /* S (sleeping)  */
+  process_state_disk_sleep   = 0x04,  /* D (disk sleep)  */
+  process_state_stopped      = 0x08,  /* T (stopped)  */
+  process_state_tracing_stop = 0x10,  /* t (tracing stop)  */
+  process_state_dead         = 0x20,  /* X (dead)  */
+  process_state_zombie       = 0x40,  /* Z (zombie)  */
+  process_state_parked       = 0x80,  /* P (parked)  */
+};
+
+/* Wait for process PID to reach state STATE.  It can be a combination of
+   multiple possible states ('process_state_running | process_state_sleeping')
+   where the function return when any of these state are observed.  */
+void support_process_state_wait (pid_t pid, enum process_state_t state);
+
+#endif
diff --git a/support/support_process_state.c b/support/support_process_state.c
new file mode 100644
index 0000000000..77c684c49c
--- /dev/null
+++ b/support/support_process_state.c
@@ -0,0 +1,89 @@
+/* Wait for process state.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <time.h>
+
+#include <array_length.h>
+
+#include <support/process_state.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+void
+support_process_state_wait (pid_t pid, enum process_state_t state)
+{
+#ifdef __linux__
+  /* For Linux it does a polling check on /proc/<pid>/status checking on
+     third field.  */
+
+  /* It mimics the kernel states from fs/proc/array.c  */
+  static struct process_states_t
+  {
+    enum process_state_t s;
+    char v;
+  } process_states[] = {
+    { process_state_running,      'R' },
+    { process_state_sleeping,     'S' },
+    { process_state_disk_sleep,   'D' },
+    { process_state_stopped,      'T' },
+    { process_state_tracing_stop, 't' },
+    { process_state_dead,         'X' },
+    { process_state_zombie,       'Z' },
+    { process_state_parked,       'P' },
+  };
+
+  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
+ + 1];
+  snprintf (proc_path, sizeof (proc_path), "/proc/%i/stat", pid);
+
+  int fd = xopen (proc_path, O_RDONLY, 0600);
+
+  char statbuf[3 * sizeof (pid_t)  /* pid.  */
+               + 2                 /* ' (' */
+               + 64                /* task name.  */
+               + 2                 /* ' )'  */
+               + 1                 /* <state>, 1 char.  */
+               + 1];               /* Final \0.  */
+  for (;;)
+    {
+      ssize_t ret = read (fd, statbuf, sizeof (statbuf) - 1);
+      TEST_VERIFY (ret >= 0);
+      statbuf[ret] = '\0';
+
+      char cur_state;
+      int sret = sscanf (statbuf, "%*i %*s %c", &cur_state);
+      TEST_VERIFY (sret == 1);
+
+      for (size_t i = 0; i < array_length (process_states); ++i)
+ if (state & process_states[i].s && cur_state == process_states[i].v)
+  {
+    close (fd);
+    return;
+  }
+
+      xlseek (fd, 0, SEEK_SET);
+      nanosleep (&(struct timespec) { 0, 10000000 }, NULL);
+    }
+
+  close (fd);
+#else
+  sleep (2);
+#endif
+}
diff --git a/support/tst-support-process_state.c b/support/tst-support-process_state.c
new file mode 100644
index 0000000000..3482f12fa5
--- /dev/null
+++ b/support/tst-support-process_state.c
@@ -0,0 +1,101 @@
+/* Wait for process state tests.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <support/test-driver.h>
+#include <support/process_state.h>
+#include <support/check.h>
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+
+static void
+sigusr1_handler (int signo)
+{
+}
+
+static void
+test_child (void)
+{
+  xsignal (SIGUSR1, sigusr1_handler);
+
+  raise (SIGSTOP);
+
+  TEST_COMPARE (pause (), -1);
+  TEST_COMPARE (errno, EINTR);
+
+  while (1)
+    asm ("");
+}
+
+static int
+do_test (void)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    {
+      test_child ();
+      _exit (127);
+    }
+
+  /* Adding process_state_tracing_stop ('t') allows the test to work under
+     trace programs such as ptrace.  */
+  enum process_state_t stop_state = process_state_stopped
+    | process_state_tracing_stop;
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_stopped/state_tracing_stop\n",
+    (int) pid);
+  support_process_state_wait (pid, stop_state);
+
+  if (kill (pid, SIGCONT) != 0)
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_sleeping\n", (int) pid);
+  support_process_state_wait (pid, process_state_sleeping);
+
+  if (kill (pid, SIGUSR1) != 0)
+    FAIL_RET ("kill (%d, SIGUSR1): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_running\n", (int) pid);
+  support_process_state_wait (pid, process_state_running);
+
+  if (kill (pid, SIGKILL) != 0)
+    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_zombie\n", (int) pid);
+  support_process_state_wait (pid, process_state_zombie);
+
+  siginfo_t info;
+  int r = waitid (P_PID, pid, &info, WEXITED);
+  TEST_COMPARE (r, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/3] posix: Refactor tst-waitid

Adhemerval Zanella-2
The main changes are:

  - Adapt to libsupport.
  - Synchronize the signal handler using atomics.
  - Replace waitpid by waitid calls.
  - Use support_process_state_wait to wait for child state.
  - Add tests for P_PGID and P_ALL.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 posix/tst-waitid.c | 519 +++++++++++++--------------------------------
 1 file changed, 153 insertions(+), 366 deletions(-)

diff --git a/posix/tst-waitid.c b/posix/tst-waitid.c
index c82ec2b566..1134d92bb3 100644
--- a/posix/tst-waitid.c
+++ b/posix/tst-waitid.c
@@ -22,13 +22,20 @@
 #include <unistd.h>
 #include <sys/wait.h>
 #include <signal.h>
+#include <time.h>
+#include <stdatomic.h>
+#include <stdbool.h>
+
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+#include <support/process_state.h>
 
 static void
-test_child (void)
+test_child (bool setgroup)
 {
-  /* Wait a second to be sure the parent set his variables before we
-     produce a SIGCHLD.  */
-  sleep (1);
+  if (setgroup)
+    TEST_COMPARE (setpgid (0, 0), 0);
 
   /* First thing, we stop ourselves.  */
   raise (SIGSTOP);
@@ -44,20 +51,16 @@ test_child (void)
 # define WSTOPPED WUNTRACED
 #endif
 
-static sig_atomic_t expecting_sigchld, spurious_sigchld;
+static atomic_int expecting_sigchld, spurious_sigchld;
 #ifdef SA_SIGINFO
 static siginfo_t sigchld_info;
 
 static void
 sigchld (int signo, siginfo_t *info, void *ctx)
 {
-  if (signo != SIGCHLD)
-    {
-      printf ("SIGCHLD handler got signal %d instead!\n", signo);
-      _exit (EXIT_FAILURE);
-    }
+  TEST_COMPARE (signo, SIGCHLD);
 
-  if (! expecting_sigchld)
+  if (expecting_sigchld == 0)
     {
       spurious_sigchld = 1;
       printf ("spurious SIGCHLD: signo %d code %d status %d pid %d\n",
@@ -69,154 +72,66 @@ sigchld (int signo, siginfo_t *info, void *ctx)
       expecting_sigchld = 0;
     }
 }
+#endif
 
 static void
-check_sigchld (const char *phase, int *ok, int code, int status, pid_t pid)
+check_sigchld (int code, int status, pid_t pid)
 {
-  if (expecting_sigchld)
-    {
-      printf ("missing SIGCHLD on %s\n", phase);
-      *ok = EXIT_FAILURE;
-      expecting_sigchld = 0;
-      return;
-    }
-
-  if (sigchld_info.si_signo != SIGCHLD)
-    {
-      printf ("SIGCHLD for %s signal %d\n", phase, sigchld_info.si_signo);
-      *ok = EXIT_FAILURE;
-    }
-  if (sigchld_info.si_code != code)
-    {
-      printf ("SIGCHLD for %s code %d\n", phase, sigchld_info.si_code);
-      *ok = EXIT_FAILURE;
-    }
-  if (sigchld_info.si_status != status)
-    {
-      printf ("SIGCHLD for %s status %d\n", phase, sigchld_info.si_status);
-      *ok = EXIT_FAILURE;
-    }
-  if (sigchld_info.si_pid != pid)
-    {
-      printf ("SIGCHLD for %s pid %d\n", phase, sigchld_info.si_pid);
-      *ok = EXIT_FAILURE;
-    }
-}
-# define CHECK_SIGCHLD(phase, code_check, status_check) \
-  check_sigchld ((phase), &status, (code_check), (status_check), pid)
-#else
-# define CHECK_SIGCHLD(phase, code, status) ((void) 0)
+#ifdef SA_SIGINFO
+  TEST_COMPARE (expecting_sigchld, 0);
+  TEST_COMPARE (sigchld_info.si_signo, SIGCHLD);
+  TEST_COMPARE (sigchld_info.si_code, code);
+  TEST_COMPARE (sigchld_info.si_status, status);
+  TEST_COMPARE (sigchld_info.si_pid, pid);
 #endif
+}
 
 static int
-do_test (int argc, char *argv[])
+do_test_waitd_common (idtype_t type, pid_t pid)
 {
-#ifdef SA_SIGINFO
-  struct sigaction sa;
-  sa.sa_flags = SA_SIGINFO|SA_RESTART;
-  sa.sa_sigaction = &sigchld;
-  if (sigemptyset (&sa.sa_mask) < 0 || sigaction (SIGCHLD, &sa, NULL) < 0)
-    {
-      printf ("setting SIGCHLD handler: %m\n");
-      return EXIT_FAILURE;
-    }
-#endif
-
-  expecting_sigchld = 1;
-
-  pid_t pid = fork ();
-  if (pid < 0)
-    {
-      printf ("fork: %m\n");
-      return EXIT_FAILURE;
-    }
-  else if (pid == 0)
-    {
-      test_child ();
-      _exit (127);
-    }
+  /* Adding process_state_tracing_stop ('t') allows the test to work under
+     trace programs such as ptrace.  */
+  enum process_state_t stop_state = process_state_stopped
+    | process_state_tracing_stop;
 
-  int status = EXIT_SUCCESS;
-#define RETURN(ok) \
-    do { if (status == EXIT_SUCCESS) status = (ok); goto out; } while (0)
+  support_process_state_wait (pid, stop_state);
 
-  /* Give the child a chance to stop.  */
-  sleep (3);
+  /* Wait the SIGCHLD handler to act.  */
+  while (expecting_sigchld == 1);
 
-  CHECK_SIGCHLD ("stopped (before waitid)", CLD_STOPPED, SIGSTOP);
+  check_sigchld (CLD_STOPPED, SIGSTOP, pid);
 
   /* Now try a wait that should not succeed.  */
   siginfo_t info;
+  int fail;
+
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
-  int fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WNOHANG on stopped: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo == 0)
- break;
-      if (info.si_signo == SIGCHLD)
- printf ("waitid WNOHANG on stopped status %d\n", info.si_status);
-      else
- printf ("waitid WNOHANG on stopped signal %d\n", info.si_signo);
-      RETURN (EXIT_FAILURE);
-    }
+  fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WNOHANG on stopped: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, 0);
 
   /* Next the wait that should succeed right away.  */
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
   fail = waitid (P_PID, pid, &info, WSTOPPED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WSTOPPED|WNOHANG returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WSTOPPED|WNOHANG on stopped: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped signal %d\n",
-  info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_STOPPED)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped code %d\n",
-  info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGSTOP)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped status %d\n",
-  info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped pid %d != %d\n",
-  info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WNOHANG on stopped: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_STOPPED);
+  TEST_COMPARE (info.si_status, SIGSTOP);
+  TEST_COMPARE (info.si_pid, pid);
 
   expecting_sigchld = WCONTINUED != 0;
 
   if (kill (pid, SIGCONT) != 0)
-    {
-      printf ("kill (%d, SIGCONT): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
 
   /* Wait for the child to have continued.  */
-  sleep (2);
+  support_process_state_wait (pid, process_state_sleeping);
 
 #if WCONTINUED != 0
   if (expecting_sigchld)
@@ -224,152 +139,68 @@ do_test (int argc, char *argv[])
       printf ("no SIGCHLD seen for SIGCONT (optional)\n");
       expecting_sigchld = 0;
     }
-  else
-    CHECK_SIGCHLD ("continued (before waitid)", CLD_CONTINUED, SIGCONT);
+  check_sigchld (CLD_CONTINUED, SIGCONT, pid);
 
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
   fail = waitid (P_PID, pid, &info, WCONTINUED|WNOWAIT);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WCONTINUED|WNOWAIT returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WCONTINUED|WNOWAIT on continued: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued signal %d\n",
-  info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_CONTINUED)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued code %d\n",
-  info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGCONT)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued status %d\n",
-  info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued pid %d != %d\n",
-  info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WCONTINUED|WNOWAIT on continued: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_CONTINUED);
+  TEST_COMPARE (info.si_status, SIGCONT);
+  TEST_COMPARE (info.si_pid, pid);
 
   /* That should leave the CLD_CONTINUED state waiting to be seen again.  */
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
   fail = waitid (P_PID, pid, &info, WCONTINUED);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WCONTINUED returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WCONTINUED on continued: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WCONTINUED on continued signal %d\n", info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_CONTINUED)
- {
-  printf ("waitid WCONTINUED on continued code %d\n", info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGCONT)
- {
-  printf ("waitid WCONTINUED on continued status %d\n",
-  info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WCONTINUED on continued pid %d != %d\n",
-  info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WCONTINUED on continued: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_CONTINUED);
+  TEST_COMPARE (info.si_status, SIGCONT);
+  TEST_COMPARE (info.si_pid, pid);
 
   /* Now try a wait that should not succeed.  */
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   fail = waitid (P_PID, pid, &info, WCONTINUED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WCONTINUED|WNOHANG on waited continued: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo == 0)
- break;
-      if (info.si_signo == SIGCHLD)
- printf ("waitid WCONTINUED|WNOHANG on waited continued status %d\n",
- info.si_status);
-      else
- printf ("waitid WCONTINUED|WNOHANG on waited continued signal %d\n",
- info.si_signo);
-      RETURN (EXIT_FAILURE);
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WCONTINUED|WNOHANG on waited continued: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, 0);
 
   /* Now stop him again and test waitpid with WCONTINUED.  */
   expecting_sigchld = 1;
   if (kill (pid, SIGSTOP) != 0)
-    {
-      printf ("kill (%d, SIGSTOP): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGSTOP): %m\n", pid);
 
-  /* Give the child a chance to stop.  The waitpid call below will block
-     until it has stopped, but if we are real quick and enter the waitpid
-     system call before the SIGCHLD has been generated, then it will be
-     discarded and never delivered.  */
-  sleep (3);
+  /* Wait the child stop.  The waitid call below will block until it has
+     stopped, but if we are real quick and enter the waitid system call
+     before the SIGCHLD has been generated, then it will be discarded and
+     never delivered.  */
+  support_process_state_wait (pid, stop_state);
 
-  pid_t wpid = waitpid (pid, &fail, WUNTRACED);
-  if (wpid < 0)
-    {
-      printf ("waitpid WUNTRACED on stopped: %m\n");
-      RETURN (EXIT_FAILURE);
-    }
-  else if (wpid != pid)
-    {
-      printf ("waitpid WUNTRACED on stopped returned %d != %d (status %x)\n",
-      wpid, pid, fail);
-      RETURN (EXIT_FAILURE);
-    }
-  else if (!WIFSTOPPED (fail) || WIFSIGNALED (fail) || WIFEXITED (fail)
-   || WIFCONTINUED (fail) || WSTOPSIG (fail) != SIGSTOP)
-    {
-      printf ("waitpid WUNTRACED on stopped: status %x\n", fail);
-      RETURN (EXIT_FAILURE);
-    }
-  CHECK_SIGCHLD ("stopped (after waitpid)", CLD_STOPPED, SIGSTOP);
+  fail = waitid (type, pid, &info, WEXITED|WSTOPPED);
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_STOPPED);
+  TEST_COMPARE (info.si_status, SIGSTOP);
+  TEST_COMPARE (info.si_pid, pid);
+
+  check_sigchld (CLD_STOPPED, SIGSTOP, pid);
 
   expecting_sigchld = 1;
+
   if (kill (pid, SIGCONT) != 0)
-    {
-      printf ("kill (%d, SIGCONT): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
 
   /* Wait for the child to have continued.  */
-  sleep (2);
+  support_process_state_wait (pid, process_state_sleeping);
 
   if (expecting_sigchld)
     {
@@ -377,142 +208,98 @@ do_test (int argc, char *argv[])
       expecting_sigchld = 0;
     }
   else
-    CHECK_SIGCHLD ("continued (before waitpid)", CLD_CONTINUED, SIGCONT);
-
-  wpid = waitpid (pid, &fail, WCONTINUED);
-  if (wpid < 0)
-    {
-      if (errno == EINVAL)
- printf ("waitpid does not support WCONTINUED\n");
-      else
- {
-  printf ("waitpid WCONTINUED on continued: %m\n");
-  RETURN (EXIT_FAILURE);
- }
-    }
-  else if (wpid != pid)
-    {
-      printf ("\
-waitpid WCONTINUED on continued returned %d != %d (status %x)\n",
-     wpid, pid, fail);
-      RETURN (EXIT_FAILURE);
-    }
-  else if (WIFSTOPPED (fail) || WIFSIGNALED (fail) || WIFEXITED (fail)
-   || !WIFCONTINUED (fail))
-    {
-      printf ("waitpid WCONTINUED on continued: status %x\n", fail);
-      RETURN (EXIT_FAILURE);
-    }
+    check_sigchld (CLD_CONTINUED, SIGCONT, pid);
+
+  fail = waitid (type, pid, &info, WCONTINUED);
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_CONTINUED);
+  TEST_COMPARE (info.si_status, SIGCONT);
+  TEST_COMPARE (info.si_pid, pid);
 #endif
 
   expecting_sigchld = 1;
 
   /* Die, child, die!  */
   if (kill (pid, SIGKILL) != 0)
-    {
-      printf ("kill (%d, SIGKILL): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
 
 #ifdef WNOWAIT
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
-  fail = waitid (P_PID, pid, &info, WEXITED|WNOWAIT);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WNOWAIT returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WNOWAIT on killed: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WNOWAIT on killed signal %d\n", info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_KILLED)
- {
-  printf ("waitid WNOWAIT on killed code %d\n", info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGKILL)
- {
-  printf ("waitid WNOWAIT on killed status %d\n", info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WNOWAIT on killed pid %d != %d\n", info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  fail = waitid (type, pid, &info, WEXITED|WNOWAIT);
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WNOHANG on killed: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
 #else
-  /* Allow enough time to be sure the child died; we didn't synchronize.  */
-  sleep (2);
+  support_process_state_wait (pid, process_state_zombie);
 #endif
-
-  CHECK_SIGCHLD ("killed", CLD_KILLED, SIGKILL);
+  check_sigchld (CLD_KILLED, SIGKILL, pid);
 
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
-  fail = waitid (P_PID, pid, &info, WEXITED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WNOHANG returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WNOHANG on killed: %m\n");
-      RETURN (EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WNOHANG on killed signal %d\n", info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_KILLED)
- {
-  printf ("waitid WNOHANG on killed code %d\n", info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGKILL)
- {
-  printf ("waitid WNOHANG on killed status %d\n", info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WNOHANG on killed pid %d != %d\n", info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  fail = waitid (type, pid, &info, WEXITED | WNOHANG);
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
 
   fail = waitid (P_PID, pid, &info, WEXITED);
-  if (fail == -1)
-    {
-      if (errno != ECHILD)
- {
-  printf ("waitid WEXITED on killed: %m\n");
-  RETURN (EXIT_FAILURE);
- }
-    }
-  else
+  TEST_COMPARE (fail, -1);
+  TEST_COMPARE (errno, ECHILD);
+
+  return 0;
+}
+
+static int
+do_test_waitid (idtype_t type)
+{
+  expecting_sigchld = 1;
+  spurious_sigchld = 0;
+
+#ifdef SA_SIGINFO
+  {
+    struct sigaction sa;
+    sa.sa_flags = SA_SIGINFO | SA_RESTART;
+    sa.sa_sigaction = sigchld;
+    sigemptyset (&sa.sa_mask);
+    xsigaction (SIGCHLD, &sa, NULL);
+  }
+#endif
+
+  pid_t pid = xfork ();
+  if (pid == 0)
     {
-      printf ("waitid WEXITED returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
+      test_child (type == P_PGID || type == P_ALL);
+      _exit (127);
     }
 
-#undef RETURN
- out:
+  int ret = do_test_waitd_common (type, pid);
+
   if (spurious_sigchld)
-    status = EXIT_FAILURE;
-  signal (SIGCHLD, SIG_IGN);
+    ret = EXIT_FAILURE;
+  xsignal (SIGCHLD, SIG_IGN);
   kill (pid, SIGKILL); /* Make sure it's dead if we bailed early.  */
-  return status;
+  return ret;
+}
+
+static int
+do_test (void)
+{
+  int ret = 0;
+
+  ret |= do_test_waitid (P_PID);
+  ret |= do_test_waitid (P_PGID);
+  ret |= do_test_waitid (P_ALL);
+
+  return ret;
 }
 
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/3] posix: Remove posix waitid

Adhemerval Zanella-2
In reply to this post by Adhemerval Zanella-2
The POSIX waitid implementation is problematic in some ways:

  - It emulates using waitpid, which default implementation calls
    wait4 and wait4 returns ENOSYS as default.

  - Also by using waitpid it does not allod support the WNOWAIT,
    WEXITED, WSTOPPED, or WCONTINUED flag.  With current POSIX
    specification the flags are no longer marked as optional.

Also due BZ#23091 Hurd still uses the implementation, so it is moved
to as a Hurd arch-specific folder (with some minor cleanups).

Checked against a i686-gnu (run-built-tests=no)
---
 sysdeps/{posix => mach/hurd}/waitid.c | 70 ++++-----------------------
 1 file changed, 9 insertions(+), 61 deletions(-)
 rename sysdeps/{posix => mach/hurd}/waitid.c (65%)

diff --git a/sysdeps/posix/waitid.c b/sysdeps/mach/hurd/waitid.c
similarity index 65%
rename from sysdeps/posix/waitid.c
rename to sysdeps/mach/hurd/waitid.c
index 0942623553..d28d0034da 100644
--- a/sysdeps/posix/waitid.c
+++ b/sysdeps/mach/hurd/waitid.c
@@ -17,25 +17,11 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <assert.h>
 #include <errno.h>
-#include <signal.h>
-#define __need_NULL
-#include <stddef.h>
 #include <sys/wait.h>
-#include <sys/types.h>
-#include <sysdep-cancel.h>
 
-
-#ifdef DO_WAITID
-# define OUR_WAITID DO_WAITID
-#elif !defined NO_DO_WAITID
-# define OUR_WAITID do_waitid
-#endif
-
-#ifdef OUR_WAITID
-static int
-OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
+int
+__waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options)
 {
   pid_t pid, child;
   int status;
@@ -43,7 +29,7 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
   switch (idtype)
     {
     case P_PID:
-      if(id <= 0)
+      if (id <= 0)
  goto invalid;
       pid = (pid_t) id;
       break;
@@ -72,31 +58,10 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
       return -1;
     }
 
-  /* This emulation using waitpid cannot support the waitid modes in which
-     we do not reap the child, or match only stopped and not dead children.  */
-  if (0
-#ifdef WNOWAIT
-      || (options & WNOWAIT)
-#endif
-#ifdef WEXITED
-      || ((options & (WEXITED|WSTOPPED|WCONTINUED))
-  != (WEXITED | (options & WSTOPPED)))
-#endif
-      )
-    {
-      __set_errno (ENOTSUP);
-      return -1;
-    }
-
   /* Note the waitid() is a cancellation point.  But since we call
      waitpid() which itself is a cancellation point we do not have
      to do anything here.  */
-  child = __waitpid (pid, &status,
-     options
-#ifdef WEXITED
-     &~ WEXITED
-#endif
-     );
+  child = __waitpid (pid, &status, options);
 
   if (child == -1)
     /* `waitpid' set `errno' for us.  */
@@ -104,9 +69,11 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
 
   if (child == 0)
     {
-      /* The WHOHANG bit in OPTIONS is set and there are children available
- but none has a status for us.  The XPG docs do not mention this
- case so we clear the `siginfo_t' struct and return successfully.  */
+      /* POSIX.1-2008, Technical Corrigendum 1 XSH/TC1-2008/0713 [153] states
+ that if waitid returns because WNOHANG was specified and status is
+ not available for any process specified by idtype and id, then the
+ si_signo and si_pid members of the structure pointed to by infop
+ shall be set to zero.  */
       infop->si_signo = 0;
       infop->si_code = 0;
       return 0;
@@ -132,27 +99,8 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
       infop->si_code = CLD_STOPPED;
       infop->si_status = WSTOPSIG (status);
     }
-#ifdef WIFCONTINUED
-  else if (WIFCONTINUED (status))
-    {
-      infop->si_code = CLD_CONTINUED;
-      infop->si_status = SIGCONT;
-    }
-#endif
-  else
-    /* Can't happen. */
-    assert (! "What?");
 
   return 0;
 }
-#endif
-
-
-int
-__waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options)
-{
-  /* __waitpid should be a cancellation point.  */
-  return do_waitid (idtype, id, infop, options);
-}
 weak_alias (__waitid, waitid)
 strong_alias (__waitid, __libc_waitid)
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] posix: Remove posix waitid

Christian Brauner-2
On Wed, Nov 20, 2019 at 11:15:35AM -0300, Adhemerval Zanella wrote:

> The POSIX waitid implementation is problematic in some ways:
>
>   - It emulates using waitpid, which default implementation calls
>     wait4 and wait4 returns ENOSYS as default.
>
>   - Also by using waitpid it does not allod support the WNOWAIT,
>     WEXITED, WSTOPPED, or WCONTINUED flag.  With current POSIX
>     specification the flags are no longer marked as optional.
>
> Also due BZ#23091 Hurd still uses the implementation, so it is moved
> to as a Hurd arch-specific folder (with some minor cleanups).
>
> Checked against a i686-gnu (run-built-tests=no)

I haven't followed that thread too closely, so sorry for maybe redundant
questions.
Does this mean you're removing support for the waitid() _syscall_ from
glibc? How do you intend to deal with P_PIDFD released with kernel 5.3?

Christian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] posix: Remove posix waitid

Adhemerval Zanella-2


On 20/11/2019 12:14, Christian Brauner wrote:

> On Wed, Nov 20, 2019 at 11:15:35AM -0300, Adhemerval Zanella wrote:
>> The POSIX waitid implementation is problematic in some ways:
>>
>>   - It emulates using waitpid, which default implementation calls
>>     wait4 and wait4 returns ENOSYS as default.
>>
>>   - Also by using waitpid it does not allod support the WNOWAIT,
>>     WEXITED, WSTOPPED, or WCONTINUED flag.  With current POSIX
>>     specification the flags are no longer marked as optional.
>>
>> Also due BZ#23091 Hurd still uses the implementation, so it is moved
>> to as a Hurd arch-specific folder (with some minor cleanups).
>>
>> Checked against a i686-gnu (run-built-tests=no)
>
> I haven't followed that thread too closely, so sorry for maybe redundant
> questions.
> Does this mean you're removing support for the waitid() _syscall_ from
> glibc? How do you intend to deal with P_PIDFD released with kernel 5.3?
>
> Christian
>

No, this patch moves the POSIX emulation at sysdeps/posix/waitid.c based
on waitpid to a Hurd specific implementation.  Linux will still be done
by the generic syscalls.list, meaning it will a direct syscall.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] posix: Refactor tst-waitid

Joseph Myers
In reply to this post by Adhemerval Zanella-2
This reminds me of the bug we have,
<https://sourceware.org/bugzilla/show_bug.cgi?id=14666>, for intermittent
failures of tst-waitid (I think the discussion it references starts at
<https://sourceware.org/ml/libc-alpha/2010-09/msg00030.html>).

Was that fixed by commit a58ad3f801960fa0dc0bb1106eb0d99f7ebd77b1 in 2013,
or is it fixed by your patch, or is it an unrelated issue?

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] posix: Refactor tst-waitid

Adhemerval Zanella-2


On 20/11/2019 14:33, Joseph Myers wrote:
> This reminds me of the bug we have,
> <https://sourceware.org/bugzilla/show_bug.cgi?id=14666>, for intermittent
> failures of tst-waitid (I think the discussion it references starts at
> <https://sourceware.org/ml/libc-alpha/2010-09/msg00030.html>).
>
> Was that fixed by commit a58ad3f801960fa0dc0bb1106eb0d99f7ebd77b1 in 2013,
> or is it fixed by your patch, or is it an unrelated issue?
>

My initial idea of replacing the sleep added by a58ad3f801960f is to
loop atomically over a variable which is set by the signal handler.
However I think a better strategy would be to use sigwaitinfo instead
as Roland has suggested later in the thread.  It allows clean up the
test a little more.

And BZ#14666 does seems to be same issue mitigated by a58ad3f801960f.

My idea of using support_process_state_wait in this case is to avoid
use timeouts to sync process/threads and move towards some ideas laid
out on a recent thread on how to speed up the testsuite spent time [1]

Below I updated the patch to use sigwaitinfo instead of global variables
set by the signal handler.

[1] https://sourceware.org/ml/libc-alpha/2019-07/msg00403.html

--

[PATCH v2 2/3] posix: Refactor tst-waitid (BZ #14666)

The main changes are:

  - Adapt to libsupport.
  - Synchronize the signal handler using atomics.
  - Replace waitpid by waitid calls.
  - Use support_process_state_wait to wait for child state.
  - Add tests for P_PGID and P_ALL.
  - Use sigwaitinfo instead of global state set by the signal handler.

Checked on x86_64-linux-gnu and i686-linux-gnu.  It fixes BZ #14666.

---

diff --git a/posix/tst-waitid.c b/posix/tst-waitid.c
index c82ec2b566..808bf6a8eb 100644
--- a/posix/tst-waitid.c
+++ b/posix/tst-waitid.c
@@ -22,13 +22,20 @@
 #include <unistd.h>
 #include <sys/wait.h>
 #include <signal.h>
+#include <time.h>
+#include <stdatomic.h>
+#include <stdbool.h>
+
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+#include <support/process_state.h>
 
 static void
-test_child (void)
+test_child (bool setgroup)
 {
-  /* Wait a second to be sure the parent set his variables before we
-     produce a SIGCHLD.  */
-  sleep (1);
+  if (setgroup)
+    TEST_COMPARE (setpgid (0, 0), 0);
 
   /* First thing, we stop ourselves.  */
   raise (SIGSTOP);
@@ -44,475 +51,239 @@ test_child (void)
 # define WSTOPPED WUNTRACED
 #endif
 
-static sig_atomic_t expecting_sigchld, spurious_sigchld;
-#ifdef SA_SIGINFO
-static siginfo_t sigchld_info;
+/* Set with only SIGCHLD on do_test_waitid.  */
+static sigset_t chldset;
 
+#ifdef SA_SIGINFO
 static void
 sigchld (int signo, siginfo_t *info, void *ctx)
 {
-  if (signo != SIGCHLD)
-    {
-      printf ("SIGCHLD handler got signal %d instead!\n", signo);
-      _exit (EXIT_FAILURE);
-    }
-
-  if (! expecting_sigchld)
-    {
-      spurious_sigchld = 1;
-      printf ("spurious SIGCHLD: signo %d code %d status %d pid %d\n",
-      info->si_signo, info->si_code, info->si_status, info->si_pid);
-    }
-  else
-    {
-      sigchld_info = *info;
-      expecting_sigchld = 0;
-    }
 }
+#endif
 
 static void
-check_sigchld (const char *phase, int *ok, int code, int status, pid_t pid)
+check_sigchld (int code, int status, pid_t pid)
 {
-  if (expecting_sigchld)
-    {
-      printf ("missing SIGCHLD on %s\n", phase);
-      *ok = EXIT_FAILURE;
-      expecting_sigchld = 0;
-      return;
-    }
+#ifdef SA_SIGINFO
+  siginfo_t siginfo;
+  TEST_COMPARE (sigwaitinfo (&chldset, &siginfo), SIGCHLD);
 
-  if (sigchld_info.si_signo != SIGCHLD)
-    {
-      printf ("SIGCHLD for %s signal %d\n", phase, sigchld_info.si_signo);
-      *ok = EXIT_FAILURE;
-    }
-  if (sigchld_info.si_code != code)
-    {
-      printf ("SIGCHLD for %s code %d\n", phase, sigchld_info.si_code);
-      *ok = EXIT_FAILURE;
-    }
-  if (sigchld_info.si_status != status)
-    {
-      printf ("SIGCHLD for %s status %d\n", phase, sigchld_info.si_status);
-      *ok = EXIT_FAILURE;
-    }
-  if (sigchld_info.si_pid != pid)
-    {
-      printf ("SIGCHLD for %s pid %d\n", phase, sigchld_info.si_pid);
-      *ok = EXIT_FAILURE;
-    }
-}
-# define CHECK_SIGCHLD(phase, code_check, status_check) \
-  check_sigchld ((phase), &status, (code_check), (status_check), pid)
-#else
-# define CHECK_SIGCHLD(phase, code, status) ((void) 0)
+  TEST_COMPARE (siginfo.si_signo, SIGCHLD);
+  TEST_COMPARE (siginfo.si_code, code);
+  TEST_COMPARE (siginfo.si_status, status);
+  TEST_COMPARE (siginfo.si_pid, pid);
 #endif
+}
 
 static int
-do_test (int argc, char *argv[])
+do_test_waitd_common (idtype_t type, pid_t pid)
 {
-#ifdef SA_SIGINFO
-  struct sigaction sa;
-  sa.sa_flags = SA_SIGINFO|SA_RESTART;
-  sa.sa_sigaction = &sigchld;
-  if (sigemptyset (&sa.sa_mask) < 0 || sigaction (SIGCHLD, &sa, NULL) < 0)
-    {
-      printf ("setting SIGCHLD handler: %m\n");
-      return EXIT_FAILURE;
-    }
-#endif
+  /* Adding process_state_tracing_stop ('t') allows the test to work under
+     trace programs such as ptrace.  */
+  enum process_state_t stop_state = process_state_stopped
+    | process_state_tracing_stop;
 
-  expecting_sigchld = 1;
+  support_process_state_wait (pid, stop_state);
 
-  pid_t pid = fork ();
-  if (pid < 0)
-    {
-      printf ("fork: %m\n");
-      return EXIT_FAILURE;
-    }
-  else if (pid == 0)
-    {
-      test_child ();
-      _exit (127);
-    }
-
-  int status = EXIT_SUCCESS;
-#define RETURN(ok) \
-    do { if (status == EXIT_SUCCESS) status = (ok); goto out; } while (0)
-
-  /* Give the child a chance to stop.  */
-  sleep (3);
-
-  CHECK_SIGCHLD ("stopped (before waitid)", CLD_STOPPED, SIGSTOP);
+  check_sigchld (CLD_STOPPED, SIGSTOP, pid);
 
   /* Now try a wait that should not succeed.  */
   siginfo_t info;
+  int fail;
+
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
-  int fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WNOHANG on stopped: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo == 0)
- break;
-      if (info.si_signo == SIGCHLD)
- printf ("waitid WNOHANG on stopped status %d\n", info.si_status);
-      else
- printf ("waitid WNOHANG on stopped signal %d\n", info.si_signo);
-      RETURN (EXIT_FAILURE);
-    }
+  fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WNOHANG on stopped: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, 0);
 
   /* Next the wait that should succeed right away.  */
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
   fail = waitid (P_PID, pid, &info, WSTOPPED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WSTOPPED|WNOHANG returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WSTOPPED|WNOHANG on stopped: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped signal %d\n",
-  info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_STOPPED)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped code %d\n",
-  info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGSTOP)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped status %d\n",
-  info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WSTOPPED|WNOHANG on stopped pid %d != %d\n",
-  info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WNOHANG on stopped: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_STOPPED);
+  TEST_COMPARE (info.si_status, SIGSTOP);
+  TEST_COMPARE (info.si_pid, pid);
 
-  expecting_sigchld = WCONTINUED != 0;
+  //expecting_sigchld = WCONTINUED != 0;
 
   if (kill (pid, SIGCONT) != 0)
-    {
-      printf ("kill (%d, SIGCONT): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
 
   /* Wait for the child to have continued.  */
-  sleep (2);
+  support_process_state_wait (pid, process_state_sleeping);
 
 #if WCONTINUED != 0
-  if (expecting_sigchld)
-    {
-      printf ("no SIGCHLD seen for SIGCONT (optional)\n");
-      expecting_sigchld = 0;
-    }
-  else
-    CHECK_SIGCHLD ("continued (before waitid)", CLD_CONTINUED, SIGCONT);
+  {
+    sigset_t set;
+    sigemptyset (&set);
+    sigaddset (&set, SIGCHLD);
+    siginfo_t info;
+    TEST_COMPARE (sigwaitinfo (&set, &info), SIGCHLD);
+
+    TEST_COMPARE (info.si_signo, SIGCHLD);
+    TEST_COMPARE (info.si_code, CLD_CONTINUED);
+    TEST_COMPARE (info.si_status, SIGCONT);
+    TEST_COMPARE (info.si_pid, pid);
+  }
 
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
   fail = waitid (P_PID, pid, &info, WCONTINUED|WNOWAIT);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WCONTINUED|WNOWAIT returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WCONTINUED|WNOWAIT on continued: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued signal %d\n",
-  info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_CONTINUED)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued code %d\n",
-  info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGCONT)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued status %d\n",
-  info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WCONTINUED|WNOWAIT on continued pid %d != %d\n",
-  info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WCONTINUED|WNOWAIT on continued: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_CONTINUED);
+  TEST_COMPARE (info.si_status, SIGCONT);
+  TEST_COMPARE (info.si_pid, pid);
 
   /* That should leave the CLD_CONTINUED state waiting to be seen again.  */
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
   fail = waitid (P_PID, pid, &info, WCONTINUED);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WCONTINUED returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WCONTINUED on continued: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WCONTINUED on continued signal %d\n", info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_CONTINUED)
- {
-  printf ("waitid WCONTINUED on continued code %d\n", info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGCONT)
- {
-  printf ("waitid WCONTINUED on continued status %d\n",
-  info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WCONTINUED on continued pid %d != %d\n",
-  info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WCONTINUED on continued: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_CONTINUED);
+  TEST_COMPARE (info.si_status, SIGCONT);
+  TEST_COMPARE (info.si_pid, pid);
 
   /* Now try a wait that should not succeed.  */
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   fail = waitid (P_PID, pid, &info, WCONTINUED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WCONTINUED|WNOHANG on waited continued: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo == 0)
- break;
-      if (info.si_signo == SIGCHLD)
- printf ("waitid WCONTINUED|WNOHANG on waited continued status %d\n",
- info.si_status);
-      else
- printf ("waitid WCONTINUED|WNOHANG on waited continued signal %d\n",
- info.si_signo);
-      RETURN (EXIT_FAILURE);
-    }
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WCONTINUED|WNOHANG on waited continued: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, 0);
 
   /* Now stop him again and test waitpid with WCONTINUED.  */
-  expecting_sigchld = 1;
+  //expecting_sigchld = 1;
   if (kill (pid, SIGSTOP) != 0)
-    {
-      printf ("kill (%d, SIGSTOP): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGSTOP): %m\n", pid);
 
-  /* Give the child a chance to stop.  The waitpid call below will block
-     until it has stopped, but if we are real quick and enter the waitpid
-     system call before the SIGCHLD has been generated, then it will be
-     discarded and never delivered.  */
-  sleep (3);
+  /* Wait the child stop.  The waitid call below will block until it has
+     stopped, but if we are real quick and enter the waitid system call
+     before the SIGCHLD has been generated, then it will be discarded and
+     never delivered.  */
+  support_process_state_wait (pid, stop_state);
 
-  pid_t wpid = waitpid (pid, &fail, WUNTRACED);
-  if (wpid < 0)
-    {
-      printf ("waitpid WUNTRACED on stopped: %m\n");
-      RETURN (EXIT_FAILURE);
-    }
-  else if (wpid != pid)
-    {
-      printf ("waitpid WUNTRACED on stopped returned %d != %d (status %x)\n",
-      wpid, pid, fail);
-      RETURN (EXIT_FAILURE);
-    }
-  else if (!WIFSTOPPED (fail) || WIFSIGNALED (fail) || WIFEXITED (fail)
-   || WIFCONTINUED (fail) || WSTOPSIG (fail) != SIGSTOP)
-    {
-      printf ("waitpid WUNTRACED on stopped: status %x\n", fail);
-      RETURN (EXIT_FAILURE);
-    }
-  CHECK_SIGCHLD ("stopped (after waitpid)", CLD_STOPPED, SIGSTOP);
+  fail = waitid (type, pid, &info, WEXITED|WSTOPPED);
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_STOPPED);
+  TEST_COMPARE (info.si_status, SIGSTOP);
+  TEST_COMPARE (info.si_pid, pid);
+
+  check_sigchld (CLD_STOPPED, SIGSTOP, pid);
 
-  expecting_sigchld = 1;
   if (kill (pid, SIGCONT) != 0)
-    {
-      printf ("kill (%d, SIGCONT): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
 
   /* Wait for the child to have continued.  */
-  sleep (2);
+  support_process_state_wait (pid, process_state_sleeping);
 
-  if (expecting_sigchld)
-    {
-      printf ("no SIGCHLD seen for SIGCONT (optional)\n");
-      expecting_sigchld = 0;
-    }
-  else
-    CHECK_SIGCHLD ("continued (before waitpid)", CLD_CONTINUED, SIGCONT);
+  check_sigchld (CLD_CONTINUED, SIGCONT, pid);
 
-  wpid = waitpid (pid, &fail, WCONTINUED);
-  if (wpid < 0)
-    {
-      if (errno == EINVAL)
- printf ("waitpid does not support WCONTINUED\n");
-      else
- {
-  printf ("waitpid WCONTINUED on continued: %m\n");
-  RETURN (EXIT_FAILURE);
- }
-    }
-  else if (wpid != pid)
-    {
-      printf ("\
-waitpid WCONTINUED on continued returned %d != %d (status %x)\n",
-     wpid, pid, fail);
-      RETURN (EXIT_FAILURE);
-    }
-  else if (WIFSTOPPED (fail) || WIFSIGNALED (fail) || WIFEXITED (fail)
-   || !WIFCONTINUED (fail))
-    {
-      printf ("waitpid WCONTINUED on continued: status %x\n", fail);
-      RETURN (EXIT_FAILURE);
-    }
+  fail = waitid (type, pid, &info, WCONTINUED);
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_CONTINUED);
+  TEST_COMPARE (info.si_status, SIGCONT);
+  TEST_COMPARE (info.si_pid, pid);
 #endif
 
-  expecting_sigchld = 1;
-
   /* Die, child, die!  */
   if (kill (pid, SIGKILL) != 0)
-    {
-      printf ("kill (%d, SIGKILL): %m\n", pid);
-      RETURN (EXIT_FAILURE);
-    }
+    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
 
 #ifdef WNOWAIT
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
-  fail = waitid (P_PID, pid, &info, WEXITED|WNOWAIT);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WNOWAIT returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WNOWAIT on killed: %m\n");
-      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WNOWAIT on killed signal %d\n", info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_KILLED)
- {
-  printf ("waitid WNOWAIT on killed code %d\n", info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGKILL)
- {
-  printf ("waitid WNOWAIT on killed status %d\n", info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WNOWAIT on killed pid %d != %d\n", info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  fail = waitid (type, pid, &info, WEXITED|WNOWAIT);
+  if (fail == -1 && errno == ENOTSUP)
+    FAIL_RET ("waitid WNOHANG on killed: %m");
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
 #else
-  /* Allow enough time to be sure the child died; we didn't synchronize.  */
-  sleep (2);
+  support_process_state_wait (pid, process_state_zombie);
 #endif
-
-  CHECK_SIGCHLD ("killed", CLD_KILLED, SIGKILL);
+  check_sigchld (CLD_KILLED, SIGKILL, pid);
 
   info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
   info.si_pid = -1;
   info.si_status = -1;
-  fail = waitid (P_PID, pid, &info, WEXITED|WNOHANG);
-  switch (fail)
-    {
-    default:
-      printf ("waitid WNOHANG returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
-    case -1:
-      printf ("waitid WNOHANG on killed: %m\n");
-      RETURN (EXIT_FAILURE);
-    case 0:
-      if (info.si_signo != SIGCHLD)
- {
-  printf ("waitid WNOHANG on killed signal %d\n", info.si_signo);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_code != CLD_KILLED)
- {
-  printf ("waitid WNOHANG on killed code %d\n", info.si_code);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_status != SIGKILL)
- {
-  printf ("waitid WNOHANG on killed status %d\n", info.si_status);
-  RETURN (EXIT_FAILURE);
- }
-      if (info.si_pid != pid)
- {
-  printf ("waitid WNOHANG on killed pid %d != %d\n", info.si_pid, pid);
-  RETURN (EXIT_FAILURE);
- }
-    }
+  fail = waitid (type, pid, &info, WEXITED | WNOHANG);
+  TEST_COMPARE (fail, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
 
   fail = waitid (P_PID, pid, &info, WEXITED);
-  if (fail == -1)
-    {
-      if (errno != ECHILD)
- {
-  printf ("waitid WEXITED on killed: %m\n");
-  RETURN (EXIT_FAILURE);
- }
-    }
-  else
+  TEST_COMPARE (fail, -1);
+  TEST_COMPARE (errno, ECHILD);
+
+  return 0;
+}
+
+static int
+do_test_waitid (idtype_t type)
+{
+#ifdef SA_SIGINFO
+  {
+    struct sigaction sa;
+    sa.sa_flags = SA_SIGINFO | SA_RESTART;
+    sa.sa_sigaction = sigchld;
+    sigemptyset (&sa.sa_mask);
+    xsigaction (SIGCHLD, &sa, NULL);
+  }
+#endif
+
+  sigemptyset (&chldset);
+  sigaddset (&chldset, SIGCHLD);
+
+  /* The SIGCHLD shall has blocked at the time of the call to sigwait;
+     otherwise, the behavior is undefined.  */
+  sigprocmask (SIG_BLOCK, &chldset, NULL);
+
+  pid_t pid = xfork ();
+  if (pid == 0)
     {
-      printf ("waitid WEXITED returned bogus value %d\n", fail);
-      RETURN (EXIT_FAILURE);
+      test_child (type == P_PGID || type == P_ALL);
+      _exit (127);
     }
 
-#undef RETURN
- out:
-  if (spurious_sigchld)
-    status = EXIT_FAILURE;
-  signal (SIGCHLD, SIG_IGN);
+  int ret = do_test_waitd_common (type, pid);
+
+  xsignal (SIGCHLD, SIG_IGN);
   kill (pid, SIGKILL); /* Make sure it's dead if we bailed early.  */
-  return status;
+  return ret;
+}
+
+static int
+do_test (void)
+{
+  int ret = 0;
+
+  ret |= do_test_waitid (P_PID);
+  ret |= do_test_waitid (P_PGID);
+  ret |= do_test_waitid (P_ALL);
+
+  return ret;
 }
 
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
--
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] support: Add support_process_state_wait

Adhemerval Zanella-2
In reply to this post by Adhemerval Zanella-2
Ping.

On 20/11/2019 11:15, Adhemerval Zanella wrote:

> It allows parent process to wait for children state using a polling
> strategy over procfs on Linux.  The polling is used over ptrace to
> avoid the need to handle signals on the target pid and to handle some
> system specific limitations (such as YAMA).
>
> The polling has some limitations, such as resource consumption due
> the procfs read in a loop and the lack of synchronization after the
> state is obtained.
>
> The interface idea is to simplify some sleep synchronization in waitid
> tests.
> ---
>  support/Makefile                    |   2 +
>  support/process_state.h             |  41 +++++++++++
>  support/support_process_state.c     |  89 ++++++++++++++++++++++++
>  support/tst-support-process_state.c | 101 ++++++++++++++++++++++++++++
>  4 files changed, 233 insertions(+)
>  create mode 100644 support/process_state.h
>  create mode 100644 support/support_process_state.c
>  create mode 100644 support/tst-support-process_state.c
>
> diff --git a/support/Makefile b/support/Makefile
> index 23c6d74627..b0d28200af 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -56,6 +56,7 @@ libsupport-routines = \
>    support_format_hostent \
>    support_format_netent \
>    support_isolate_in_subprocess \
> +  support_process_state \
>    support_ptrace \
>    support_openpty \
>    support_paths \
> @@ -223,6 +224,7 @@ tests = \
>    tst-support_capture_subprocess \
>    tst-support_descriptors \
>    tst-support_format_dns_packet \
> +  tst-support-process_state \
>    tst-support_quote_blob \
>    tst-support_quote_string \
>    tst-support_record_failure \
> diff --git a/support/process_state.h b/support/process_state.h
> new file mode 100644
> index 0000000000..d616f0e3e6
> --- /dev/null
> +++ b/support/process_state.h
> @@ -0,0 +1,41 @@
> +/* Wait for process state.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef SUPPORT_PROCESS_STATE_H
> +#define SUPPORT_PROCESS_STATE_H
> +
> +#include <sys/types.h>
> +
> +enum process_state_t
> +{
> +  process_state_running      = 0x01,  /* R (running)  */
> +  process_state_sleeping     = 0x02,  /* S (sleeping)  */
> +  process_state_disk_sleep   = 0x04,  /* D (disk sleep)  */
> +  process_state_stopped      = 0x08,  /* T (stopped)  */
> +  process_state_tracing_stop = 0x10,  /* t (tracing stop)  */
> +  process_state_dead         = 0x20,  /* X (dead)  */
> +  process_state_zombie       = 0x40,  /* Z (zombie)  */
> +  process_state_parked       = 0x80,  /* P (parked)  */
> +};
> +
> +/* Wait for process PID to reach state STATE.  It can be a combination of
> +   multiple possible states ('process_state_running | process_state_sleeping')
> +   where the function return when any of these state are observed.  */
> +void support_process_state_wait (pid_t pid, enum process_state_t state);
> +
> +#endif
> diff --git a/support/support_process_state.c b/support/support_process_state.c
> new file mode 100644
> index 0000000000..77c684c49c
> --- /dev/null
> +++ b/support/support_process_state.c
> @@ -0,0 +1,89 @@
> +/* Wait for process state.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <time.h>
> +
> +#include <array_length.h>
> +
> +#include <support/process_state.h>
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +
> +void
> +support_process_state_wait (pid_t pid, enum process_state_t state)
> +{
> +#ifdef __linux__
> +  /* For Linux it does a polling check on /proc/<pid>/status checking on
> +     third field.  */
> +
> +  /* It mimics the kernel states from fs/proc/array.c  */
> +  static struct process_states_t
> +  {
> +    enum process_state_t s;
> +    char v;
> +  } process_states[] = {
> +    { process_state_running,      'R' },
> +    { process_state_sleeping,     'S' },
> +    { process_state_disk_sleep,   'D' },
> +    { process_state_stopped,      'T' },
> +    { process_state_tracing_stop, 't' },
> +    { process_state_dead,         'X' },
> +    { process_state_zombie,       'Z' },
> +    { process_state_parked,       'P' },
> +  };
> +
> +  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
> + + 1];
> +  snprintf (proc_path, sizeof (proc_path), "/proc/%i/stat", pid);
> +
> +  int fd = xopen (proc_path, O_RDONLY, 0600);
> +
> +  char statbuf[3 * sizeof (pid_t)  /* pid.  */
> +               + 2                 /* ' (' */
> +               + 64                /* task name.  */
> +               + 2                 /* ' )'  */
> +               + 1                 /* <state>, 1 char.  */
> +               + 1];               /* Final \0.  */
> +  for (;;)
> +    {
> +      ssize_t ret = read (fd, statbuf, sizeof (statbuf) - 1);
> +      TEST_VERIFY (ret >= 0);
> +      statbuf[ret] = '\0';
> +
> +      char cur_state;
> +      int sret = sscanf (statbuf, "%*i %*s %c", &cur_state);
> +      TEST_VERIFY (sret == 1);
> +
> +      for (size_t i = 0; i < array_length (process_states); ++i)
> + if (state & process_states[i].s && cur_state == process_states[i].v)
> +  {
> +    close (fd);
> +    return;
> +  }
> +
> +      xlseek (fd, 0, SEEK_SET);
> +      nanosleep (&(struct timespec) { 0, 10000000 }, NULL);
> +    }
> +
> +  close (fd);
> +#else
> +  sleep (2);
> +#endif
> +}
> diff --git a/support/tst-support-process_state.c b/support/tst-support-process_state.c
> new file mode 100644
> index 0000000000..3482f12fa5
> --- /dev/null
> +++ b/support/tst-support-process_state.c
> @@ -0,0 +1,101 @@
> +/* Wait for process state tests.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +#include <support/test-driver.h>
> +#include <support/process_state.h>
> +#include <support/check.h>
> +#include <support/xsignal.h>
> +#include <support/xunistd.h>
> +
> +static void
> +sigusr1_handler (int signo)
> +{
> +}
> +
> +static void
> +test_child (void)
> +{
> +  xsignal (SIGUSR1, sigusr1_handler);
> +
> +  raise (SIGSTOP);
> +
> +  TEST_COMPARE (pause (), -1);
> +  TEST_COMPARE (errno, EINTR);
> +
> +  while (1)
> +    asm ("");
> +}
> +
> +static int
> +do_test (void)
> +{
> +  pid_t pid = xfork ();
> +  if (pid == 0)
> +    {
> +      test_child ();
> +      _exit (127);
> +    }
> +
> +  /* Adding process_state_tracing_stop ('t') allows the test to work under
> +     trace programs such as ptrace.  */
> +  enum process_state_t stop_state = process_state_stopped
> +    | process_state_tracing_stop;
> +
> +  if (test_verbose)
> +    printf ("info: waiting pid %d, state_stopped/state_tracing_stop\n",
> +    (int) pid);
> +  support_process_state_wait (pid, stop_state);
> +
> +  if (kill (pid, SIGCONT) != 0)
> +    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
> +
> +  if (test_verbose)
> +    printf ("info: waiting pid %d, state_sleeping\n", (int) pid);
> +  support_process_state_wait (pid, process_state_sleeping);
> +
> +  if (kill (pid, SIGUSR1) != 0)
> +    FAIL_RET ("kill (%d, SIGUSR1): %m\n", pid);
> +
> +  if (test_verbose)
> +    printf ("info: waiting pid %d, state_running\n", (int) pid);
> +  support_process_state_wait (pid, process_state_running);
> +
> +  if (kill (pid, SIGKILL) != 0)
> +    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
> +
> +  if (test_verbose)
> +    printf ("info: waiting pid %d, state_zombie\n", (int) pid);
> +  support_process_state_wait (pid, process_state_zombie);
> +
> +  siginfo_t info;
> +  int r = waitid (P_PID, pid, &info, WEXITED);
> +  TEST_COMPARE (r, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_KILLED);
> +  TEST_COMPARE (info.si_status, SIGKILL);
> +  TEST_COMPARE (info.si_pid, pid);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] posix: Refactor tst-waitid

Adhemerval Zanella-2
In reply to this post by Adhemerval Zanella-2
Ping.

On 20/11/2019 16:42, Adhemerval Zanella wrote:

>
>
> On 20/11/2019 14:33, Joseph Myers wrote:
>> This reminds me of the bug we have,
>> <https://sourceware.org/bugzilla/show_bug.cgi?id=14666>, for intermittent
>> failures of tst-waitid (I think the discussion it references starts at
>> <https://sourceware.org/ml/libc-alpha/2010-09/msg00030.html>).
>>
>> Was that fixed by commit a58ad3f801960fa0dc0bb1106eb0d99f7ebd77b1 in 2013,
>> or is it fixed by your patch, or is it an unrelated issue?
>>
>
> My initial idea of replacing the sleep added by a58ad3f801960f is to
> loop atomically over a variable which is set by the signal handler.
> However I think a better strategy would be to use sigwaitinfo instead
> as Roland has suggested later in the thread.  It allows clean up the
> test a little more.
>
> And BZ#14666 does seems to be same issue mitigated by a58ad3f801960f.
>
> My idea of using support_process_state_wait in this case is to avoid
> use timeouts to sync process/threads and move towards some ideas laid
> out on a recent thread on how to speed up the testsuite spent time [1]
>
> Below I updated the patch to use sigwaitinfo instead of global variables
> set by the signal handler.
>
> [1] https://sourceware.org/ml/libc-alpha/2019-07/msg00403.html
>
> --
>
> [PATCH v2 2/3] posix: Refactor tst-waitid (BZ #14666)
>
> The main changes are:
>
>   - Adapt to libsupport.
>   - Synchronize the signal handler using atomics.
>   - Replace waitpid by waitid calls.
>   - Use support_process_state_wait to wait for child state.
>   - Add tests for P_PGID and P_ALL.
>   - Use sigwaitinfo instead of global state set by the signal handler.
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.  It fixes BZ #14666.
>
> ---
>
> diff --git a/posix/tst-waitid.c b/posix/tst-waitid.c
> index c82ec2b566..808bf6a8eb 100644
> --- a/posix/tst-waitid.c
> +++ b/posix/tst-waitid.c
> @@ -22,13 +22,20 @@
>  #include <unistd.h>
>  #include <sys/wait.h>
>  #include <signal.h>
> +#include <time.h>
> +#include <stdatomic.h>
> +#include <stdbool.h>
> +
> +#include <support/xsignal.h>
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +#include <support/process_state.h>
>  
>  static void
> -test_child (void)
> +test_child (bool setgroup)
>  {
> -  /* Wait a second to be sure the parent set his variables before we
> -     produce a SIGCHLD.  */
> -  sleep (1);
> +  if (setgroup)
> +    TEST_COMPARE (setpgid (0, 0), 0);
>  
>    /* First thing, we stop ourselves.  */
>    raise (SIGSTOP);
> @@ -44,475 +51,239 @@ test_child (void)
>  # define WSTOPPED WUNTRACED
>  #endif
>  
> -static sig_atomic_t expecting_sigchld, spurious_sigchld;
> -#ifdef SA_SIGINFO
> -static siginfo_t sigchld_info;
> +/* Set with only SIGCHLD on do_test_waitid.  */
> +static sigset_t chldset;
>  
> +#ifdef SA_SIGINFO
>  static void
>  sigchld (int signo, siginfo_t *info, void *ctx)
>  {
> -  if (signo != SIGCHLD)
> -    {
> -      printf ("SIGCHLD handler got signal %d instead!\n", signo);
> -      _exit (EXIT_FAILURE);
> -    }
> -
> -  if (! expecting_sigchld)
> -    {
> -      spurious_sigchld = 1;
> -      printf ("spurious SIGCHLD: signo %d code %d status %d pid %d\n",
> -      info->si_signo, info->si_code, info->si_status, info->si_pid);
> -    }
> -  else
> -    {
> -      sigchld_info = *info;
> -      expecting_sigchld = 0;
> -    }
>  }
> +#endif
>  
>  static void
> -check_sigchld (const char *phase, int *ok, int code, int status, pid_t pid)
> +check_sigchld (int code, int status, pid_t pid)
>  {
> -  if (expecting_sigchld)
> -    {
> -      printf ("missing SIGCHLD on %s\n", phase);
> -      *ok = EXIT_FAILURE;
> -      expecting_sigchld = 0;
> -      return;
> -    }
> +#ifdef SA_SIGINFO
> +  siginfo_t siginfo;
> +  TEST_COMPARE (sigwaitinfo (&chldset, &siginfo), SIGCHLD);
>  
> -  if (sigchld_info.si_signo != SIGCHLD)
> -    {
> -      printf ("SIGCHLD for %s signal %d\n", phase, sigchld_info.si_signo);
> -      *ok = EXIT_FAILURE;
> -    }
> -  if (sigchld_info.si_code != code)
> -    {
> -      printf ("SIGCHLD for %s code %d\n", phase, sigchld_info.si_code);
> -      *ok = EXIT_FAILURE;
> -    }
> -  if (sigchld_info.si_status != status)
> -    {
> -      printf ("SIGCHLD for %s status %d\n", phase, sigchld_info.si_status);
> -      *ok = EXIT_FAILURE;
> -    }
> -  if (sigchld_info.si_pid != pid)
> -    {
> -      printf ("SIGCHLD for %s pid %d\n", phase, sigchld_info.si_pid);
> -      *ok = EXIT_FAILURE;
> -    }
> -}
> -# define CHECK_SIGCHLD(phase, code_check, status_check) \
> -  check_sigchld ((phase), &status, (code_check), (status_check), pid)
> -#else
> -# define CHECK_SIGCHLD(phase, code, status) ((void) 0)
> +  TEST_COMPARE (siginfo.si_signo, SIGCHLD);
> +  TEST_COMPARE (siginfo.si_code, code);
> +  TEST_COMPARE (siginfo.si_status, status);
> +  TEST_COMPARE (siginfo.si_pid, pid);
>  #endif
> +}
>  
>  static int
> -do_test (int argc, char *argv[])
> +do_test_waitd_common (idtype_t type, pid_t pid)
>  {
> -#ifdef SA_SIGINFO
> -  struct sigaction sa;
> -  sa.sa_flags = SA_SIGINFO|SA_RESTART;
> -  sa.sa_sigaction = &sigchld;
> -  if (sigemptyset (&sa.sa_mask) < 0 || sigaction (SIGCHLD, &sa, NULL) < 0)
> -    {
> -      printf ("setting SIGCHLD handler: %m\n");
> -      return EXIT_FAILURE;
> -    }
> -#endif
> +  /* Adding process_state_tracing_stop ('t') allows the test to work under
> +     trace programs such as ptrace.  */
> +  enum process_state_t stop_state = process_state_stopped
> +    | process_state_tracing_stop;
>  
> -  expecting_sigchld = 1;
> +  support_process_state_wait (pid, stop_state);
>  
> -  pid_t pid = fork ();
> -  if (pid < 0)
> -    {
> -      printf ("fork: %m\n");
> -      return EXIT_FAILURE;
> -    }
> -  else if (pid == 0)
> -    {
> -      test_child ();
> -      _exit (127);
> -    }
> -
> -  int status = EXIT_SUCCESS;
> -#define RETURN(ok) \
> -    do { if (status == EXIT_SUCCESS) status = (ok); goto out; } while (0)
> -
> -  /* Give the child a chance to stop.  */
> -  sleep (3);
> -
> -  CHECK_SIGCHLD ("stopped (before waitid)", CLD_STOPPED, SIGSTOP);
> +  check_sigchld (CLD_STOPPED, SIGSTOP, pid);
>  
>    /* Now try a wait that should not succeed.  */
>    siginfo_t info;
> +  int fail;
> +
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
> -  int fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WNOHANG on stopped: %m\n");
> -      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo == 0)
> - break;
> -      if (info.si_signo == SIGCHLD)
> - printf ("waitid WNOHANG on stopped status %d\n", info.si_status);
> -      else
> - printf ("waitid WNOHANG on stopped signal %d\n", info.si_signo);
> -      RETURN (EXIT_FAILURE);
> -    }
> +  fail = waitid (P_PID, pid, &info, WEXITED|WCONTINUED|WNOHANG);
> +  if (fail == -1 && errno == ENOTSUP)
> +    FAIL_RET ("waitid WNOHANG on stopped: %m");
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, 0);
>  
>    /* Next the wait that should succeed right away.  */
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
>    info.si_pid = -1;
>    info.si_status = -1;
>    fail = waitid (P_PID, pid, &info, WSTOPPED|WNOHANG);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid WSTOPPED|WNOHANG returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WSTOPPED|WNOHANG on stopped: %m\n");
> -      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo != SIGCHLD)
> - {
> -  printf ("waitid WSTOPPED|WNOHANG on stopped signal %d\n",
> -  info.si_signo);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_code != CLD_STOPPED)
> - {
> -  printf ("waitid WSTOPPED|WNOHANG on stopped code %d\n",
> -  info.si_code);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_status != SIGSTOP)
> - {
> -  printf ("waitid WSTOPPED|WNOHANG on stopped status %d\n",
> -  info.si_status);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_pid != pid)
> - {
> -  printf ("waitid WSTOPPED|WNOHANG on stopped pid %d != %d\n",
> -  info.si_pid, pid);
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> +  if (fail == -1 && errno == ENOTSUP)
> +    FAIL_RET ("waitid WNOHANG on stopped: %m");
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_STOPPED);
> +  TEST_COMPARE (info.si_status, SIGSTOP);
> +  TEST_COMPARE (info.si_pid, pid);
>  
> -  expecting_sigchld = WCONTINUED != 0;
> +  //expecting_sigchld = WCONTINUED != 0;
>  
>    if (kill (pid, SIGCONT) != 0)
> -    {
> -      printf ("kill (%d, SIGCONT): %m\n", pid);
> -      RETURN (EXIT_FAILURE);
> -    }
> +    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
>  
>    /* Wait for the child to have continued.  */
> -  sleep (2);
> +  support_process_state_wait (pid, process_state_sleeping);
>  
>  #if WCONTINUED != 0
> -  if (expecting_sigchld)
> -    {
> -      printf ("no SIGCHLD seen for SIGCONT (optional)\n");
> -      expecting_sigchld = 0;
> -    }
> -  else
> -    CHECK_SIGCHLD ("continued (before waitid)", CLD_CONTINUED, SIGCONT);
> +  {
> +    sigset_t set;
> +    sigemptyset (&set);
> +    sigaddset (&set, SIGCHLD);
> +    siginfo_t info;
> +    TEST_COMPARE (sigwaitinfo (&set, &info), SIGCHLD);
> +
> +    TEST_COMPARE (info.si_signo, SIGCHLD);
> +    TEST_COMPARE (info.si_code, CLD_CONTINUED);
> +    TEST_COMPARE (info.si_status, SIGCONT);
> +    TEST_COMPARE (info.si_pid, pid);
> +  }
>  
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
>    info.si_pid = -1;
>    info.si_status = -1;
>    fail = waitid (P_PID, pid, &info, WCONTINUED|WNOWAIT);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid WCONTINUED|WNOWAIT returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WCONTINUED|WNOWAIT on continued: %m\n");
> -      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo != SIGCHLD)
> - {
> -  printf ("waitid WCONTINUED|WNOWAIT on continued signal %d\n",
> -  info.si_signo);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_code != CLD_CONTINUED)
> - {
> -  printf ("waitid WCONTINUED|WNOWAIT on continued code %d\n",
> -  info.si_code);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_status != SIGCONT)
> - {
> -  printf ("waitid WCONTINUED|WNOWAIT on continued status %d\n",
> -  info.si_status);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_pid != pid)
> - {
> -  printf ("waitid WCONTINUED|WNOWAIT on continued pid %d != %d\n",
> -  info.si_pid, pid);
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> +  if (fail == -1 && errno == ENOTSUP)
> +    FAIL_RET ("waitid WCONTINUED|WNOWAIT on continued: %m");
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_CONTINUED);
> +  TEST_COMPARE (info.si_status, SIGCONT);
> +  TEST_COMPARE (info.si_pid, pid);
>  
>    /* That should leave the CLD_CONTINUED state waiting to be seen again.  */
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
>    info.si_pid = -1;
>    info.si_status = -1;
>    fail = waitid (P_PID, pid, &info, WCONTINUED);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid WCONTINUED returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WCONTINUED on continued: %m\n");
> -      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo != SIGCHLD)
> - {
> -  printf ("waitid WCONTINUED on continued signal %d\n", info.si_signo);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_code != CLD_CONTINUED)
> - {
> -  printf ("waitid WCONTINUED on continued code %d\n", info.si_code);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_status != SIGCONT)
> - {
> -  printf ("waitid WCONTINUED on continued status %d\n",
> -  info.si_status);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_pid != pid)
> - {
> -  printf ("waitid WCONTINUED on continued pid %d != %d\n",
> -  info.si_pid, pid);
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> +  if (fail == -1 && errno == ENOTSUP)
> +    FAIL_RET ("waitid WCONTINUED on continued: %m");
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_CONTINUED);
> +  TEST_COMPARE (info.si_status, SIGCONT);
> +  TEST_COMPARE (info.si_pid, pid);
>  
>    /* Now try a wait that should not succeed.  */
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
>    fail = waitid (P_PID, pid, &info, WCONTINUED|WNOHANG);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WCONTINUED|WNOHANG on waited continued: %m\n");
> -      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo == 0)
> - break;
> -      if (info.si_signo == SIGCHLD)
> - printf ("waitid WCONTINUED|WNOHANG on waited continued status %d\n",
> - info.si_status);
> -      else
> - printf ("waitid WCONTINUED|WNOHANG on waited continued signal %d\n",
> - info.si_signo);
> -      RETURN (EXIT_FAILURE);
> -    }
> +  if (fail == -1 && errno == ENOTSUP)
> +    FAIL_RET ("waitid WCONTINUED|WNOHANG on waited continued: %m");
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, 0);
>  
>    /* Now stop him again and test waitpid with WCONTINUED.  */
> -  expecting_sigchld = 1;
> +  //expecting_sigchld = 1;
>    if (kill (pid, SIGSTOP) != 0)
> -    {
> -      printf ("kill (%d, SIGSTOP): %m\n", pid);
> -      RETURN (EXIT_FAILURE);
> -    }
> +    FAIL_RET ("kill (%d, SIGSTOP): %m\n", pid);
>  
> -  /* Give the child a chance to stop.  The waitpid call below will block
> -     until it has stopped, but if we are real quick and enter the waitpid
> -     system call before the SIGCHLD has been generated, then it will be
> -     discarded and never delivered.  */
> -  sleep (3);
> +  /* Wait the child stop.  The waitid call below will block until it has
> +     stopped, but if we are real quick and enter the waitid system call
> +     before the SIGCHLD has been generated, then it will be discarded and
> +     never delivered.  */
> +  support_process_state_wait (pid, stop_state);
>  
> -  pid_t wpid = waitpid (pid, &fail, WUNTRACED);
> -  if (wpid < 0)
> -    {
> -      printf ("waitpid WUNTRACED on stopped: %m\n");
> -      RETURN (EXIT_FAILURE);
> -    }
> -  else if (wpid != pid)
> -    {
> -      printf ("waitpid WUNTRACED on stopped returned %d != %d (status %x)\n",
> -      wpid, pid, fail);
> -      RETURN (EXIT_FAILURE);
> -    }
> -  else if (!WIFSTOPPED (fail) || WIFSIGNALED (fail) || WIFEXITED (fail)
> -   || WIFCONTINUED (fail) || WSTOPSIG (fail) != SIGSTOP)
> -    {
> -      printf ("waitpid WUNTRACED on stopped: status %x\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    }
> -  CHECK_SIGCHLD ("stopped (after waitpid)", CLD_STOPPED, SIGSTOP);
> +  fail = waitid (type, pid, &info, WEXITED|WSTOPPED);
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_STOPPED);
> +  TEST_COMPARE (info.si_status, SIGSTOP);
> +  TEST_COMPARE (info.si_pid, pid);
> +
> +  check_sigchld (CLD_STOPPED, SIGSTOP, pid);
>  
> -  expecting_sigchld = 1;
>    if (kill (pid, SIGCONT) != 0)
> -    {
> -      printf ("kill (%d, SIGCONT): %m\n", pid);
> -      RETURN (EXIT_FAILURE);
> -    }
> +    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
>  
>    /* Wait for the child to have continued.  */
> -  sleep (2);
> +  support_process_state_wait (pid, process_state_sleeping);
>  
> -  if (expecting_sigchld)
> -    {
> -      printf ("no SIGCHLD seen for SIGCONT (optional)\n");
> -      expecting_sigchld = 0;
> -    }
> -  else
> -    CHECK_SIGCHLD ("continued (before waitpid)", CLD_CONTINUED, SIGCONT);
> +  check_sigchld (CLD_CONTINUED, SIGCONT, pid);
>  
> -  wpid = waitpid (pid, &fail, WCONTINUED);
> -  if (wpid < 0)
> -    {
> -      if (errno == EINVAL)
> - printf ("waitpid does not support WCONTINUED\n");
> -      else
> - {
> -  printf ("waitpid WCONTINUED on continued: %m\n");
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> -  else if (wpid != pid)
> -    {
> -      printf ("\
> -waitpid WCONTINUED on continued returned %d != %d (status %x)\n",
> -     wpid, pid, fail);
> -      RETURN (EXIT_FAILURE);
> -    }
> -  else if (WIFSTOPPED (fail) || WIFSIGNALED (fail) || WIFEXITED (fail)
> -   || !WIFCONTINUED (fail))
> -    {
> -      printf ("waitpid WCONTINUED on continued: status %x\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    }
> +  fail = waitid (type, pid, &info, WCONTINUED);
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_CONTINUED);
> +  TEST_COMPARE (info.si_status, SIGCONT);
> +  TEST_COMPARE (info.si_pid, pid);
>  #endif
>  
> -  expecting_sigchld = 1;
> -
>    /* Die, child, die!  */
>    if (kill (pid, SIGKILL) != 0)
> -    {
> -      printf ("kill (%d, SIGKILL): %m\n", pid);
> -      RETURN (EXIT_FAILURE);
> -    }
> +    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
>  
>  #ifdef WNOWAIT
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
>    info.si_pid = -1;
>    info.si_status = -1;
> -  fail = waitid (P_PID, pid, &info, WEXITED|WNOWAIT);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid WNOWAIT returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WNOWAIT on killed: %m\n");
> -      RETURN (errno == ENOTSUP ? EXIT_SUCCESS : EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo != SIGCHLD)
> - {
> -  printf ("waitid WNOWAIT on killed signal %d\n", info.si_signo);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_code != CLD_KILLED)
> - {
> -  printf ("waitid WNOWAIT on killed code %d\n", info.si_code);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_status != SIGKILL)
> - {
> -  printf ("waitid WNOWAIT on killed status %d\n", info.si_status);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_pid != pid)
> - {
> -  printf ("waitid WNOWAIT on killed pid %d != %d\n", info.si_pid, pid);
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> +  fail = waitid (type, pid, &info, WEXITED|WNOWAIT);
> +  if (fail == -1 && errno == ENOTSUP)
> +    FAIL_RET ("waitid WNOHANG on killed: %m");
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_KILLED);
> +  TEST_COMPARE (info.si_status, SIGKILL);
> +  TEST_COMPARE (info.si_pid, pid);
>  #else
> -  /* Allow enough time to be sure the child died; we didn't synchronize.  */
> -  sleep (2);
> +  support_process_state_wait (pid, process_state_zombie);
>  #endif
> -
> -  CHECK_SIGCHLD ("killed", CLD_KILLED, SIGKILL);
> +  check_sigchld (CLD_KILLED, SIGKILL, pid);
>  
>    info.si_signo = 0; /* A successful call sets it to SIGCHLD.  */
>    info.si_pid = -1;
>    info.si_status = -1;
> -  fail = waitid (P_PID, pid, &info, WEXITED|WNOHANG);
> -  switch (fail)
> -    {
> -    default:
> -      printf ("waitid WNOHANG returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> -    case -1:
> -      printf ("waitid WNOHANG on killed: %m\n");
> -      RETURN (EXIT_FAILURE);
> -    case 0:
> -      if (info.si_signo != SIGCHLD)
> - {
> -  printf ("waitid WNOHANG on killed signal %d\n", info.si_signo);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_code != CLD_KILLED)
> - {
> -  printf ("waitid WNOHANG on killed code %d\n", info.si_code);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_status != SIGKILL)
> - {
> -  printf ("waitid WNOHANG on killed status %d\n", info.si_status);
> -  RETURN (EXIT_FAILURE);
> - }
> -      if (info.si_pid != pid)
> - {
> -  printf ("waitid WNOHANG on killed pid %d != %d\n", info.si_pid, pid);
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> +  fail = waitid (type, pid, &info, WEXITED | WNOHANG);
> +  TEST_COMPARE (fail, 0);
> +  TEST_COMPARE (info.si_signo, SIGCHLD);
> +  TEST_COMPARE (info.si_code, CLD_KILLED);
> +  TEST_COMPARE (info.si_status, SIGKILL);
> +  TEST_COMPARE (info.si_pid, pid);
>  
>    fail = waitid (P_PID, pid, &info, WEXITED);
> -  if (fail == -1)
> -    {
> -      if (errno != ECHILD)
> - {
> -  printf ("waitid WEXITED on killed: %m\n");
> -  RETURN (EXIT_FAILURE);
> - }
> -    }
> -  else
> +  TEST_COMPARE (fail, -1);
> +  TEST_COMPARE (errno, ECHILD);
> +
> +  return 0;
> +}
> +
> +static int
> +do_test_waitid (idtype_t type)
> +{
> +#ifdef SA_SIGINFO
> +  {
> +    struct sigaction sa;
> +    sa.sa_flags = SA_SIGINFO | SA_RESTART;
> +    sa.sa_sigaction = sigchld;
> +    sigemptyset (&sa.sa_mask);
> +    xsigaction (SIGCHLD, &sa, NULL);
> +  }
> +#endif
> +
> +  sigemptyset (&chldset);
> +  sigaddset (&chldset, SIGCHLD);
> +
> +  /* The SIGCHLD shall has blocked at the time of the call to sigwait;
> +     otherwise, the behavior is undefined.  */
> +  sigprocmask (SIG_BLOCK, &chldset, NULL);
> +
> +  pid_t pid = xfork ();
> +  if (pid == 0)
>      {
> -      printf ("waitid WEXITED returned bogus value %d\n", fail);
> -      RETURN (EXIT_FAILURE);
> +      test_child (type == P_PGID || type == P_ALL);
> +      _exit (127);
>      }
>  
> -#undef RETURN
> - out:
> -  if (spurious_sigchld)
> -    status = EXIT_FAILURE;
> -  signal (SIGCHLD, SIG_IGN);
> +  int ret = do_test_waitd_common (type, pid);
> +
> +  xsignal (SIGCHLD, SIG_IGN);
>    kill (pid, SIGKILL); /* Make sure it's dead if we bailed early.  */
> -  return status;
> +  return ret;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  int ret = 0;
> +
> +  ret |= do_test_waitid (P_PID);
> +  ret |= do_test_waitid (P_PGID);
> +  ret |= do_test_waitid (P_ALL);
> +
> +  return ret;
>  }
>  
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> --
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] posix: Remove posix waitid

Adhemerval Zanella-2
In reply to this post by Adhemerval Zanella-2
Ping.

On 20/11/2019 11:15, Adhemerval Zanella wrote:

> The POSIX waitid implementation is problematic in some ways:
>
>   - It emulates using waitpid, which default implementation calls
>     wait4 and wait4 returns ENOSYS as default.
>
>   - Also by using waitpid it does not allod support the WNOWAIT,
>     WEXITED, WSTOPPED, or WCONTINUED flag.  With current POSIX
>     specification the flags are no longer marked as optional.
>
> Also due BZ#23091 Hurd still uses the implementation, so it is moved
> to as a Hurd arch-specific folder (with some minor cleanups).
>
> Checked against a i686-gnu (run-built-tests=no)
> ---
>  sysdeps/{posix => mach/hurd}/waitid.c | 70 ++++-----------------------
>  1 file changed, 9 insertions(+), 61 deletions(-)
>  rename sysdeps/{posix => mach/hurd}/waitid.c (65%)
>
> diff --git a/sysdeps/posix/waitid.c b/sysdeps/mach/hurd/waitid.c
> similarity index 65%
> rename from sysdeps/posix/waitid.c
> rename to sysdeps/mach/hurd/waitid.c
> index 0942623553..d28d0034da 100644
> --- a/sysdeps/posix/waitid.c
> +++ b/sysdeps/mach/hurd/waitid.c
> @@ -17,25 +17,11 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#include <assert.h>
>  #include <errno.h>
> -#include <signal.h>
> -#define __need_NULL
> -#include <stddef.h>
>  #include <sys/wait.h>
> -#include <sys/types.h>
> -#include <sysdep-cancel.h>
>  
> -
> -#ifdef DO_WAITID
> -# define OUR_WAITID DO_WAITID
> -#elif !defined NO_DO_WAITID
> -# define OUR_WAITID do_waitid
> -#endif
> -
> -#ifdef OUR_WAITID
> -static int
> -OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
> +int
> +__waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options)
>  {
>    pid_t pid, child;
>    int status;
> @@ -43,7 +29,7 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
>    switch (idtype)
>      {
>      case P_PID:
> -      if(id <= 0)
> +      if (id <= 0)
>   goto invalid;
>        pid = (pid_t) id;
>        break;
> @@ -72,31 +58,10 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
>        return -1;
>      }
>  
> -  /* This emulation using waitpid cannot support the waitid modes in which
> -     we do not reap the child, or match only stopped and not dead children.  */
> -  if (0
> -#ifdef WNOWAIT
> -      || (options & WNOWAIT)
> -#endif
> -#ifdef WEXITED
> -      || ((options & (WEXITED|WSTOPPED|WCONTINUED))
> -  != (WEXITED | (options & WSTOPPED)))
> -#endif
> -      )
> -    {
> -      __set_errno (ENOTSUP);
> -      return -1;
> -    }
> -
>    /* Note the waitid() is a cancellation point.  But since we call
>       waitpid() which itself is a cancellation point we do not have
>       to do anything here.  */
> -  child = __waitpid (pid, &status,
> -     options
> -#ifdef WEXITED
> -     &~ WEXITED
> -#endif
> -     );
> +  child = __waitpid (pid, &status, options);
>  
>    if (child == -1)
>      /* `waitpid' set `errno' for us.  */
> @@ -104,9 +69,11 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
>  
>    if (child == 0)
>      {
> -      /* The WHOHANG bit in OPTIONS is set and there are children available
> - but none has a status for us.  The XPG docs do not mention this
> - case so we clear the `siginfo_t' struct and return successfully.  */
> +      /* POSIX.1-2008, Technical Corrigendum 1 XSH/TC1-2008/0713 [153] states
> + that if waitid returns because WNOHANG was specified and status is
> + not available for any process specified by idtype and id, then the
> + si_signo and si_pid members of the structure pointed to by infop
> + shall be set to zero.  */
>        infop->si_signo = 0;
>        infop->si_code = 0;
>        return 0;
> @@ -132,27 +99,8 @@ OUR_WAITID (idtype_t idtype, id_t id, siginfo_t *infop, int options)
>        infop->si_code = CLD_STOPPED;
>        infop->si_status = WSTOPSIG (status);
>      }
> -#ifdef WIFCONTINUED
> -  else if (WIFCONTINUED (status))
> -    {
> -      infop->si_code = CLD_CONTINUED;
> -      infop->si_status = SIGCONT;
> -    }
> -#endif
> -  else
> -    /* Can't happen. */
> -    assert (! "What?");
>  
>    return 0;
>  }
> -#endif
> -
> -
> -int
> -__waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options)
> -{
> -  /* __waitpid should be a cancellation point.  */
> -  return do_waitid (idtype, id, infop, options);
> -}
>  weak_alias (__waitid, waitid)
>  strong_alias (__waitid, __libc_waitid)
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] support: Add support_process_state_wait

Florian Weimer-5
In reply to this post by Adhemerval Zanella-2
* Adhemerval Zanella:

> It allows parent process to wait for children state using a polling
> strategy over procfs on Linux.  The polling is used over ptrace to
> avoid the need to handle signals on the target pid and to handle some
> system specific limitations (such as YAMA).
>
> The polling has some limitations, such as resource consumption due
> the procfs read in a loop and the lack of synchronization after the
> state is obtained.
>
> The interface idea is to simplify some sleep synchronization in waitid
> tests.

The goal is to reduce timeouts, I assume, replacing arbitrary two-second
waits.

> diff --git a/support/process_state.h b/support/process_state.h
> new file mode 100644
> index 0000000000..d616f0e3e6
> --- /dev/null
> +++ b/support/process_state.h
> @@ -0,0 +1,41 @@

> +enum process_state_t
> +{
> +  process_state_running      = 0x01,  /* R (running)  */
> +  process_state_sleeping     = 0x02,  /* S (sleeping)  */
> +  process_state_disk_sleep   = 0x04,  /* D (disk sleep)  */
> +  process_state_stopped      = 0x08,  /* T (stopped)  */
> +  process_state_tracing_stop = 0x10,  /* t (tracing stop)  */
> +  process_state_dead         = 0x20,  /* X (dead)  */
> +  process_state_zombie       = 0x40,  /* Z (zombie)  */
> +  process_state_parked       = 0x80,  /* P (parked)  */
> +};

Maybe drop the reversed _t suffix and add a support_ prefix?

> +
> +/* Wait for process PID to reach state STATE.  It can be a combination of
> +   multiple possible states ('process_state_running | process_state_sleeping')
> +   where the function return when any of these state are observed.  */
> +void support_process_state_wait (pid_t pid, enum process_state_t state);

This interface does not allow to poll for a process not to have a
specific state.  I guess we can adjust it once the need arises, so you
can leave it for now.

> --- /dev/null
> +++ b/support/support_process_state.c
> @@ -0,0 +1,89 @@

> +void
> +support_process_state_wait (pid_t pid, enum process_state_t state)
> +{
> +#ifdef __linux__
> +  /* For Linux it does a polling check on /proc/<pid>/status checking on
> +     third field.  */
> +
> +  /* It mimics the kernel states from fs/proc/array.c  */
> +  static struct process_states_t
> +  {
> +    enum process_state_t s;
> +    char v;
> +  } process_states[] = {
> +    { process_state_running,      'R' },
> +    { process_state_sleeping,     'S' },
> +    { process_state_disk_sleep,   'D' },
> +    { process_state_stopped,      'T' },
> +    { process_state_tracing_stop, 't' },
> +    { process_state_dead,         'X' },
> +    { process_state_zombie,       'Z' },
> +    { process_state_parked,       'P' },
> +  };

Should be const.

> +  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
> + + 1];
> +  snprintf (proc_path, sizeof (proc_path), "/proc/%i/stat", pid);
> +
> +  int fd = xopen (proc_path, O_RDONLY, 0600);
> +
> +  char statbuf[3 * sizeof (pid_t)  /* pid.  */
> +               + 2                 /* ' (' */
> +               + 64                /* task name.  */
> +               + 2                 /* ' )'  */
> +               + 1                 /* <state>, 1 char.  */
> +               + 1];               /* Final \0.  */
> +  for (;;)
> +    {
> +      ssize_t ret = read (fd, statbuf, sizeof (statbuf) - 1);
> +      TEST_VERIFY (ret >= 0);
> +      statbuf[ret] = '\0';
> +
> +      char cur_state;
> +      int sret = sscanf (statbuf, "%*i %*s %c", &cur_state);
> +      TEST_VERIFY (sret == 1);

Hmm.  Does this work if COMM includes a space?  I think it's safer to
parse /proc/PID/status.

> +      for (size_t i = 0; i < array_length (process_states); ++i)
> + if (state & process_states[i].s && cur_state == process_states[i].v)
> +  {
> +    close (fd);
> +    return;
> +  }
> +
> +      xlseek (fd, 0, SEEK_SET);
> +      nanosleep (&(struct timespec) { 0, 10000000 }, NULL);
> +    }
> +
> +  close (fd);

Should this have a fallback timeout?

The sleep should probably be usleep or nanosleep, to avoid interfering
with laram.

The test looks reasonable.

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] support: Add support_process_state_wait

Adhemerval Zanella-2


On 20/12/2019 11:09, Florian Weimer wrote:

> * Adhemerval Zanella:
>
>> It allows parent process to wait for children state using a polling
>> strategy over procfs on Linux.  The polling is used over ptrace to
>> avoid the need to handle signals on the target pid and to handle some
>> system specific limitations (such as YAMA).
>>
>> The polling has some limitations, such as resource consumption due
>> the procfs read in a loop and the lack of synchronization after the
>> state is obtained.
>>
>> The interface idea is to simplify some sleep synchronization in waitid
>> tests.
>
> The goal is to reduce timeouts, I assume, replacing arbitrary two-second
> waits.

Indeed, I updated the commit message.

>
>> diff --git a/support/process_state.h b/support/process_state.h
>> new file mode 100644
>> index 0000000000..d616f0e3e6
>> --- /dev/null
>> +++ b/support/process_state.h
>> @@ -0,0 +1,41 @@
>
>> +enum process_state_t
>> +{
>> +  process_state_running      = 0x01,  /* R (running)  */
>> +  process_state_sleeping     = 0x02,  /* S (sleeping)  */
>> +  process_state_disk_sleep   = 0x04,  /* D (disk sleep)  */
>> +  process_state_stopped      = 0x08,  /* T (stopped)  */
>> +  process_state_tracing_stop = 0x10,  /* t (tracing stop)  */
>> +  process_state_dead         = 0x20,  /* X (dead)  */
>> +  process_state_zombie       = 0x40,  /* Z (zombie)  */
>> +  process_state_parked       = 0x80,  /* P (parked)  */
>> +};
>
> Maybe drop the reversed _t suffix and add a support_ prefix?

Ack.

>
>> +
>> +/* Wait for process PID to reach state STATE.  It can be a combination of
>> +   multiple possible states ('process_state_running | process_state_sleeping')
>> +   where the function return when any of these state are observed.  */
>> +void support_process_state_wait (pid_t pid, enum process_state_t state);
>
> This interface does not allow to poll for a process not to have a
> specific state.  I guess we can adjust it once the need arises, so you
> can leave it for now.

Ok.

>
>> --- /dev/null
>> +++ b/support/support_process_state.c
>> @@ -0,0 +1,89 @@
>
>> +void
>> +support_process_state_wait (pid_t pid, enum process_state_t state)
>> +{
>> +#ifdef __linux__
>> +  /* For Linux it does a polling check on /proc/<pid>/status checking on
>> +     third field.  */
>> +
>> +  /* It mimics the kernel states from fs/proc/array.c  */
>> +  static struct process_states_t
>> +  {
>> +    enum process_state_t s;
>> +    char v;
>> +  } process_states[] = {
>> +    { process_state_running,      'R' },
>> +    { process_state_sleeping,     'S' },
>> +    { process_state_disk_sleep,   'D' },
>> +    { process_state_stopped,      'T' },
>> +    { process_state_tracing_stop, 't' },
>> +    { process_state_dead,         'X' },
>> +    { process_state_zombie,       'Z' },
>> +    { process_state_parked,       'P' },
>> +  };
>
> Should be const.

Ack.

>
>> +  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
>> + + 1];
>> +  snprintf (proc_path, sizeof (proc_path), "/proc/%i/stat", pid);
>> +
>> +  int fd = xopen (proc_path, O_RDONLY, 0600);
>> +
>> +  char statbuf[3 * sizeof (pid_t)  /* pid.  */
>> +               + 2                 /* ' (' */
>> +               + 64                /* task name.  */
>> +               + 2                 /* ' )'  */
>> +               + 1                 /* <state>, 1 char.  */
>> +               + 1];               /* Final \0.  */
>> +  for (;;)
>> +    {
>> +      ssize_t ret = read (fd, statbuf, sizeof (statbuf) - 1);
>> +      TEST_VERIFY (ret >= 0);
>> +      statbuf[ret] = '\0';
>> +
>> +      char cur_state;
>> +      int sret = sscanf (statbuf, "%*i %*s %c", &cur_state);
>> +      TEST_VERIFY (sret == 1);
>
> Hmm.  Does this work if COMM includes a space?  I think it's safer to
> parse /proc/PID/status.

It does not, I updated to parse /proc/PID/status.

>
>> +      for (size_t i = 0; i < array_length (process_states); ++i)
>> + if (state & process_states[i].s && cur_state == process_states[i].v)
>> +  {
>> +    close (fd);
>> +    return;
>> +  }
>> +
>> +      xlseek (fd, 0, SEEK_SET);
>> +      nanosleep (&(struct timespec) { 0, 10000000 }, NULL);
>> +    }
>> +
>> +  close (fd);
>
> Should this have a fallback timeout?

Do you mean if it can not get the process state? Currently
support_process_state_wait fails either if it can not open the status
file or if it can't find 'State:' field. Should we add a fallback for
each case?

>
> The sleep should probably be usleep or nanosleep, to avoid interfering
> with laram.

Ok.

>
> The test looks reasonable.


Updated patch below:

--

diff --git a/support/Makefile b/support/Makefile
index 23c6d74627..b0d28200af 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -56,6 +56,7 @@ libsupport-routines = \
   support_format_hostent \
   support_format_netent \
   support_isolate_in_subprocess \
+  support_process_state \
   support_ptrace \
   support_openpty \
   support_paths \
@@ -223,6 +224,7 @@ tests = \
   tst-support_capture_subprocess \
   tst-support_descriptors \
   tst-support_format_dns_packet \
+  tst-support-process_state \
   tst-support_quote_blob \
   tst-support_quote_string \
   tst-support_record_failure \
diff --git a/support/process_state.h b/support/process_state.h
new file mode 100644
index 0000000000..d983197247
--- /dev/null
+++ b/support/process_state.h
@@ -0,0 +1,45 @@
+/* Wait for process state.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef SUPPORT_PROCESS_STATE_H
+#define SUPPORT_PROCESS_STATE_H
+
+#include <sys/types.h>
+
+enum support_process_state
+{
+  support_process_state_running      = 0x01,  /* R (running).  */
+  support_process_state_sleeping     = 0x02,  /* S (sleeping).  */
+  support_process_state_disk_sleep   = 0x04,  /* D (disk sleep).  */
+  support_process_state_stopped      = 0x08,  /* T (stopped).  */
+  support_process_state_tracing_stop = 0x10,  /* t (tracing stop).  */
+  support_process_state_dead         = 0x20,  /* X (dead).  */
+  support_process_state_zombie       = 0x40,  /* Z (zombie).  */
+  support_process_state_parked       = 0x80,  /* P (parked).  */
+};
+
+/* Wait for process PID to reach state STATE.  It can be a combination of
+   multiple possible states ('process_state_running | process_state_sleeping')
+   where the function return when any of these state are observed.
+   The timeout POLL_TS might be used to wait before polling the PID status
+   information from the kernel.  A NULL value will set to use a default
+   value (10000000 ns).  */
+void support_process_state_wait (pid_t pid, enum support_process_state state,
+ struct timespec *poll_ts);
+
+#endif
diff --git a/support/support_process_state.c b/support/support_process_state.c
new file mode 100644
index 0000000000..16c2c3642e
--- /dev/null
+++ b/support/support_process_state.c
@@ -0,0 +1,102 @@
+/* Wait for process state.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <time.h>
+#include <string.h>
+
+#include <array_length.h>
+
+#include <support/process_state.h>
+#include <support/xunistd.h>
+#include <support/check.h>
+
+/* Returns the character representing the task state from FD.  */
+static char find_state (int fd)
+{
+  char statusbuf[strlen ("Name:\t")
+ + 64                   /* task name.  */
+ + strlen ("Umask:\t")  /* optional.  */
+ + 4                    /* umask size.  */
+ + strlen ("State:\t")
+ + 1                    /* <state>, 1 char.  */
+ + 1];                  /* Final \0.  */
+
+  ssize_t ret = read (fd, statusbuf, sizeof (statusbuf) - 1);
+  TEST_VERIFY (ret >= 0);
+  statusbuf[ret] = '\0';
+
+  char *statestr = strstr (statusbuf, "State:\t");
+  TEST_VERIFY (statestr != NULL);
+  char state;
+  int sret = sscanf (statestr, "%*s %c", &state);
+  TEST_VERIFY (sret == 1);
+
+  return state;
+}
+
+void
+support_process_state_wait (pid_t pid, enum support_process_state state,
+    struct timespec *poll_ts)
+{
+#ifdef __linux__
+  /* For Linux it does a polling check on /proc/<pid>/status checking on
+     third field.  */
+
+  /* It mimics the kernel states from fs/proc/array.c  */
+  static const struct process_states_t
+  {
+    enum support_process_state s;
+    char v;
+  } process_states[] = {
+    { support_process_state_running,      'R' },
+    { support_process_state_sleeping,     'S' },
+    { support_process_state_disk_sleep,   'D' },
+    { support_process_state_stopped,      'T' },
+    { support_process_state_tracing_stop, 't' },
+    { support_process_state_dead,         'X' },
+    { support_process_state_zombie,       'Z' },
+    { support_process_state_parked,       'P' },
+  };
+
+  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
+ + 1];
+  snprintf (proc_path, sizeof (proc_path), "/proc/%i/status", pid);
+
+  int fd = xopen (proc_path, O_RDONLY, 0600);
+
+  for (;;)
+    {
+      char cur_state = find_state (fd);
+
+      for (size_t i = 0; i < array_length (process_states); ++i)
+ if (state & process_states[i].s && cur_state == process_states[i].v)
+  goto found;
+
+      xlseek (fd, 0, SEEK_SET);
+
+      nanosleep (poll_ts ? poll_ts : &(struct timespec) { 0, 10000000 }, NULL);
+    }
+
+found:
+  xclose (fd);
+#else
+  nanosleep (&(struct timespec) { 2, 0 }, NULL);
+#endif
+}
diff --git a/support/tst-support-process_state.c b/support/tst-support-process_state.c
new file mode 100644
index 0000000000..d0aa142c76
--- /dev/null
+++ b/support/tst-support-process_state.c
@@ -0,0 +1,101 @@
+/* Wait for process state tests.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <support/test-driver.h>
+#include <support/process_state.h>
+#include <support/check.h>
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+
+static void
+sigusr1_handler (int signo)
+{
+}
+
+static void
+test_child (void)
+{
+  xsignal (SIGUSR1, sigusr1_handler);
+
+  raise (SIGSTOP);
+
+  TEST_COMPARE (pause (), -1);
+  TEST_COMPARE (errno, EINTR);
+
+  while (1)
+    asm ("");
+}
+
+static int
+do_test (void)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    {
+      test_child ();
+      _exit (127);
+    }
+
+  /* Adding process_state_tracing_stop ('t') allows the test to work under
+     trace programs such as ptrace.  */
+  enum support_process_state stop_state = support_process_state_stopped
+    | support_process_state_tracing_stop;
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_stopped/state_tracing_stop\n",
+    (int) pid);
+  support_process_state_wait (pid, stop_state, NULL);
+
+  if (kill (pid, SIGCONT) != 0)
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_sleeping\n", (int) pid);
+  support_process_state_wait (pid, support_process_state_sleeping, NULL);
+
+  if (kill (pid, SIGUSR1) != 0)
+    FAIL_RET ("kill (%d, SIGUSR1): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_running\n", (int) pid);
+  support_process_state_wait (pid, support_process_state_running, NULL);
+
+  if (kill (pid, SIGKILL) != 0)
+    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_zombie\n", (int) pid);
+  support_process_state_wait (pid, support_process_state_zombie, NULL);
+
+  siginfo_t info;
+  int r = waitid (P_PID, pid, &info, WEXITED);
+  TEST_COMPARE (r, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] support: Add support_process_state_wait

Florian Weimer
* Adhemerval Zanella:

> diff --git a/support/process_state.h b/support/process_state.h
> new file mode 100644
> index 0000000000..d983197247
> --- /dev/null
> +++ b/support/process_state.h
> @@ -0,0 +1,45 @@

> +#ifndef SUPPORT_PROCESS_STATE_H
> +#define SUPPORT_PROCESS_STATE_H
> +
> +#include <sys/types.h>

I think you also need to include <time.h> for struct timespec,
strictly speaking.

> diff --git a/support/support_process_state.c b/support/support_process_state.c
> new file mode 100644
> index 0000000000..16c2c3642e
> --- /dev/null
> +++ b/support/support_process_state.c

> +/* Returns the character representing the task state from FD.  */
> +static char find_state (int fd)
> +{
> +  char statusbuf[strlen ("Name:\t")
> + + 64                   /* task name.  */
> + + strlen ("Umask:\t")  /* optional.  */
> + + 4                    /* umask size.  */
> + + strlen ("State:\t")
> + + 1                    /* <state>, 1 char.  */
> + + 1];                  /* Final \0.  */
> +
> +  ssize_t ret = read (fd, statusbuf, sizeof (statusbuf) - 1);
> +  TEST_VERIFY (ret >= 0);
> +  statusbuf[ret] = '\0';
> +
> +  char *statestr = strstr (statusbuf, "State:\t");
> +  TEST_VERIFY (statestr != NULL);
> +  char state;
> +  int sret = sscanf (statestr, "%*s %c", &state);
> +  TEST_VERIFY (sret == 1);
> +
> +  return state;
> +}

I think the expectation here is that more things will be inserted
after Name:.  As you note, Umask: is already a late arrival.

So if simplicity is a concern, I would just read 4 KiB or so, and then
use strstr with "\nState:\t" (note the \n).  The sscanf seems
superfluous in this context.  Every else heads in the direction of
getline …

I think it also makes sense to return 0 in case the state cannot be
determined, rather than an indeterminate value.

> +void
> +support_process_state_wait (pid_t pid, enum support_process_state state,
> +    struct timespec *poll_ts)
> +{
> +#ifdef __linux__
> +  /* For Linux it does a polling check on /proc/<pid>/status checking on
> +     third field.  */
> +
> +  /* It mimics the kernel states from fs/proc/array.c  */
> +  static const struct process_states_t
> +  {
> +    enum support_process_state s;
> +    char v;
> +  } process_states[] = {
> +    { support_process_state_running,      'R' },
> +    { support_process_state_sleeping,     'S' },
> +    { support_process_state_disk_sleep,   'D' },
> +    { support_process_state_stopped,      'T' },
> +    { support_process_state_tracing_stop, 't' },
> +    { support_process_state_dead,         'X' },
> +    { support_process_state_zombie,       'Z' },
> +    { support_process_state_parked,       'P' },
> +  };
> +
> +  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
> + + 1];
> +  snprintf (proc_path, sizeof (proc_path), "/proc/%i/status", pid);
> +
> +  int fd = xopen (proc_path, O_RDONLY, 0600);
> +
> +  for (;;)
> +    {
> +      char cur_state = find_state (fd);
> +
> +      for (size_t i = 0; i < array_length (process_states); ++i)
> + if (state & process_states[i].s && cur_state == process_states[i].v)
> +  goto found;

I missed that before, I think we should error out if the state is
completely unknown.  Maybe use strchr on "RSDTtXZP" instead, and use
that for the shift? 8-)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] support: Add support_process_state_wait

Adhemerval Zanella-2


On 20/12/2019 17:32, Florian Weimer wrote:

> * Adhemerval Zanella:
>
>> diff --git a/support/process_state.h b/support/process_state.h
>> new file mode 100644
>> index 0000000000..d983197247
>> --- /dev/null
>> +++ b/support/process_state.h
>> @@ -0,0 +1,45 @@
>
>> +#ifndef SUPPORT_PROCESS_STATE_H
>> +#define SUPPORT_PROCESS_STATE_H
>> +
>> +#include <sys/types.h>
>
> I think you also need to include <time.h> for struct timespec,
> strictly speaking.

Ack.

>
>> diff --git a/support/support_process_state.c b/support/support_process_state.c
>> new file mode 100644
>> index 0000000000..16c2c3642e
>> --- /dev/null
>> +++ b/support/support_process_state.c
>
>> +/* Returns the character representing the task state from FD.  */
>> +static char find_state (int fd)
>> +{
>> +  char statusbuf[strlen ("Name:\t")
>> + + 64                   /* task name.  */
>> + + strlen ("Umask:\t")  /* optional.  */
>> + + 4                    /* umask size.  */
>> + + strlen ("State:\t")
>> + + 1                    /* <state>, 1 char.  */
>> + + 1];                  /* Final \0.  */
>> +
>> +  ssize_t ret = read (fd, statusbuf, sizeof (statusbuf) - 1);
>> +  TEST_VERIFY (ret >= 0);
>> +  statusbuf[ret] = '\0';
>> +
>> +  char *statestr = strstr (statusbuf, "State:\t");
>> +  TEST_VERIFY (statestr != NULL);
>> +  char state;
>> +  int sret = sscanf (statestr, "%*s %c", &state);
>> +  TEST_VERIFY (sret == 1);
>> +
>> +  return state;
>> +}
>
> I think the expectation here is that more things will be inserted
> after Name:.  As you note, Umask: is already a late arrival.
>
> So if simplicity is a concern, I would just read 4 KiB or so, and then
> use strstr with "\nState:\t" (note the \n).  The sscanf seems
> superfluous in this context.  Every else heads in the direction of
> getline …
>
> I think it also makes sense to return 0 in case the state cannot be
> determined, rather than an indeterminate value.

I was trying more to make support_process_state_wait not consume much
resources, but it seems since /proc/pid/status does not have a
stable format we will need to parse it in the end.  I have adjusted
to use getline.

>
>> +void
>> +support_process_state_wait (pid_t pid, enum support_process_state state,
>> +    struct timespec *poll_ts)
>> +{
>> +#ifdef __linux__
>> +  /* For Linux it does a polling check on /proc/<pid>/status checking on
>> +     third field.  */
>> +
>> +  /* It mimics the kernel states from fs/proc/array.c  */
>> +  static const struct process_states_t
>> +  {
>> +    enum support_process_state s;
>> +    char v;
>> +  } process_states[] = {
>> +    { support_process_state_running,      'R' },
>> +    { support_process_state_sleeping,     'S' },
>> +    { support_process_state_disk_sleep,   'D' },
>> +    { support_process_state_stopped,      'T' },
>> +    { support_process_state_tracing_stop, 't' },
>> +    { support_process_state_dead,         'X' },
>> +    { support_process_state_zombie,       'Z' },
>> +    { support_process_state_parked,       'P' },
>> +  };
>> +
>> +  char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
>> + + 1];
>> +  snprintf (proc_path, sizeof (proc_path), "/proc/%i/status", pid);
>> +
>> +  int fd = xopen (proc_path, O_RDONLY, 0600);
>> +
>> +  for (;;)
>> +    {
>> +      char cur_state = find_state (fd);
>> +
>> +      for (size_t i = 0; i < array_length (process_states); ++i)
>> + if (state & process_states[i].s && cur_state == process_states[i].v)
>> +  goto found;
>
> I missed that before, I think we should error out if the state is
> completely unknown.  Maybe use strchr on "RSDTtXZP" instead, and use
> that for the shift? 8-)
>

What I did instead is to to fallback to nanosleep (2s) for such case
(another possibility would be to just assert/abort).

--

diff --git a/support/Makefile b/support/Makefile
index 23c6d74627..5a3bdd8e9f 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -56,6 +56,7 @@ libsupport-routines = \
   support_format_hostent \
   support_format_netent \
   support_isolate_in_subprocess \
+  support_process_state \
   support_ptrace \
   support_openpty \
   support_paths \
@@ -95,6 +96,7 @@ libsupport-routines = \
   xfopen \
   xfork \
   xftruncate \
+  xgetline \
   xgetsockname \
   xlisten \
   xlseek \
@@ -223,6 +225,7 @@ tests = \
   tst-support_capture_subprocess \
   tst-support_descriptors \
   tst-support_format_dns_packet \
+  tst-support-process_state \
   tst-support_quote_blob \
   tst-support_quote_string \
   tst-support_record_failure \
diff --git a/support/process_state.h b/support/process_state.h
new file mode 100644
index 0000000000..4325909efd
--- /dev/null
+++ b/support/process_state.h
@@ -0,0 +1,47 @@
+/* Wait for process state.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef SUPPORT_PROCESS_STATE_H
+#define SUPPORT_PROCESS_STATE_H
+
+#include <sys/types.h>
+
+enum support_process_state
+{
+  support_process_state_running      = 0x01,  /* R (running).  */
+  support_process_state_sleeping     = 0x02,  /* S (sleeping).  */
+  support_process_state_disk_sleep   = 0x04,  /* D (disk sleep).  */
+  support_process_state_stopped      = 0x08,  /* T (stopped).  */
+  support_process_state_tracing_stop = 0x10,  /* t (tracing stop).  */
+  support_process_state_dead         = 0x20,  /* X (dead).  */
+  support_process_state_zombie       = 0x40,  /* Z (zombie).  */
+  support_process_state_parked       = 0x80,  /* P (parked).  */
+};
+
+/* Wait for process PID to reach state STATE.  It can be a combination of
+   multiple possible states ('process_state_running | process_state_sleeping')
+   where the function return when any of these state are observed.
+   The timeout POLL_TS might be used to wait before polling the PID status
+   information from the kernel.  A NULL value will set to use a default
+   value (10000000 ns).
+   For an invalid state not represented by SUPPORT_PROCESS_STATE, it fallbacks
+   to a 2 second sleep.  */
+void support_process_state_wait (pid_t pid, enum support_process_state state,
+ struct timespec *poll_ts);
+
+#endif
diff --git a/support/support_process_state.c b/support/support_process_state.c
new file mode 100644
index 0000000000..6a6dbc6287
--- /dev/null
+++ b/support/support_process_state.c
@@ -0,0 +1,91 @@
+/* Wait for process state.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <time.h>
+#include <string.h>
+
+#include <array_length.h>
+
+#include <support/process_state.h>
+#include <support/xstdio.h>
+#include <support/check.h>
+
+void
+support_process_state_wait (pid_t pid, enum support_process_state state,
+    struct timespec *poll_ts)
+{
+#ifdef __linux__
+  /* For Linux it does a polling check on /proc/<pid>/status checking on
+     third field.  */
+
+  /* It mimics the kernel states from fs/proc/array.c  */
+  static struct process_states
+  {
+    enum support_process_state s;
+    char v;
+  } process_states[] = {
+    { support_process_state_running,      'R' },
+    { support_process_state_sleeping,     'S' },
+    { support_process_state_disk_sleep,   'D' },
+    { support_process_state_stopped,      'T' },
+    { support_process_state_tracing_stop, 't' },
+    { support_process_state_dead,         'X' },
+    { support_process_state_zombie,       'Z' },
+    { support_process_state_parked,       'P' },
+  };
+
+  char spath[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/status") + 1];
+  snprintf (spath, sizeof (spath), "/proc/%i/status", pid);
+
+  FILE *fstatus = xfopen (spath, "r");
+  char *line = NULL;
+  size_t linesiz = 0;
+
+  for (;;)
+    {
+      char cur_state = -1;
+      while (xgetline (&line, &linesiz, fstatus) != -1)
+ if (strncmp (line, "State:", strlen ("State:")) == 0)
+  {
+    TEST_COMPARE (sscanf (line, "%*s %c", &cur_state), 1);
+    break;
+  }
+      if (cur_state == -1)
+ break;
+
+      for (size_t i = 0; i < array_length (process_states); ++i)
+ if (state & process_states[i].s && cur_state == process_states[i].v)
+  {
+    free (line);
+    xfclose (fstatus);
+    return;
+  }
+
+      rewind (fstatus);
+      fflush (fstatus);
+
+      nanosleep (poll_ts ? poll_ts : &(struct timespec) { 0, 10000000 }, NULL);
+    }
+
+  free (line);
+  xfclose (fstatus);
+  /* Fallback to nanosleep if an invalid state is found.  */
+#endif
+  nanosleep (&(struct timespec) { 2, 0 }, NULL);
+}
diff --git a/support/tst-support-process_state.c b/support/tst-support-process_state.c
new file mode 100644
index 0000000000..3615bbe192
--- /dev/null
+++ b/support/tst-support-process_state.c
@@ -0,0 +1,105 @@
+/* Wait for process state tests.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <support/test-driver.h>
+#include <support/process_state.h>
+#include <support/check.h>
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+
+#ifndef WEXITED
+# define WEXITED 0
+#endif
+
+static void
+sigusr1_handler (int signo)
+{
+}
+
+static void
+test_child (void)
+{
+  xsignal (SIGUSR1, sigusr1_handler);
+
+  raise (SIGSTOP);
+
+  TEST_COMPARE (pause (), -1);
+  TEST_COMPARE (errno, EINTR);
+
+  while (1)
+    asm ("");
+}
+
+static int
+do_test (void)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    {
+      test_child ();
+      _exit (127);
+    }
+
+  /* Adding process_state_tracing_stop ('t') allows the test to work under
+     trace programs such as ptrace.  */
+  enum support_process_state stop_state = support_process_state_stopped
+    | support_process_state_tracing_stop;
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_stopped/state_tracing_stop\n",
+    (int) pid);
+  support_process_state_wait (pid, stop_state, NULL);
+
+  if (kill (pid, SIGCONT) != 0)
+    FAIL_RET ("kill (%d, SIGCONT): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_sleeping\n", (int) pid);
+  support_process_state_wait (pid, support_process_state_sleeping, NULL);
+
+  if (kill (pid, SIGUSR1) != 0)
+    FAIL_RET ("kill (%d, SIGUSR1): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_running\n", (int) pid);
+  support_process_state_wait (pid, support_process_state_running, NULL);
+
+  if (kill (pid, SIGKILL) != 0)
+    FAIL_RET ("kill (%d, SIGKILL): %m\n", pid);
+
+  if (test_verbose)
+    printf ("info: waiting pid %d, state_zombie\n", (int) pid);
+  support_process_state_wait (pid, support_process_state_zombie, NULL);
+
+  siginfo_t info;
+  int r = waitid (P_PID, pid, &info, WEXITED);
+  TEST_COMPARE (r, 0);
+  TEST_COMPARE (info.si_signo, SIGCHLD);
+  TEST_COMPARE (info.si_code, CLD_KILLED);
+  TEST_COMPARE (info.si_status, SIGKILL);
+  TEST_COMPARE (info.si_pid, pid);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/support/xgetline.c b/support/xgetline.c
new file mode 100644
index 0000000000..5b010bcf56
--- /dev/null
+++ b/support/xgetline.c
@@ -0,0 +1,34 @@
+/* fopen with error checking.
+   Copyright (C) 2016-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xstdio.h>
+
+#include <support/check.h>
+#include <errno.h>
+
+ssize_t
+xgetline (char **lineptr, size_t *n, FILE *stream)
+{
+  int old_errno = errno;
+  errno = 0;
+  ssize_t ret = getline (lineptr, n, stream);
+  if (ret == -1 && errno != 0)
+    FAIL_EXIT1 ("getline failed: %m");
+  errno = old_errno;
+  return ret;
+}
diff --git a/support/xstdio.h b/support/xstdio.h
index e4e52b4c7e..df6ee27711 100644
--- a/support/xstdio.h
+++ b/support/xstdio.h
@@ -27,6 +27,8 @@ __BEGIN_DECLS
 FILE *xfopen (const char *path, const char *mode);
 void xfclose (FILE *);
 
+ssize_t xgetline (char **lineptr, size_t *n, FILE *stream);
+
 __END_DECLS
 
 #endif /* SUPPORT_XSTDIO_H */