8259375 ("runner: Remove threads and mutexes") did not correctly account
for the case where multiple tests are being run in parallel by
ptest-runner. Fix the code to track all of the child processes (but
still have them share the same stdout/stderr pipe) and wait for all of
them to finish before exiting.
Signed-off-by: Joshua Watt <jpewhac...@gmail.com>
---
tests/utils.c | 39 +++++++-
utils.c | 248 +++++++++++++++++++++++++++++---------------------
2 files changed, 182 insertions(+), 105 deletions(-)
diff --git a/tests/utils.c b/tests/utils.c
index 19657ee..4560e93 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -50,9 +50,10 @@ static char *ptests_found[] = {
"glibc",
"hang",
"python",
+ "signal",
NULL
};
-static int ptests_found_length = 6;
+static int ptests_found_length = 7;
static char *ptests_not_found[] = {
"busybox",
"perl",
@@ -186,6 +187,7 @@ START_TEST(test_run_ptests)
head = get_available_ptests(opts_directory);
ptest_list_remove(head, "hang", 1);
ptest_list_remove(head, "fail", 1);
+ ptest_list_remove(head, "signal", 1);
rc = run_ptests(head, opts, "test_run_ptests", fp_stdout, fp_stderr);
ck_assert(rc == 0);
@@ -215,8 +217,8 @@ search_for_timeout_and_duration(const int rp, FILE
*fp_stdout)
found_duration = found_duration ? found_duration :
find_word(line, duration_str);
}
- ck_assert(found_timeout == true);
- ck_assert(found_duration == true);
+ ck_assert_msg(found_timeout == true, "TIMEOUT not found");
+ ck_assert_msg(found_duration == true, "DURATION not found");
}
START_TEST(test_run_timeout_duration_ptest)
@@ -230,6 +232,36 @@ START_TEST(test_run_timeout_duration_ptest)
}
END_TEST
+static void
+search_for_signal_and_duration(const int rp, FILE *fp_stdout)
+{
+ char line_buf[PRINT_PTEST_BUF_SIZE];
+ bool found_error = false, found_duration = false;
+ char *line = NULL;
+
+ ck_assert(rp != 0);
+
+ while ((line = fgets(line_buf, PRINT_PTEST_BUF_SIZE, fp_stdout)) !=
NULL) {
+ // once true, stay true
+ found_error = found_error ? found_error : find_word(line,
"ERROR: Exited with signal");
+ found_duration = found_duration ? found_duration :
find_word(line, "DURATION");
+ }
+
+ ck_assert_msg(found_error == true, "ERROR not found");
+ ck_assert_msg(found_duration == true, "DURATION not found");
+}
+
+START_TEST(test_run_signal_ptest)
+{
+ struct ptest_list *head = get_available_ptests(opts_directory);
+ unsigned int timeout = 10;
+
+ test_ptest_expected_failure(head, timeout, "signal",
search_for_signal_and_duration);
+
+ ptest_list_free_all(head);
+}
+END_TEST
+
static void
search_for_fail(const int rp, FILE *fp_stdout)
{
@@ -318,6 +350,7 @@ utils_suite(void)
tcase_add_test(tc_core, test_filter_ptests);
tcase_add_test(tc_core, test_run_ptests);
tcase_add_test(tc_core, test_run_timeout_duration_ptest);
+ tcase_add_test(tc_core, test_run_signal_ptest);
tcase_add_test(tc_core, test_run_fail_ptest);
tcase_add_test(tc_core, test_xml_pass);
tcase_add_test(tc_core, test_xml_fail);
diff --git a/utils.c b/utils.c
index 6a6e848..c444b2a 100644
--- a/utils.c
+++ b/utils.c
@@ -60,11 +60,20 @@ static struct {
FILE *fps[2];
unsigned int timeout;
- int timeouted;
- pid_t pid;
int padding1;
} _child_reader;
+struct running_test {
+ struct running_test *next;
+ char *ptest_dir;
+ pid_t pid;
+ time_t start_time;
+ time_t end_time;
+ int status;
+ bool timed_out;
+ bool exited;
+};
+
static inline char *
get_stime(char *stime, size_t size, time_t t)
{
@@ -344,16 +353,18 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
/* exit(1); not needed? */
}
-static inline int
-wait_child(pid_t pid)
+static void
+wait_child(struct running_test *test, int options)
{
- int status = -1;
-
- waitpid(pid, &status, 0);
- if (WIFEXITED(status))
- status = WEXITSTATUS(status);
+ int status;
- return status;
+ if (!test->exited) {
+ if (waitpid(test->pid, &status, options) == test->pid) {
+ test->status = status;
+ test->end_time = time(NULL);
+ test->exited = true;
+ }
+ }
}
/* Returns an integer file descriptor.
@@ -416,10 +427,9 @@ run_ptests(struct ptest_list *head, const struct
ptest_options opts,
pid_t child;
int pipefd_stdout[2];
int pipefd_stderr[2];
- time_t sttime, entime;
- time_t duration;
int slave;
int pgid = -1;
+ struct running_test *running_tests = NULL;
if (opts.xml_filename) {
xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -447,7 +457,6 @@ run_ptests(struct ptest_list *head, const struct
ptest_options opts,
_child_reader.fps[0] = fp;
_child_reader.fps[1] = fp_stderr;
_child_reader.timeout = opts.timeout;
- _child_reader.timeouted = 0;
fprintf(fp, "START: %s\n", progname);
PTEST_LIST_ITERATE_START(head, p)
@@ -491,123 +500,158 @@ run_ptests(struct ptest_list *head, const struct
ptest_options opts,
run_child(p->run_ptest, pipefd_stdout[1],
pipefd_stderr[1]);
} else {
- int status;
-
- /* Close write ends of the pipe, otherwise this
process will never get EOF when the child dies */
- do_close(&pipefd_stdout[1]);
- do_close(&pipefd_stderr[1]);
+ struct running_test *test =
malloc(sizeof(*test));
+ test->pid = child;
+ test->status = 0;
+ test->timed_out = false;
+ test->exited = false;
+ test->ptest_dir = ptest_dir;
+ test->start_time = time(NULL);
+ test->next = running_tests;
+ running_tests = test;
- _child_reader.pid = child;
if (setpgid(child, pgid) == -1) {
fprintf(fp, "ERROR: setpgid() failed,
%s\n", strerror(errno));
}
- sttime = time(NULL);
- fprintf(fp, "%s\n", get_stime(stime,
GET_STIME_BUF_SIZE, sttime));
+ fprintf(fp, "%s\n", get_stime(stime,
GET_STIME_BUF_SIZE, test->start_time));
fprintf(fp, "BEGIN: %s\n", ptest_dir);
+ }
+ PTEST_LIST_ITERATE_END
+
+ /*
+ * Close write ends of the pipe, otherwise this process will
+ * never get EOF when the child dies
+ */
+ do_close(&pipefd_stdout[1]);
+ do_close(&pipefd_stderr[1]);
- set_nonblocking(_child_reader.fds[0]);
- set_nonblocking(_child_reader.fds[1]);
+ set_nonblocking(_child_reader.fds[0]);
+ set_nonblocking(_child_reader.fds[1]);
- struct pollfd pfds[2];
- for (int i = 0; i < 2; i++) {
- pfds[i].fd = _child_reader.fds[i];
- pfds[i].events = POLLIN;
+ struct pollfd pfds[2];
+ for (int i = 0; i < 2; i++) {
+ pfds[i].fd = _child_reader.fds[i];
+ pfds[i].events = POLLIN;
+ }
+
+ while (true) {
+ /*
+ * Check all the poll file descriptors. Only when all
+ * of them are done (negative) will the poll() loop
+ * exit. This ensures all output is read from all
+ * children.
+ */
+ bool done = true;
+ for (int i = 0; i < 2; i++) {
+ if (pfds[i].fd >= 0) {
+ done = false;
+ break;
}
+ }
- while (true) {
- /*
- * Check all the poll file descriptors.
- * Only when all of them are done
- * (negative) will we exit the poll()
- * loop
- */
- bool done = true;
- for (int i = 0; i < 2; i++) {
- if (pfds[i].fd >= 0) {
- done = false;
- break;
- }
- }
+ if (done) {
+ break;
+ }
- if (done) {
- break;
- }
+ int ret = poll(pfds, 2, _child_reader.timeout*1000);
- int ret = poll(pfds, 2,
_child_reader.timeout*1000);
-
- if (ret == 0 &&
!_child_reader.timeouted) {
- /* kill the child if we haven't
- * already. Note that we
- * continue to read data from
- * the pipes until EOF to make
- * sure we get all the output
- */
- kill(-_child_reader.pid,
SIGKILL);
- _child_reader.timeouted = 1;
+ for (int i = 0; i < 2; i++) {
+ if (pfds[i].revents & (POLLIN | POLLHUP)) {
+ char buf[WAIT_CHILD_BUF_MAX_SIZE];
+ ssize_t n = read(pfds[i].fd, buf,
sizeof(buf));
+
+ if (n == 0) {
+ /* Closed */
+ pfds[i].fd = -1;
+ continue;
}
- for (int i = 0; i < 2; i++) {
- if (pfds[i].revents & (POLLIN |
POLLHUP)) {
- char
buf[WAIT_CHILD_BUF_MAX_SIZE];
- ssize_t n =
read(pfds[i].fd, buf, sizeof(buf));
-
- if (n == 0) {
- /* Closed */
- pfds[i].fd = -1;
- continue;
- }
-
- if (n < 0) {
- if (errno !=
EAGAIN && errno != EWOULDBLOCK) {
-
pfds[i].fd = -1;
-
fprintf(stderr, "Error reading from stream %d: %s\n", i, strerror(errno));
- }
- continue;
- } else {
- fwrite(buf,
(size_t)n, 1, _child_reader.fps[i]);
- }
+ if (n < 0) {
+ if (errno != EAGAIN && errno !=
EWOULDBLOCK) {
+ pfds[i].fd = -1;
+ fprintf(stderr, "Error
reading from stream %d: %s\n", i, strerror(errno));
}
+ continue;
+ } else {
+ fwrite(buf, (size_t)n, 1,
_child_reader.fps[i]);
}
}
- collect_system_state(_child_reader.fps[0]);
+ }
- for (int i = 0; i < 2; i++) {
- fflush(_child_reader.fps[i]);
- }
+ for (struct running_test *test = running_tests; test;
test = test->next) {
+ /*
+ * Check if this child has exited yet.
+ */
+ wait_child(test, WNOHANG);
/*
- * This kill is just in case the child did
- * something really silly like close its
- * stdout and stderr but then go into an
- * infinite loop and never exit. Normally, it
- * will just fail because the child is already
- * dead
+ * If a timeout has occurred, kill the child if
+ * it has not been done already and has not
+ * exited
*/
- if (!_child_reader.timeouted) {
- kill(-_child_reader.pid, SIGKILL);
+ if (ret == 0 && !test->exited &&
!test->timed_out) {
+ kill(-test->pid, SIGKILL);
+ test->timed_out = true;
}
- status = wait_child(child);
+ }
+ }
+ collect_system_state(_child_reader.fps[0]);
+
+ for (int i = 0; i < 2; i++) {
+ fflush(_child_reader.fps[i]);
+ }
+
+ for (struct running_test *test = running_tests; test; test =
test->next) {
+ time_t duration;
+ int exit_code = 0;
+
+ /*
+ * This kill is just in case the child did something
really
+ * silly like close its stdout and stderr but then go
into an
+ * infinite loop and never exit. Normally, it will just
fail
+ * because the child is already dead
+ */
+ if (!test->timed_out && !test->exited) {
+ kill(-test->pid, SIGKILL);
+ }
- entime = time(NULL);
- duration = entime - sttime;
+ wait_child(test, 0);
- if (status) {
- fprintf(fp, "\nERROR: Exit status is
%d\n", status);
+ duration = test->end_time - test->start_time;
+
+ if (WIFEXITED(test->status)) {
+ exit_code = WEXITSTATUS(test->status);
+ if (exit_code) {
+ fprintf(fp, "\nERROR: Exit status is
%d\n", exit_code);
rc += 1;
}
- fprintf(fp, "DURATION: %d\n", (int) duration);
- if (_child_reader.timeouted)
- fprintf(fp, "TIMEOUT: %s\n", ptest_dir);
+ } else if (WIFSIGNALED(test->status) &&
!test->timed_out) {
+ int signal = WTERMSIG(test->status);
+ fprintf(fp, "\nERROR: Exited with signal %s
(%d)\n", strsignal(signal), signal);
+ rc += 1;
+ }
+ fprintf(fp, "DURATION: %d\n", (int) duration);
+ if (test->timed_out) {
+ fprintf(fp, "TIMEOUT: %s\n", test->ptest_dir);
+ rc += 1;
+ }
- if (opts.xml_filename)
- xml_add_case(xh, status, ptest_dir,
_child_reader.timeouted, (int) duration);
+ if (opts.xml_filename)
+ xml_add_case(xh, exit_code, test->ptest_dir,
test->timed_out, (int) duration);
+
+ fprintf(fp, "END: %s\n", test->ptest_dir);
+ fprintf(fp, "%s\n", get_stime(stime,
GET_STIME_BUF_SIZE, test->end_time));
+ }
+
+ while (running_tests) {
+ struct running_test *test = running_tests;
+ running_tests = running_tests->next;
+
+ free(test->ptest_dir);
+ free(test);
+ }
- fprintf(fp, "END: %s\n", ptest_dir);
- fprintf(fp, "%s\n", get_stime(stime,
GET_STIME_BUF_SIZE, entime));
- }
- free(ptest_dir);
- PTEST_LIST_ITERATE_END
fprintf(fp, "STOP: %s\n", progname);
do_close(&pipefd_stdout[0]);
--
2.33.0
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#60568): https://lists.yoctoproject.org/g/yocto/message/60568
Mute This Topic: https://lists.yoctoproject.org/mt/100122645/21656
Group Owner: yocto+ow...@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
