Since we are using threads to read from child, no complex logic is
needed for handle timeouts use alarm(2).

[YOCTO #14220]

Signed-off-by: Aníbal Limón <anibal.limon@...>
---
utils.c | 60 ++++++++++++++++++++-------------------------------------
1 file changed, 21 insertions(+), 39 deletions(-)

diff --git a/utils.c b/utils.c
index 84cb570..1a3c90f 100644
--- a/utils.c
+++ b/utils.c
@@ -57,6 +57,9 @@
static struct {
int fds[2];
FILE *fps[2];
+
+ int timeouted;
+ pid_t pid;
} _child_reader;

static inline char *
@@ -335,45 +338,25 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
/* exit(1); not needed? */
}

-static inline int
-wait_child(pid_t pid, int timeout, int *timeouted)
+static void
+timeout_child_handler(int signo)
{
- struct timespec sentinel;
- clockid_t clock = CLOCK_MONOTONIC;
- int looping = 1;
+ _child_reader.timeouted = 1;
+ kill(-_child_reader.pid, SIGKILL);
+}

+static inline int
+wait_child(pid_t pid, int timeout)
+{
int status = -1;
- int waitflags;
-
- if (clock_gettime(clock, &sentinel) == -1) {
- clock = CLOCK_REALTIME;
- clock_gettime(clock, &sentinel);
- }
-
- *timeouted = 0;

- while (looping) {
- waitflags = WNOHANG;
+ _child_reader.timeouted = 0;
+ _child_reader.pid = pid;

- if (timeout >= 0) {
- struct timespec time;
-
- clock_gettime(clock, &time);
- if ((time.tv_sec - sentinel.tv_sec) > timeout) {
- *timeouted = 1;
- kill(-pid, SIGKILL);
- waitflags = 0;
- }
- }
-
- if (waitpid(pid, &status, waitflags) == pid)
- looping = 0;
-
- clock_gettime(clock, &sentinel);
-
- if (WIFEXITED(status))
- status = WEXITSTATUS(status);
- }
+ alarm(timeout);
+ waitpid(pid, &status, 0);
+ if (WIFEXITED(status))
+ status = WEXITSTATUS(status);

return status;
}
@@ -438,7 +421,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
pid_t child;
int pipefd_stdout[2];
int pipefd_stderr[2];
- int timeouted;
time_t sttime, entime;
time_t duration;
int slave;
@@ -477,6 +459,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
close(pipefd_stdout[1]);
break;
}
+ signal(SIGALRM, timeout_child_handler);

fprintf(fp, "START: %s\n", progname);
PTEST_LIST_ITERATE_START(head, p)
@@ -527,8 +510,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
fprintf(fp, "BEGIN: %s\n", ptest_dir);


- status = wait_child(child, opts.timeout, &timeouted);
-
+ status = wait_child(child, opts.timeout);

entime = time(NULL);
duration = entime - sttime;
@@ -538,11 +520,11 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
rc += 1;
}
fprintf(fp, "DURATION: %d\n", (int) duration);
- if (timeouted)
+ if (_child_reader.timeouted)
fprintf(fp, "TIMEOUT: %s\n", ptest_dir);

if (opts.xml_filename)
- xml_add_case(xh, status, ptest_dir, timeouted, (int) duration);
+ xml_add_case(xh, status, ptest_dir, _child_reader.timeouted, (int) duration);

fprintf(fp, "END: %s\n", ptest_dir);
fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, entime));
--
2.31.0

On 3/22/21 10:10 PM, Aníbal Limón wrote:

Since we are using threads to read from child, no complex logic is
needed for handle timeouts use alarm(2).

TY for the patches.

This would change its behaviour to timeout exactly at X seconds.

New behaviour: if test exceeds X seconds, gets killed.
Old behaviour: if test exceeds X seconds with no output, get killed. If output is detected X-1 seconds, the timeout timer resets.
so a program could take 2X seconds and not get killed


Comment about the other patches:

We don't even need an extra thread because before the wait_for_child had to do 2 things:

* read a pipe and output that pipe to stdout (child_reader does that)
* check for timeout (sigalarm takes care of that now)



last thing, can you push this to a branch (let's say) master-next, so I can test it with a full ptest run.

yifan