Patchwork [1/3] support: Add support_process_state_wait

login
register
mail settings
Submitter Adhemerval Zanella Netto
Date Dec. 26, 2019, 2:20 p.m.
Message ID <5bd5cb39-1906-bb76-13f9-c7d2dc31b68d@linaro.org>
Download mbox | patch
Permalink /patch/37091/
State New
Headers show

Comments

Adhemerval Zanella Netto - Dec. 26, 2019, 2:20 p.m.
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).

--

Patch

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 */