From e0ca403f4c830edac0dd377a3f694a31ade8167a Mon Sep 17 00:00:00 2001 From: Andrew Boie Date: Sat, 5 Sep 2020 19:36:08 -0700 Subject: [PATCH] kernel: add assert for mis-used k_thread_create() k_thread_create() works as expected on both uninitialized memory, or threads that have completely exited. However, horrible and difficult to comprehend things can happen if a thread object is already being used by the kernel and k_thread_create() is called on it. Historically this has been a problem with test cases trying to be parsimonious with thread objects and not properly cleaning up after themselves. Add an assertion for this which should catch both the illegal creation of a thread already active, or threads racing to create the same thread object. Signed-off-by: Andrew Boie --- include/kernel.h | 6 ++++++ kernel/sched.c | 4 ++++ kernel/thread.c | 12 ++++++++++++ 3 files changed, 22 insertions(+) diff --git a/include/kernel.h b/include/kernel.h index 4aa6ebdddc5..ad281824a05 100644 --- a/include/kernel.h +++ b/include/kernel.h @@ -516,6 +516,12 @@ struct _thread_base { #endif _wait_q_t join_waiters; +#if __ASSERT_ON + /* For detecting calls to k_thread_create() on threads that are + * already active + */ + atomic_t cookie; +#endif }; typedef struct _thread_base _thread_base_t; diff --git a/kernel/sched.c b/kernel/sched.c index b9c7e3b55b0..a5932070d24 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -15,6 +15,7 @@ #include #include #include +#include LOG_MODULE_DECLARE(os); /* Maximum time between the time a self-aborting thread flags itself @@ -603,6 +604,9 @@ void z_thread_single_abort(struct k_thread *thread) * somewhat undefined behavoir. It must be safe to call * k_thread_create() or free the object at this point. */ +#if __ASSERT_ON + atomic_clear(&thread->base.cookie); +#endif } if (fn_abort != NULL) { diff --git a/kernel/thread.c b/kernel/thread.c index 64ec7c50cb8..c0dada6619e 100644 --- a/kernel/thread.c +++ b/kernel/thread.c @@ -27,6 +27,7 @@ #include #include #include +#include #define LOG_LEVEL CONFIG_KERNEL_LOG_LEVEL #include @@ -515,6 +516,8 @@ static char *setup_thread_stack(struct k_thread *new_thread, return stack_ptr; } +#define THREAD_COOKIE 0x1337C0D3 + /* * The provided stack_size value is presumed to be either the result of * K_THREAD_STACK_SIZEOF(stack), or the size value passed to the instance @@ -528,6 +531,15 @@ char *z_setup_new_thread(struct k_thread *new_thread, { char *stack_ptr; +#if __ASSERT_ON + atomic_val_t old_val = atomic_set(&new_thread->base.cookie, + THREAD_COOKIE); + /* Must be garbage or 0, never already set. Cleared at the end of + * z_thread_single_abort() + */ + __ASSERT(old_val != THREAD_COOKIE, + "re-use of active thread object %p detected", new_thread); +#endif Z_ASSERT_VALID_PRIO(prio, entry); #ifdef CONFIG_USERSPACE