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).
--
@@ -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 \
new file mode 100644
@@ -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
new file mode 100644
@@ -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);
+}
new file mode 100644
@@ -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>
new file mode 100644
@@ -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;
+}
@@ -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 */