* [ptest-runner][PATCH 1/4] utils.c: get_available_ptests allow to specify relative directories
@ 2021-03-23 2:10 Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 2/4] utils.c: Fix exit status of a child Anibal Limon
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Anibal Limon @ 2021-03-23 2:10 UTC (permalink / raw)
To: yocto; +Cc: yifan.yu, Randy.MacLeod, nicolas.dechesne, Aníbal Limón
Fixes,
$ ./ptest-runner -d ./tests/data bash
Signed-off-by: Aníbal Limón <anibal.limon@linaro.org>
---
utils.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/utils.c b/utils.c
index a4e190e..43ab03b 100644
--- a/utils.c
+++ b/utils.c
@@ -34,6 +34,7 @@
#include <poll.h>
#include <pty.h>
#include <signal.h>
+#include <limits.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
@@ -85,6 +86,9 @@ get_available_ptests(const char *dir)
struct dirent **namelist;
int fail;
int saved_errno = -1; /* Initalize to invalid errno. */
+ char realdir[PATH_MAX];
+
+ realpath(dir, realdir);
do
{
@@ -93,7 +97,7 @@ get_available_ptests(const char *dir)
if (head == NULL)
break;
- if (stat(dir, &st_buf) == -1) {
+ if (stat(realdir, &st_buf) == -1) {
PTEST_LIST_FREE_CLEAN(head);
break;
}
@@ -104,7 +108,7 @@ get_available_ptests(const char *dir)
break;
}
- n = scandir(dir, &namelist, NULL, alphasort);
+ n = scandir(realdir, &namelist, NULL, alphasort);
if (n == -1) {
PTEST_LIST_FREE_CLEAN(head);
break;
@@ -130,7 +134,7 @@ get_available_ptests(const char *dir)
}
if (asprintf(&run_ptest, "%s/%s/ptest/run-ptest",
- dir, d_name) == -1) {
+ realdir, d_name) == -1) {
fail = 1;
saved_errno = errno;
free(d_name);
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [ptest-runner][PATCH 2/4] utils.c: Fix exit status of a child
2021-03-23 2:10 [ptest-runner][PATCH 1/4] utils.c: get_available_ptests allow to specify relative directories Anibal Limon
@ 2021-03-23 2:10 ` Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 3/4] utils.c: Use a thread to read from child Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm Anibal Limon
2 siblings, 0 replies; 6+ messages in thread
From: Anibal Limon @ 2021-03-23 2:10 UTC (permalink / raw)
To: yocto; +Cc: yifan.yu, Randy.MacLeod, nicolas.dechesne, Aníbal Limón
Modify testcase to validate the actual exit status.
[YOCTO #14217]
Signed-off-by: Aníbal Limón <anibal.limon@linaro.org>
---
tests/data/fail/ptest/run-ptest | 3 +++
tests/utils.c | 2 +-
utils.c | 3 +++
3 files changed, 7 insertions(+), 1 deletion(-)
mode change 100644 => 100755 tests/data/fail/ptest/run-ptest
diff --git a/tests/data/fail/ptest/run-ptest b/tests/data/fail/ptest/run-ptest
old mode 100644
new mode 100755
index e69de29..f4e5d0b
--- a/tests/data/fail/ptest/run-ptest
+++ b/tests/data/fail/ptest/run-ptest
@@ -0,0 +1,3 @@
+#!/bin/bash
+
+exit 10
diff --git a/tests/utils.c b/tests/utils.c
index 132d98f..105e0c8 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -234,7 +234,7 @@ END_TEST
static void
search_for_fail(const int rp, FILE *fp_stdout)
{
- const char *fail_str = "ERROR: Exit status is";
+ const char *fail_str = "ERROR: Exit status is 10";
char line_buf[PRINT_PTEST_BUF_SIZE];
int found_fail = 0;
char *line = NULL;
diff --git a/utils.c b/utils.c
index 43ab03b..d784736 100644
--- a/utils.c
+++ b/utils.c
@@ -352,6 +352,9 @@ wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
clock_gettime(clock, &sentinel);
}
+
+ if (WIFEXITED(status))
+ status = WEXITSTATUS(status);
}
fflush(fps[0]);
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [ptest-runner][PATCH 3/4] utils.c: Use a thread to read from child
2021-03-23 2:10 [ptest-runner][PATCH 1/4] utils.c: get_available_ptests allow to specify relative directories Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 2/4] utils.c: Fix exit status of a child Anibal Limon
@ 2021-03-23 2:10 ` Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm Anibal Limon
2 siblings, 0 replies; 6+ messages in thread
From: Anibal Limon @ 2021-03-23 2:10 UTC (permalink / raw)
To: yocto; +Cc: yifan.yu, Randy.MacLeod, nicolas.dechesne, Aníbal Limón
In order to handle large output add a thread for read from childs using
a pipe and remove non-blocking option.
Modify bash unittest to output large data and cover this scenario.
[YOCTO #14220]
Signed-off-by: Aníbal Limón <anibal.limon@linaro.org>
---
Makefile | 2 +-
tests/data/bash/ptest/run-ptest | 4 +-
utils.c | 112 +++++++++++++++++++++-----------
3 files changed, 78 insertions(+), 40 deletions(-)
diff --git a/Makefile b/Makefile
index 3cca17b..1aa1830 100644
--- a/Makefile
+++ b/Makefile
@@ -31,7 +31,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
all: $(SOURCES) $(EXECUTABLE)
$(EXECUTABLE): $(OBJECTS)
- $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
+ $(CC) $(LDFLAGS) $(OBJECTS) -pthread -lutil -o $@
tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
diff --git a/tests/data/bash/ptest/run-ptest b/tests/data/bash/ptest/run-ptest
index 09f997f..f36e077 100755
--- a/tests/data/bash/ptest/run-ptest
+++ b/tests/data/bash/ptest/run-ptest
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
echo "bash"
printf "Hello World!,stderr\n" >&2
@@ -7,3 +7,5 @@ printf "Hello World!,stderr2\n" >&2
echo "bash3"
echo "bash4"
printf "Hello World!,stderr3\n" >&2
+
+echo {1..1000000}\\n
diff --git a/utils.c b/utils.c
index d784736..84cb570 100644
--- a/utils.c
+++ b/utils.c
@@ -39,6 +39,7 @@
#include <string.h>
#include <time.h>
#include <unistd.h>
+#include <pthread.h>
#include <sys/ioctl.h>
#include <sys/resource.h>
@@ -53,6 +54,11 @@
#define WAIT_CHILD_POLL_TIMEOUT_MS 200
#define WAIT_CHILD_BUF_MAX_SIZE 1024
+static struct {
+ int fds[2];
+ FILE *fps[2];
+} _child_reader;
+
static inline char *
get_stime(char *stime, size_t size, time_t t)
{
@@ -269,6 +275,44 @@ close_fds(void)
}
}
+static void *
+read_child(void *arg)
+{
+ struct pollfd pfds[2];
+ int r;
+
+ pfds[0].fd = _child_reader.fds[0];
+ pfds[0].events = POLLIN;
+ pfds[1].fd = _child_reader.fds[1];
+ pfds[1].events = POLLIN;
+
+ do {
+ r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS);
+ if (r > 0) {
+ char buf[WAIT_CHILD_BUF_MAX_SIZE];
+ ssize_t n;
+
+ if (pfds[0].revents != 0) {
+ n = read(_child_reader.fds[0], buf, WAIT_CHILD_BUF_MAX_SIZE);
+ if (n > 0)
+ fwrite(buf, (size_t)n, 1, _child_reader.fps[0]);
+ }
+
+ if (pfds[1].revents != 0) {
+ n = read(_child_reader.fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE);
+ if (n > 0)
+ fwrite(buf, (size_t)n, 1, _child_reader.fps[1]);
+ }
+
+ }
+
+ fflush(_child_reader.fps[0]);
+ fflush(_child_reader.fps[1]);
+ } while (1);
+
+ return NULL;
+}
+
static inline void
run_child(char *run_ptest, int fd_stdout, int fd_stderr)
{
@@ -292,22 +336,15 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
}
static inline int
-wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
+wait_child(pid_t pid, int timeout, int *timeouted)
{
- struct pollfd pfds[2];
struct timespec sentinel;
clockid_t clock = CLOCK_MONOTONIC;
int looping = 1;
- int r;
int status = -1;
int waitflags;
- pfds[0].fd = fds[0];
- pfds[0].events = POLLIN;
- pfds[1].fd = fds[1];
- pfds[1].events = POLLIN;
-
if (clock_gettime(clock, &sentinel) == -1) {
clock = CLOCK_REALTIME;
clock_gettime(clock, &sentinel);
@@ -332,32 +369,12 @@ wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
if (waitpid(pid, &status, waitflags) == pid)
looping = 0;
- r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS);
- if (r > 0) {
- char buf[WAIT_CHILD_BUF_MAX_SIZE];
- ssize_t n;
-
- if (pfds[0].revents != 0) {
- while ((n = read(fds[0], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
- fwrite(buf, (size_t)n, 1, fps[0]);
- }
-
- if (pfds[1].revents != 0) {
- while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
- fflush(fps[0]);
- fwrite(buf, (size_t)n, 1, fps[1]);
- fflush(fps[1]);
- }
- }
-
- clock_gettime(clock, &sentinel);
- }
+ clock_gettime(clock, &sentinel);
if (WIFEXITED(status))
status = WEXITSTATUS(status);
}
- fflush(fps[0]);
return status;
}
@@ -426,6 +443,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
time_t duration;
int slave;
int pgid = -1;
+ pthread_t tid;
if (opts.xml_filename) {
xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -435,18 +453,32 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
do
{
- if ((rc = pipe2(pipefd_stdout, O_NONBLOCK)) == -1)
+ if ((rc = pipe(pipefd_stdout)) == -1)
break;
- if ((rc = pipe2(pipefd_stderr, O_NONBLOCK)) == -1) {
+ if ((rc = pipe(pipefd_stderr)) == -1) {
close(pipefd_stdout[0]);
close(pipefd_stdout[1]);
break;
}
- fprintf(fp, "START: %s\n", progname);
+
if (isatty(0) && ioctl(0, TIOCNOTTY) == -1) {
fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
}
+
+ _child_reader.fds[0] = pipefd_stdout[0];
+ _child_reader.fds[1] = pipefd_stderr[0];
+ _child_reader.fps[0] = fp;
+ _child_reader.fps[1] = fp_stderr;
+ rc = pthread_create(&tid, NULL, read_child, NULL);
+ if (rc != 0) {
+ fprintf(fp, "ERROR: Failed to create reader thread, %s\n", strerror(errno));
+ close(pipefd_stdout[0]);
+ close(pipefd_stdout[1]);
+ break;
+ }
+
+ fprintf(fp, "START: %s\n", progname);
PTEST_LIST_ITERATE_START(head, p)
char *ptest_dir = strdup(p->run_ptest);
if (ptest_dir == NULL) {
@@ -485,8 +517,6 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
} else {
int status;
- int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
- FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;
if (setpgid(child, pgid) == -1) {
fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
@@ -496,17 +526,20 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
fprintf(fp, "BEGIN: %s\n", ptest_dir);
- status = wait_child(child, opts.timeout, fds, fps, &timeouted);
+
+ status = wait_child(child, opts.timeout, &timeouted);
+
+
entime = time(NULL);
duration = entime - sttime;
if (status) {
- fprintf(fps[0], "\nERROR: Exit status is %d\n", status);
+ fprintf(fp, "\nERROR: Exit status is %d\n", status);
rc += 1;
}
- fprintf(fps[0], "DURATION: %d\n", (int) duration);
+ fprintf(fp, "DURATION: %d\n", (int) duration);
if (timeouted)
- fprintf(fps[0], "TIMEOUT: %s\n", ptest_dir);
+ fprintf(fp, "TIMEOUT: %s\n", ptest_dir);
if (opts.xml_filename)
xml_add_case(xh, status, ptest_dir, timeouted, (int) duration);
@@ -517,6 +550,9 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
PTEST_LIST_ITERATE_END
fprintf(fp, "STOP: %s\n", progname);
+ pthread_cancel(tid);
+ pthread_join(tid, NULL);
+
close(pipefd_stdout[0]); close(pipefd_stdout[1]);
close(pipefd_stderr[0]); close(pipefd_stderr[1]);
} while (0);
--
2.31.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm
2021-03-23 2:10 [ptest-runner][PATCH 1/4] utils.c: get_available_ptests allow to specify relative directories Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 2/4] utils.c: Fix exit status of a child Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 3/4] utils.c: Use a thread to read from child Anibal Limon
@ 2021-03-23 2:10 ` Anibal Limon
2021-03-24 15:56 ` Yi Fan Yu
2 siblings, 1 reply; 6+ messages in thread
From: Anibal Limon @ 2021-03-23 2:10 UTC (permalink / raw)
To: yocto; +Cc: yifan.yu, Randy.MacLeod, nicolas.dechesne, Aníbal Limón
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@linaro.org>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm
2021-03-23 2:10 ` [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm Anibal Limon
@ 2021-03-24 15:56 ` Yi Fan Yu
2021-03-24 18:30 ` Anibal Limon
0 siblings, 1 reply; 6+ messages in thread
From: Yi Fan Yu @ 2021-03-24 15:56 UTC (permalink / raw)
To: Aníbal Limón, yocto; +Cc: Randy.MacLeod, nicolas.dechesne
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm
2021-03-24 15:56 ` Yi Fan Yu
@ 2021-03-24 18:30 ` Anibal Limon
0 siblings, 0 replies; 6+ messages in thread
From: Anibal Limon @ 2021-03-24 18:30 UTC (permalink / raw)
To: Yi Fan Yu; +Cc: Yocto discussion list, MacLeod, Randy, Nicolas Dechesne
[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]
On Wed, 24 Mar 2021 at 09:56, Yi Fan Yu <yifan.yu@windriver.com> wrote:
> 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
>
Yeah, thanks for remind me the old behavior, some years and I forget about
it, I will fix it and put certain comments about that.
>
>
> 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.
>
I pushed to dev branch,
http://git.yoctoproject.org/cgit/cgit.cgi/ptest-runner2/log/?h=dev
Regards,
Anibal
>
> yifan
>
[-- Attachment #2: Type: text/html, Size: 2108 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-24 18:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 2:10 [ptest-runner][PATCH 1/4] utils.c: get_available_ptests allow to specify relative directories Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 2/4] utils.c: Fix exit status of a child Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 3/4] utils.c: Use a thread to read from child Anibal Limon
2021-03-23 2:10 ` [ptest-runner][PATCH 4/4] utils.c: wait_child reimplement timeout using alarm Anibal Limon
2021-03-24 15:56 ` Yi Fan Yu
2021-03-24 18:30 ` Anibal Limon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.