michaelh/zephyr
1
0
Fork 0
Code Releases Activity

ztest: end tests more robustly

Browse source 
If a ztest test case creates child thread(s), and one of the
descendent threads invokes ztest_test_pass(), ztest_test_fail(),
or ztest_thread_skip(), only that descendent thread will be
aborted.

Then ztest will try to run the next scenario on the ztest_thread
which is already in use. This was causing corruption issues on
SMP systems, and possibly other subtle, hard-to-debug
situations.

This patch ensures that ztest_thread is always dead before
re-using it, as run_test() now attempts to join on it instead
of using a semaphore.

The ztest_test_* functions now ensure that the ztest_thread
is always aborted, in addition to the current thread.

This isn't perfect. If the testcase spawned other threads,
they will keep running. The most robust way to fix this is to
iterate over all non-essential threads in the system and abort
them. Unfortunately, Zephyr doesn't have a facility to do
this safely.

It would also be simpler to re-use thread objects if
k_thread_create() could detect whether the thread was already
active and abort it, but this is currently not possible
since k_thread_create() can be used with uninitialzed
thread object memory and no checks are possible. This
may be improved in the future, see #23030.

Fixes: #22738
Partial fix for: #24713

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This commit is contained in:
Andrew Boie 2020-05-05 20:34:12 -07:00 • committed by Anas Nashif
commit 1ff066548a
1 changed files with 6 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

29
subsys/testsuite/ztest/src/ztest.c
View file

@ -241,33 +241,30 @@ K_THREAD_STACK_DEFINE(ztest_thread_stack, CONFIG_ZTEST_STACKSIZE +
CONFIG_TEST_EXTRA_STACKSIZE);
static ZTEST_BMEM int test_result;
static struct k_sem test_end_signal;
void ztest_test_fail(void)
{
test_result = -1;
k_sem_give(&test_end_signal);
k_thread_abort(&ztest_thread);
k_thread_abort(k_current_get());
}
void ztest_test_pass(void)
{
test_result = 0;
k_sem_give(&test_end_signal);
k_thread_abort(&ztest_thread);
k_thread_abort(k_current_get());
}
void ztest_test_skip(void)
{
test_result = -2;
k_sem_give(&test_end_signal);
k_thread_abort(&ztest_thread);
k_thread_abort(k_current_get());
}
static void init_testing(void)
{
k_sem_init(&test_end_signal, 0, 1);
k_object_access_all_grant(&test_end_signal);
k_object_access_all_grant(&ztest_thread);
}
static void test_cb(void *a, void *dummy2, void *dummy)
@ -280,8 +277,6 @@ static void test_cb(void *a, void *dummy2, void *dummy)
test_result = 1;
run_test_functions(test);
test_result = 0;
k_sem_give(&test_end_signal);
}
static int run_test(struct unit_test *test)
@ -295,20 +290,8 @@ static int run_test(struct unit_test *test)
NULL, NULL, CONFIG_ZTEST_THREAD_PRIORITY,
test->thread_options | K_INHERIT_PERMS,
K_NO_WAIT);
/*
* There is an implicit expectation here that the thread that was
* spawned is still higher priority than the current thread.
*
* If that is not the case, it will have given the semaphore, which
* will have caused the current thread to run, _if_ the test case
* thread is preemptible, since it is higher priority. If there is
* another test case to be run after the current one finishes, the
* thread_stack will be reused for that new test case while the current
* test case has not finished running yet (it has given the semaphore,
* but has _not_ gone back to z_thread_entry() and completed it's "abort
* phase": this will corrupt the kernel ready queue.
*/
k_sem_take(&test_end_signal, K_FOREVER);
k_thread_join(&ztest_thread, K_FOREVER);
phase = TEST_PHASE_TEARDOWN;
test->teardown();