Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

schedule_api test uses zassert without cleaning up properly #3666

Closed
zephyrbot opened this issue Jun 1, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@zephyrbot
Copy link
Collaborator

commented Jun 1, 2017

Reported by Andrew Boie:

see tests/kernel/threads_scheduling/schedule_api/src/test_sched_timeslice_and_lock.c

The problem is the zassert() calls in the test functions. If any fail, the functions are immediately aborted without calling teardown_threads().

Then, in the next test case, it will try to spawn the same threads again, which leads to undefined behavior, on ARM we are seeing hardfaults. See GH-3657

Either fix the test case so that all cleanup is done before zasserts are called,
or introduce variant zassert APIs which additionally take a cleanup function pointer.

(Imported from Jira ZEP-2230)

@zephyrbot

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 16, 2017

by Sharron LIU:

The test case itself supposed to keep the original logic to ensure assertion is made before test exiting, which is expected in the normal test flow.
Introducing variant zassert APIs to support a cleanup function pointer would be the way to go. Will need visit from ztest framework. I removed myself from the assignee. Thanks.

@zephyrbot

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 26, 2017

by Mark Linkmeyer:

Since this was previously assigned to Sharron, I'm reassigning it to Chandrakala Kempanna .

@nashif

This comment has been minimized.

Copy link
Member

commented Jul 25, 2018

@kumarvikash1 can you please assign to someone and see if this is still valid?

@nashif

This comment has been minimized.

Copy link
Member

commented Aug 31, 2018

@kumarvikash1 assign to some please..

@ManojSubbarao

This comment has been minimized.

Copy link

commented Sep 6, 2018

@spoorthik Please verify if this is still an issue and a valid scenario.

@spoorthik

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

@ManojSubbarao : Sure Manoj, I will have a look at it post 1.13 release and get back.

@spoorthik

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2018

As per the details provided in #3657, when there is any zassert failure, the threads are not terminated or clean up of threads (teardown_threads()) are not called, causing next set of tests to crash. (as the thread to be instantiated is already existing).

#3657 (comment)

This issue is no more seen. I tried to reproduce the issue by causing zassert condition to fail, see the code below:

index 1703f0d..31dadf2 100644
--- a/tests/kernel/sched/schedule_api/src/test_sched_timeslice_and_lock.c
+++ b/tests/kernel/sched/schedule_api/src/test_sched_timeslice_and_lock.c
@@ -179,6 +179,7 @@ void test_sleep_wakeup_preemptible(void)
                zassert_true(tdata[i].executed == 0, NULL);
        }
        k_wakeup(tdata[0].tid);
+       tdata[0].executed = 0;
        zassert_true(tdata[0].executed == 1, NULL);
        /* restore environment */
        teardown_threads();
@@ -222,6 +223,7 @@ void test_pending_thread_wakeup(void)
        /* TESTPOINT: k_wakeup() shouldn't resume
         * execution of pending thread
         */
+       executed = 1;
        zassert_true(executed != 1, "k_wakeup woke up a"
                     " pending thread!");

but ztest handles the assert failures gracefully and doesn't propagate any crash. See the execution log below with the code changes mentioned above:

Tested on nrf51_pca10028:

***** delaying boot 1000ms (per build configuration) *****
***** Booting Zephyr OS v1.13.0-rc3-8-g38a7950 (delayed boot 1000ms) *****
Running test suite threads_scheduling
===================================================================
starting test - test_priority_cooperative
PASS - test_priority_cooperative
===================================================================
starting test - test_priority_preemptible
PASS - test_priority_preemptible
===================================================================
starting test - test_yield_cooperative
PASS - test_yield_cooperative
===================================================================
starting test - test_sleep_cooperative
PASS - test_sleep_cooperative
===================================================================
starting test - test_sleep_wakeup_preemptible

    Assertion failed at /home/kspoorth/work/latest_zephyr/tests/kernel/sched/schedule_api/src/test_sched_timeslice_and_lock.c:183: test_sleep_wakeup_preemptible: tdata[0].executed == 1 is false

FAIL - test_sleep_wakeup_preemptible
===================================================================
starting test - test_pending_thread_wakeup

    Assertion failed at /home/kspoorth/work/latest_zephyr/tests/kernel/sched/schedule_api/src/test_sched_timeslice_and_lock.c:228: test_pending_thread_wakeup: (executed != 1 is false)
k_wakeup woke up a pending thread!
FAIL - test_pending_thread_wakeup
===================================================================
starting test - test_time_slicing_preemptible
PASS - test_time_slicing_preemptible
===================================================================
starting test - test_time_slicing_disable_preemptible
PASS - test_time_slicing_disable_preemptible
===================================================================
starting test - test_lock_preemptible
PASS - test_lock_preemptible
===================================================================
starting test - test_unlock_preemptible
PASS - test_unlock_preemptible
===================================================================
starting test - test_sched_is_preempt_thread
PASS - test_sched_is_preempt_thread
===================================================================
starting test - test_slice_reset
PASS - test_slice_reset
===================================================================
starting test - test_slice_scheduling
AB
AB
AB
AB
AB
AB
PASS - test_slice_scheduling
===================================================================
starting test - test_priority_scheduling
AB
AB
AB
AB
AB
PASS - test_priority_scheduling
===================================================================
starting test - test_wakeup_expired_timer_thread
PASS - test_wakeup_expired_timer_thread
===================================================================
===================================================================

PROJECT EXECUTION FAILED

So, the issue reported is no more seen and the issue can be closed.

@spoorthik

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2018

@nashif @kumarvikash1 @ManojSubbarao : Closing the issue as it is no more seen on the latest master.

@spoorthik spoorthik closed this Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.